Merge branch 'fix/a-photo-must-not-turn-on-its-side' into 'main'
Photos must not turn on their side; the keyboard must not cover the chat See merge request paulhitt/flamenet-messenger-android!96
This commit was merged in pull request #123.
This commit is contained in:
@@ -184,6 +184,10 @@ dependencies {
|
||||
// module's classpath would make `:client` unable to change its own
|
||||
// dependencies without breaking a screen that never mentioned it.
|
||||
implementation(project(":obfs4"))
|
||||
// 🔴 BitmapFactory ignores EXIF orientation, so a photo from a phone
|
||||
// camera decodes on its side. This reads the tag so the rotation can be
|
||||
// applied before re-encoding drops it. See ImagePrep.orientedFrom.
|
||||
implementation("androidx.exifinterface:exifinterface:1.3.7")
|
||||
implementation(platform("androidx.compose:compose-bom:2024.12.01"))
|
||||
implementation("androidx.compose.material3:material3")
|
||||
implementation("androidx.compose.material:material-icons-extended")
|
||||
|
||||
@@ -2,6 +2,9 @@ package io.flamenet.messenger.app
|
||||
|
||||
import android.graphics.Bitmap
|
||||
import android.graphics.BitmapFactory
|
||||
import android.graphics.Matrix
|
||||
import androidx.exifinterface.media.ExifInterface
|
||||
import java.io.ByteArrayInputStream
|
||||
import java.io.ByteArrayOutputStream
|
||||
|
||||
/**
|
||||
@@ -50,7 +53,7 @@ object ImagePrep {
|
||||
BitmapFactory.decodeByteArray(source, 0, source.size, bounds)
|
||||
if (bounds.outWidth <= 0 || bounds.outHeight <= 0) return null
|
||||
|
||||
val decoded = decodeScaled(source, bounds, maxSide) ?: return null
|
||||
val decoded = orientedFrom(source, decodeScaled(source, bounds, maxSide) ?: return null)
|
||||
|
||||
// Steps, best first. Each is a real fallback rather than a retry: for a
|
||||
// very large picture quality alone will not reach the budget and the
|
||||
@@ -105,7 +108,9 @@ object ImagePrep {
|
||||
val bounds = BitmapFactory.Options().apply { inJustDecodeBounds = true }
|
||||
BitmapFactory.decodeByteArray(source, 0, source.size, bounds)
|
||||
if (bounds.outWidth <= 0 || bounds.outHeight <= 0) return null
|
||||
return decodeScaled(source, bounds, maxSide)
|
||||
// Same tag, same problem: an image received from a peer that still
|
||||
// carries EXIF must be turned before it is drawn.
|
||||
return decodeScaled(source, bounds, maxSide)?.let { orientedFrom(source, it) }
|
||||
}
|
||||
|
||||
private fun decodeScaled(source: ByteArray, bounds: BitmapFactory.Options, maxSide: Int): Bitmap? {
|
||||
@@ -115,6 +120,57 @@ object ImagePrep {
|
||||
return BitmapFactory.decodeByteArray(source, 0, source.size, opts)
|
||||
}
|
||||
|
||||
/**
|
||||
* Turn the pixels the way the EXIF tag says, then forget the tag.
|
||||
*
|
||||
* 🔴 **`BitmapFactory` does not apply EXIF orientation, and re-encoding
|
||||
* throws the tag away.** A phone camera almost never writes upright
|
||||
* pixels: it writes landscape pixels plus "rotate this 90°". Decoding
|
||||
* ignores that, the JPEG we write out carries no EXIF at all — deliberately,
|
||||
* because that block holds the GPS coordinates the shot was taken at — and
|
||||
* so the only thing that knew which way was up is gone. Every portrait
|
||||
* photo arrived on its side.
|
||||
*
|
||||
* ⚠️ **iOS is not affected and that is why this survived**, so a fix
|
||||
* confirmed on one app proves nothing about the other: `UIImage(data:)`
|
||||
* applies the orientation as it decodes, so the same strip-and-re-encode is
|
||||
* safe there. Same design, opposite platform default.
|
||||
*
|
||||
* ⚠️ Returns the bitmap unchanged when there is nothing to do — the common
|
||||
* case, and it avoids copying a large bitmap for no reason.
|
||||
*/
|
||||
internal fun orientedFrom(source: ByteArray, bitmap: Bitmap): Bitmap {
|
||||
val tag = runCatching {
|
||||
ExifInterface(ByteArrayInputStream(source))
|
||||
.getAttributeInt(ExifInterface.TAG_ORIENTATION,
|
||||
ExifInterface.ORIENTATION_NORMAL)
|
||||
}.getOrDefault(ExifInterface.ORIENTATION_NORMAL)
|
||||
|
||||
val matrix = Matrix()
|
||||
when (tag) {
|
||||
ExifInterface.ORIENTATION_ROTATE_90 -> matrix.postRotate(90f)
|
||||
ExifInterface.ORIENTATION_ROTATE_180 -> matrix.postRotate(180f)
|
||||
ExifInterface.ORIENTATION_ROTATE_270 -> matrix.postRotate(270f)
|
||||
// ⚠️ The mirrored tags are rare but real — a front camera writes
|
||||
// them. Handled rather than ignored, because a selfie coming back
|
||||
// flipped is the same class of bug as one coming back sideways.
|
||||
ExifInterface.ORIENTATION_FLIP_HORIZONTAL -> matrix.postScale(-1f, 1f)
|
||||
ExifInterface.ORIENTATION_FLIP_VERTICAL -> matrix.postScale(1f, -1f)
|
||||
ExifInterface.ORIENTATION_TRANSPOSE -> {
|
||||
matrix.postRotate(90f); matrix.postScale(-1f, 1f)
|
||||
}
|
||||
ExifInterface.ORIENTATION_TRANSVERSE -> {
|
||||
matrix.postRotate(270f); matrix.postScale(-1f, 1f)
|
||||
}
|
||||
else -> return bitmap
|
||||
}
|
||||
val turned = runCatching {
|
||||
Bitmap.createBitmap(bitmap, 0, 0, bitmap.width, bitmap.height, matrix, true)
|
||||
}.getOrNull() ?: return bitmap
|
||||
if (turned !== bitmap) bitmap.recycle()
|
||||
return turned
|
||||
}
|
||||
|
||||
/** Scale so the longest side is at most [maxSide]. Never enlarges. */
|
||||
private fun scaleTo(bitmap: Bitmap, maxSide: Int): Bitmap {
|
||||
val longest = maxOf(bitmap.width, bitmap.height)
|
||||
|
||||
@@ -13,6 +13,7 @@ import androidx.compose.foundation.layout.Row
|
||||
import androidx.compose.foundation.layout.RowScope
|
||||
import androidx.compose.foundation.layout.Spacer
|
||||
import androidx.compose.foundation.layout.fillMaxSize
|
||||
import androidx.compose.foundation.layout.imePadding
|
||||
import androidx.compose.foundation.layout.fillMaxWidth
|
||||
import androidx.compose.foundation.layout.height
|
||||
import androidx.compose.foundation.layout.heightIn
|
||||
@@ -109,7 +110,21 @@ class MainActivity : androidx.fragment.app.FragmentActivity() {
|
||||
|
||||
setContent {
|
||||
MessengerTheme {
|
||||
Surface(modifier = Modifier.fillMaxSize()) {
|
||||
// 🔴 **`android:windowSoftInputMode="adjustResize"` stopped doing
|
||||
// anything at targetSdk 35.** Android 15 forces apps that target
|
||||
// it edge-to-edge, and an edge-to-edge window does not resize
|
||||
// for the keyboard — so the manifest line has been a no-op on
|
||||
// every device this ships to (minSdk 33), and the keyboard sat
|
||||
// over the chat and the box being typed into. Reported as
|
||||
// "bringing the chat above the keyboard would help me see what
|
||||
// I'm typing", which is exactly the symptom.
|
||||
//
|
||||
// ⚠️ At the root rather than per screen: every screen with a
|
||||
// text field has this problem, and one that is added later
|
||||
// would have it too. `imePadding` is inset consumption, so the
|
||||
// content shrinks and a scrolled list keeps its position —
|
||||
// adding padding under the keyboard, not scrolling over it.
|
||||
Surface(modifier = Modifier.fillMaxSize().imePadding()) {
|
||||
// 🔴 **The gate is around Root, not inside it.** Drawn over
|
||||
// the top, the message list still renders underneath for a
|
||||
// frame — and a frame is all a screenshot or a shoulder
|
||||
|
||||
@@ -81,8 +81,13 @@ object MetadataScrub {
|
||||
*/
|
||||
fun scrub(data: ByteArray): Result {
|
||||
val kind = kind(data) ?: return Result(data, null)
|
||||
val bitmap = runCatching { BitmapFactory.decodeByteArray(data, 0, data.size) }.getOrNull()
|
||||
val raw = runCatching { BitmapFactory.decodeByteArray(data, 0, data.size) }.getOrNull()
|
||||
?: return Result(data, null)
|
||||
// 🔴 This function's whole job is to remove the EXIF block — which
|
||||
// holds the GPS coordinates AND the tag saying which way is up. Turning
|
||||
// the pixels first is what keeps the second from being collateral
|
||||
// damage of removing the first. See ImagePrep.orientedFrom.
|
||||
val bitmap = ImagePrep.orientedFrom(data, raw)
|
||||
val out = ByteArrayOutputStream()
|
||||
// PNG where a lossy pass would be a second surprise; JPEG otherwise, at
|
||||
// a quality high enough not to be visible.
|
||||
|
||||
@@ -0,0 +1,90 @@
|
||||
package io.flamenet.messenger.app
|
||||
|
||||
import org.junit.Assert.assertEquals
|
||||
import org.junit.Assert.assertTrue
|
||||
import org.junit.Test
|
||||
import java.io.File
|
||||
|
||||
/**
|
||||
* A photo must arrive the way up it was taken.
|
||||
*
|
||||
* 🔴 Reported from a real phone: a photo auto-rotated. `BitmapFactory` does
|
||||
* NOT apply EXIF orientation, and every image this app sends is re-encoded to
|
||||
* strip EXIF — right, because that block carries the GPS coordinates the shot
|
||||
* was taken at. But the same block carries the tag saying which way is up, and
|
||||
* a phone camera rarely writes upright pixels: it writes landscape pixels plus
|
||||
* "turn this 90°". Decode ignoring the tag, then throw the tag away, and every
|
||||
* portrait photo lands on its side.
|
||||
*
|
||||
* ⚠️ These are source-level checks. The rotation itself needs a real Bitmap,
|
||||
* which is an instrumented test — but the wiring is what regressed and is what
|
||||
* a unit test can hold. See `PARITY.md`.
|
||||
*/
|
||||
class PhotoOrientationTest {
|
||||
|
||||
private fun source(path: String): String {
|
||||
val file = File("src/main/kotlin/io/flamenet/messenger/app/$path")
|
||||
assertTrue("missing $path", file.exists())
|
||||
return file.readText()
|
||||
}
|
||||
|
||||
/** The send path must turn the pixels before it drops the tag. */
|
||||
@Test
|
||||
fun preparingAPhotoAppliesTheExifTag() {
|
||||
val prep = source("ImagePrep.kt")
|
||||
assertTrue("prepare() must orient before encoding",
|
||||
prep.contains("orientedFrom(source, decodeScaled(source, bounds, maxSide)"))
|
||||
assertTrue("must read the tag rather than guess",
|
||||
prep.contains("ExifInterface.TAG_ORIENTATION"))
|
||||
}
|
||||
|
||||
/** 🔴 And so must the scrubber, whose entire job is removing that block. */
|
||||
@Test
|
||||
fun scrubbingAppliesTheExifTagBeforeRemovingIt() {
|
||||
assertTrue("scrub() must orient before re-encoding",
|
||||
source("MetadataScrub.kt").contains("ImagePrep.orientedFrom(data, raw)"))
|
||||
}
|
||||
|
||||
/** Images received from a peer carry the tag too. */
|
||||
@Test
|
||||
fun renderingAReceivedImageAppliesTheTag() {
|
||||
assertTrue("decodeBounded must orient what it decodes",
|
||||
source("ImagePrep.kt").contains("?.let { orientedFrom(source, it) }"))
|
||||
}
|
||||
|
||||
/**
|
||||
* ⚠️ The mirrored tags are rare but real — a front camera writes them, and
|
||||
* a selfie coming back flipped is the same class of bug as one coming back
|
||||
* sideways.
|
||||
*/
|
||||
@Test
|
||||
fun everyOrientationTagIsHandled() {
|
||||
val prep = source("ImagePrep.kt")
|
||||
for (tag in listOf(
|
||||
"ORIENTATION_ROTATE_90", "ORIENTATION_ROTATE_180", "ORIENTATION_ROTATE_270",
|
||||
"ORIENTATION_FLIP_HORIZONTAL", "ORIENTATION_FLIP_VERTICAL",
|
||||
"ORIENTATION_TRANSPOSE", "ORIENTATION_TRANSVERSE",
|
||||
)) {
|
||||
assertTrue("$tag is not handled", prep.contains(tag))
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* 🔴 `windowSoftInputMode="adjustResize"` is a no-op at targetSdk 35:
|
||||
* Android 15 forces edge-to-edge, and an edge-to-edge window does not
|
||||
* resize for the keyboard. The insets have to be handled in Compose.
|
||||
*/
|
||||
@Test
|
||||
fun theKeyboardDoesNotCoverWhatIsBeingTypedInto() {
|
||||
val activity = source("MainActivity.kt")
|
||||
assertTrue("the root must consume the IME inset",
|
||||
activity.contains("Modifier.fillMaxSize().imePadding()"))
|
||||
assertTrue("and import it", activity.contains(
|
||||
"import androidx.compose.foundation.layout.imePadding"))
|
||||
|
||||
// The manifest line is kept — it still applies on older targets — but
|
||||
// it must never again be the only thing relied on.
|
||||
val target = File("build.gradle.kts").readText()
|
||||
assertTrue("this test's premise is targetSdk 35", target.contains("targetSdk = 35"))
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user