fix(images): register image views mounted after the map is ready (Android) - #4283
Open
duysolo wants to merge 1 commit into
Open
fix(images): register image views mounted after the map is ready (Android)#4283duysolo wants to merge 1 commit into
duysolo wants to merge 1 commit into
Conversation
…roid) RNMBXImages only connects its children during addToMap and never reconciles afterwards, so an image view that arrives at a different moment is silently left unwired. Two places on Android: - RNMBXImagesManager.addView: a child mounted after the component was added to the map is appended to mImageViews but never given the map, because addToMap only visits the views that were present when it ran. It can then never place its own child view. - RNMBXImage.addToMap: the reverse order, and the common one. With Fabric the child view is mounted into RNMBXImage before RNMBXImage is mounted into RNMBXImages, so addView() runs while mMapView is still null - and since addView() does not call super, the child is not placed anywhere at all. A view in no hierarchy is never attached to a window, and a Drawee controller only submits its request on attach, so an <Image> inside that child never loads. Drawing a view into a bitmap does not require attachment, which is why such a marker snapshots complete except for its image, with no onLoad, no onLoadStart and no onError to show for it. Because a later child never registers, applications remount the whole <Images> block whenever a new image appears, which tears down and re-snapshots every image already on the map to add one. Measured in a production app: 90 reloads to add 2 images, markers blank for up to 5.7 seconds; afterwards 1 image costs 1 load. Placement is guarded on childView.parent == null, so addToMap stays idempotent - it runs again on style changes.
duysolo
requested a deployment
to
CI with Mapbox Tokens
August 18, 2026 13:27 — with
GitHub Actions
Waiting
duysolo
requested a deployment
to
CI with Mapbox Tokens
August 18, 2026 13:27 — with
GitHub Actions
Waiting
duysolo
requested a deployment
to
CI with Mapbox Tokens
August 18, 2026 13:27 — with
GitHub Actions
Waiting
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
RNMBXImagesconnects its children into the map only insideaddToMap, and never reconcilesafterwards, so an image view that arrives at a different moment is silently left unwired. Two places
on Android:
RNMBXImagesManager.addViewmImageViewsbut never given the map, becauseaddToMaponly visits the views present when it ran. It can then never place its own child view.RNMBXImage.addToMapRNMBXImagebeforeRNMBXImageis mounted intoRNMBXImages, soaddView()runs whilemMapViewis stillnull- and sinceaddView()never callssuper.addView, the child is not placed anywhere at all.Why the symptom is confusing
A view in no hierarchy is never attached to a window, and a Drawee controller only submits its
request on attach. But drawing a view into a bitmap does not require attachment. So the
marker snapshots complete - circle, border, badge, text - and the only thing missing is the
<Image>inside it, with noonLoad, noonLoadStartand noonErrorto show for it.Two probes that look like they should catch this, and do not:
onLayoutfires with correct, non-zero bounds. Fabric applies layout metrics from the shadowtree whether or not the view is in a window.
measureInWindowreturns plausible coordinates for the same reason - not the ~-10000offset ofoffscreenAnnotationViewContainer, which is what a genuinely placed view reports.Case 2 is not an edge case, it is the first mount. In the app I measured, 27 of 27 image views
mounted with the map reported no
onLoadwithin a 4 s watchdog window; every marker renderedwithout its image. After the change the same watchdog reports 27 of 27 loaded.
What applications do about it today
Because a later child never registers, apps remount the whole
<Images>block whenever a new imageappears - which tears down and re-snapshots every image already on the map to add one.
Measured in a production app (Redmi, Android 16, RN 0.85.3, Fabric,
@rnmapbox/maps10.3.5), bytimestamping every
onLoadagainst the remount that caused it:expected 27, loaded 0expected 27, loaded 27, and 9 of 9 later generationssilent: 0Three map-type switches (standard / satellite / terrain) plus repeated fast panning, including
53-image and 38-image sets: all images intact afterwards.
The change
23 added lines, no deletions, 3 files:
RNMBXImages.attachImageViewIfOnMap(new): hand the map to a view that mounted later.RNMBXImagesManager.addView: call it.RNMBXImage.addToMap: placemChildViewif it arrived before the map did.Placement is guarded on
childView.parent == null, soaddToMapstays idempotent - it runs againon style changes.
Reproducer
Based on
example/src/examples/BugReportExample.js, self-contained, no extra libraries. EARLYmounts with the map; LATE mounts when you press Start. Both draw the same view: a coloured square
with a remote
<Image>on top. Counters showonLoadStart/onLoad/onErrorand anyonImageMissing.Expected: two markers, each showing the picture. On 10.3.5 the picture never appears and the image
counters stay at zero.
Honest status, per
.github/REPRODUCING.mditem 5: I could not run this scene in the exampleapp on my machine. The example app crashes at startup on
java.lang.NoClassDefFoundError: Failed resolution of: Lcom/facebook/react/viewmanagers/RNMBXCameraManagerInterface;fromPackageList.getPackages, before any scene loads. It is not caused by this change: the unfixedbuild fails identically, and the class is in fact present in the APK (
classes15.dex), so it lookslike a local dex/autolinking problem rather than a code issue. The measurements above therefore come
from a production app on a real device, instrumented to log every
onLoadStart/onLoad/onError/onImageMissingand to watchdog images that never load - not from this scene. If amaintainer can run it, it should fail on 10.3.5 and pass with this patch.
BugReportExample.js
Scope and risks
yarn generateproduces no diff (verified by running it).attachImageViewIfOnMapis a new public method onRNMBXImages. Additive only.addToMap, map-then-child by the manager;whichever runs second finds the other side already present.
addToMapruns again after a style reload; theparent == nullguard makesthe placement a no-op and the existing
refresh()in the style-loaded callback re-registers thebitmaps. Verified by hand on a device.
offscreenAnnotationViewContaineris aFrameLayoutwithLayoutParams(0, 0); markers render at the correct size after the change, so being placed theredoes not collapse the child.
RNMBXImageManager.onDropViewInstancedoesnot detach
mChildViewfromoffscreenAnnotationViewContainer, andRNMBXImagedoes notoverride
removeView. That leak already exists for every child that was successfully placed; thischange makes more children get placed, so it applies to more views. Happy to fix it here or in a
follow-up - tell me which you prefer.
addImageViewappends toimageViews, butimage.imagesisonly assigned by
addImageViews()fromaddToMap()), and a one-line fix works in my app. I amnot including it here because I have not reproduced it on iOS. Happy to send it separately.
Verification
yarn lintyarn type:checkyarn unittestyarn generateBUILD SUCCESSFUL, unfixed and fixed