From e47360864816c6fb9b608bce19c757732a8ca2f8 Mon Sep 17 00:00:00 2001 From: Bao Nguyen Date: Thu, 13 Aug 2026 16:22:58 +0700 Subject: [PATCH] fix: apply the documented 'cover' default for an omitted resizeMode An omitted `resizeMode` prop is never marked dirty, so `setResizeMode(...)` is never called and the view keeps whatever the platform's own default is - `scaleToFill` (stretch) on iOS, `FIT_CENTER` (contain) on Android - instead of the documented `cover`. Recycled views are worse still: they inherit the resize mode the previous View set. Apply the default where the prop system cannot skip it - at construction, and again when a view is prepared for recycling. Fixes #43 --- .../nitro-image-resize-mode-default-cover.png | Bin 0 -> 1094 bytes .../nitro-image-resize-mode-default-cover.png | Bin 0 -> 3926 bytes example/__tests__/resize-mode.harness.tsx | 141 ++++++++++++++++++ .../margelo/nitro/image/HybridImageView.kt | 15 +- .../ios/HybridImageView.swift | 4 + .../ios/Utils/CustomImageView.swift | 6 + 6 files changed, 165 insertions(+), 1 deletion(-) create mode 100644 example/__tests__/__image_snapshots__/android/nitro-image-resize-mode-default-cover.png create mode 100644 example/__tests__/__image_snapshots__/ios/nitro-image-resize-mode-default-cover.png create mode 100644 example/__tests__/resize-mode.harness.tsx diff --git a/example/__tests__/__image_snapshots__/android/nitro-image-resize-mode-default-cover.png b/example/__tests__/__image_snapshots__/android/nitro-image-resize-mode-default-cover.png new file mode 100644 index 0000000000000000000000000000000000000000..a421b1c6b78eb0912bb15fffe551c1f43d9bfb92 GIT binary patch literal 1094 zcmeAS@N?(olHy`uVBq!ia0y~yU}Ohj4mP03;?>Q|ffQqLkh>GZx^prwfgF}%C(jTL zAgJL;>0n@B{^0527*a9k?VXKz!hr$~7b`P%m)!V2Ti$O)#>F6uZo{1`a~~GZN;J|7 zinZNvq^fpq?b?~O^9_A3+dTVLC)eP=ZTscF+^MskJwAJT`r*PKTB`5fz4I^LzelF; zaUkFByZ^rY{`R;;^Jvs(b;+{bclX8Bo!gi8>Aj4*)1A9_=fB^VW?#iv@b&Gz{R|&w z9AWf$#Gq2hFv)_!Q>H>K0F*JAz|0K?tx_MW-v*Ur58 z{gk=>eB^Zg-Me?&->sef{W8O!?AhBdKVLLs+0QSV~+pJ6>W?b@uH z<(Ey)zP@wrdG&YB3YR#(Wtq>;Oh5dwas7_nyZ`_HK70D%gL&I;*E0)G*m(9wy1xUk PEMV|-^>bP0l+XkKI(mvj literal 0 HcmV?d00001 diff --git a/example/__tests__/__image_snapshots__/ios/nitro-image-resize-mode-default-cover.png b/example/__tests__/__image_snapshots__/ios/nitro-image-resize-mode-default-cover.png new file mode 100644 index 0000000000000000000000000000000000000000..6aee9ac31341b1fdc1f9f9c0a8024cc912c7ff8b GIT binary patch literal 3926 zcmeAS@N?(olHy`uVBq!ia0y~yVAKI&0Vbfxvi_rqKu&U|vvWYUv%7PCPJU5vL1J>M zOJ;FFPGV(%F$06f#M%iPy$=Tn9NQnZbU~?rtc;9Nr}LCaT)Ql`h`JPVgdTh~W#6)u z6OOr0Qerj#-`MoOk?UNZSi6;#1;?VvFP=SF@?g=^O8w3yN>%-j>uTQber%y&nOBha zoq2`zzUY`!+OwDHs($TZ*{i_w?Bkqu>%IQ1vw!^8?#n&D(~#Ala^D2BaZOd6x=Yl(845p)Bexq#{CVkMdlBK=C?}KygE5iNqxqV(**|% zn5RZKw*^fu?|WA+e%HTuYqNU7Lgkhn9b2dJ6_~lJXCJ?H#m2yhC1&f+hf3&7TYr!SfFa6fHVl)0AiqSD9yBh38sQkX8|*U4N`h|&F14kinG8YvY3HE z(E@}SU8fl)00m_-UHn6UG$^Tnz%(G%fYLxVJL^lfn@1QJ*e-gyIEGX(zPYwAZ$_y| z`@>?+-OR6M97s4Zsmp2F1f670SJg%?rbL#YBTh}3SJR${g{P#du1I>mEZfTdFzbW{ zi|HTjo_&6P^8e}Iv+w7}{rI-ZGzLqOqz$c*@Wy?#5(Z>kOr42(>^jXz&L`1S2_ z`#zAY!vakM`#FD0zubHd=imST`P;t_m!<<1vv8QOK0DsMd$m4a4JS}_gF|pa-OtB| ze@-`tE7$k?Z~N=j6`1mt3miao42@e3nEZa91$M#)CZ`$q9P5}t904aq6^Qf=u$;pJ z9fg*cvLSW9^3)v~4sdZS6c7U06P2LD1Xd6rCUEErpO$^?c3}mF1lE=S4v<3>bPdE9 zLGBaK;O0oIVxKyH-)*2c&?b!*kiRB~NpypJ%fc~*wZ&j>^C|!PZ-L?wjEW*oAYC5Z zJ+9#3U^xYL+|%Xn%Yfo*8U(l$LAsW(9#aB&gOMqfQPJo9f=`#rt%2fU4jio$K*nBT zN)`l#2Sekg27zVe0Y6{vo(mM$R%mhZ0O`tX+{gh6-Uf$A2ae0uEB<`B>jzXUitv5} z3&@?L-XD$q(ZoNR_vujdHU9j0u>G^TIK1G#|F^dM>ytCEg8SL)&D*!1m-_)Kei<5< z9hf8Yr?S52jW@U$oyGL|%ipi=_ho*=q;2Z{K6~+Vr9P;%6v*H@^KkL>`~SY3gPHb^ zoq^&1{{@eVCj%=t22kbp_L^dz2!lk+MSI3(#WmRm2eg{9ZZU7+6g}bVv2m@!)U#5T zq!$>flxABv{Z+DHl5=f3lA|qt&~RV+vGc~p#b2IZo_=0^5~%P6RZ3^y9)BONuL85Z z=N`RLt?yZvk7l0|>B{ub-Qsx!ITin8CoonYQNb9v@n zHwb5i! zso?Zz5DexZNGRM;vFElIs0;`76rR3*dhp$JSV?JVXJdEgExZ`L`SRw=znyT&__^!f vnZwKDSD#*ede{n++!=X)jl3C5@*m@7F5lqIrmIf^J3b7au6{1-oD!M = [ + [255, 0, 0], // red + [0, 255, 0], // green + [0, 0, 255], // blue + [255, 255, 255], // white +] + +/** + * A 20x40 (portrait) RGBA image made of four horizontal bands. + * + * Scaled into a 100x100 tile: + * - `stretch` squashes it vertically, so all four bands stay visible (25px each). + * - `cover` scales by 5 to 100x200 and centre-crops, so only the two middle + * bands (green, blue) remain visible, 50px each. + * - `contain` scales by 2.5 to 50x100 and letterboxes horizontally. + * + * So each mode is distinguishable from the others by pixels alone. + */ +function createStripedImage() { + const bytes = new Uint8Array(IMAGE_WIDTH * IMAGE_HEIGHT * 4) + for (let y = 0; y < IMAGE_HEIGHT; y++) { + const band = BANDS[Math.floor(y / (IMAGE_HEIGHT / BANDS.length))] + if (band == null) throw new Error(`No band for row ${y}!`) + const [r, g, b] = band + for (let x = 0; x < IMAGE_WIDTH; x++) { + const i = (y * IMAGE_WIDTH + x) * 4 + bytes[i] = r + bytes[i + 1] = g + bytes[i + 2] = b + bytes[i + 3] = 255 + } + } + return Images.loadFromRawPixelData( + { + buffer: bytes.buffer, + width: IMAGE_WIDTH, + height: IMAGE_HEIGHT, + pixelFormat: 'RGBA', + }, + false, + ) +} + +/** Number of differing bytes between two PNG screenshots of the same size. */ +function countDifferingBytes(a: Uint8Array, b: Uint8Array): number { + if (a.length !== b.length) return Math.max(a.length, b.length) + let differing = 0 + for (let i = 0; i < a.length; i++) { + if (a[i] !== b[i]) differing++ + } + return differing +} + +/** + * Renders one tile - the striped image inside a 100x100 square - with the given + * `resizeMode`, and returns a screenshot of it. `undefined` means the prop is + * not passed at all, which is the case under test. + */ +async function screenshotTile( + testID: string, + resizeMode?: NitroImageProps['resizeMode'], +) { + const image = createStripedImage() + await render( + + + , + ) + const tile = await screen.findByTestId(testID) + const shot = await screen.screenshot(tile) + if (shot == null) throw new Error(`Failed to screenshot <${testID} />!`) + return shot +} + +describe('NitroImage view - default resizeMode', () => { + it('renders an omitted resizeMode exactly like resizeMode="cover"', async () => { + const omitted = await screenshotTile('resize-mode-omitted') + const cover = await screenshotTile('resize-mode-cover', 'cover') + + // The actual regression assertion: no `resizeMode` must render identically + // to `resizeMode="cover"`. Before the fix the omitted one was stretched, so + // it showed all four bands while `cover` showed only the middle two. + expect(countDifferingBytes(omitted.data, cover.data)).toBe(0) + + // Both renders are additionally locked against a single committed baseline, + // so a future change to what `cover` itself means is caught as well. The + // small tolerance only absorbs anti-aliasing along the one colour boundary; + // a different resize mode moves ~50% of the pixels. + const snapshot = { + name: 'nitro-image-resize-mode-default-cover', + failureThreshold: 0.01, + failureThresholdType: 'percent', + } as const + await expect(cover).toMatchImageSnapshot(snapshot) + await expect(omitted).toMatchImageSnapshot(snapshot) + }) + + it('still defaults to cover after a View that set a different resizeMode', async () => { + // NitroImage views are recycled (`HybridNitroImageViewComponent` + // `shouldBeRecycled` / `RecyclableView`). A recycled view keeps the resize + // mode the previous View set, and an omitted `resizeMode` never overwrites + // it - so without a reset on recycle this render inherits `contain`. + await screenshotTile('resize-mode-contain', 'contain') + const omitted = await screenshotTile('resize-mode-omitted-2') + const cover = await screenshotTile('resize-mode-cover-2', 'cover') + + expect(countDifferingBytes(omitted.data, cover.data)).toBe(0) + }) + + it('distinguishes cover from stretch, so the assertions above are not vacuous', async () => { + // If this ever hits 0, the striped fixture stopped being able to tell the + // resize modes apart and every test above silently became meaningless. + const cover = await screenshotTile('resize-mode-cover-3', 'cover') + const stretch = await screenshotTile('resize-mode-stretch', 'stretch') + + expect(countDifferingBytes(cover.data, stretch.data)).toBeGreaterThan(0) + }) +}) diff --git a/packages/react-native-nitro-image/android/src/main/java/com/margelo/nitro/image/HybridImageView.kt b/packages/react-native-nitro-image/android/src/main/java/com/margelo/nitro/image/HybridImageView.kt index 672c3e2c..d43a490d 100644 --- a/packages/react-native-nitro-image/android/src/main/java/com/margelo/nitro/image/HybridImageView.kt +++ b/packages/react-native-nitro-image/android/src/main/java/com/margelo/nitro/image/HybridImageView.kt @@ -27,7 +27,7 @@ class HybridImageView(context: Context): HybridNitroImageViewSpec(), RecyclableV } override val view: View = imageView - override var resizeMode: ResizeMode? = ResizeMode.CONTAIN + override var resizeMode: ResizeMode? = ResizeMode.COVER set(value) { field = value uiScope.launch { @@ -49,9 +49,22 @@ class HybridImageView(context: Context): HybridNitroImageViewSpec(), RecyclableV field = value } + init { + // A property initializer assigns the backing field directly, so the + // `resizeMode` setter (which applies the mapping) does not run. And an + // omitted `resizeMode` prop is never marked dirty, so it is never + // assigned either - without this, `scaleType` would stay at + // `ImageView`'s own default (`FIT_CENTER`, i.e. `contain`). + updateResizeMode() + } + override fun prepareForRecycle() { onDisappear() imageView.setImageBitmap(null) + // A recycled view keeps the `scaleType` the previous View set. If the + // next View omits `resizeMode`, its prop is never dirty and never + // assigned, so it would silently inherit that mode - reset it here. + resizeMode = ResizeMode.COVER } private fun updateResizeMode() { diff --git a/packages/react-native-nitro-image/ios/HybridImageView.swift b/packages/react-native-nitro-image/ios/HybridImageView.swift index 96f47848..4f60dd2a 100644 --- a/packages/react-native-nitro-image/ios/HybridImageView.swift +++ b/packages/react-native-nitro-image/ios/HybridImageView.swift @@ -111,6 +111,10 @@ extension HybridImageView: RecyclableView { func prepareForRecycle() { willHide() view.image = nil + // A recycled view keeps the `contentMode` the previous View set. If the + // next View omits `resizeMode`, its prop is never dirty and never assigned, + // so it would silently inherit that mode - reset to the default here. + resizeMode = nil } } diff --git a/packages/react-native-nitro-image/ios/Utils/CustomImageView.swift b/packages/react-native-nitro-image/ios/Utils/CustomImageView.swift index fb810479..c960ca49 100644 --- a/packages/react-native-nitro-image/ios/Utils/CustomImageView.swift +++ b/packages/react-native-nitro-image/ios/Utils/CustomImageView.swift @@ -23,6 +23,12 @@ internal class CustomImageView: UIImageView { init() { super.init(image: nil) self.clipsToBounds = true + // `UIImageView` defaults `contentMode` to `.scaleToFill` (= stretch), but + // `resizeMode` is documented to default to `cover`. When the `resizeMode` + // prop is omitted it is never marked dirty, so `HybridImageView.resizeMode` + // never gets assigned and its `didSet` (which applies the mapping) never + // runs - which means the default has to be applied here, at construction. + self.contentMode = .scaleAspectFill } required init?(coder: NSCoder) { fatalError("init(coder:) has not been implemented")