[Bug]: iOS: remote <Images> are removed once unused (no StyleImageRemoveUnused subscriber), and every { uri } entry is re-downloaded on any images change #4436

Mapbox Version

11.23.1 (the version 10.3.8 pins; 10.3.17 pins 11.33.0, not checked for a behaviour change in the core)

React Native Version

0.88.1 (New Architecture)

Platform

iOS

@rnmapbox/maps version

10.3.8 (observed from 10.3.5). The code involved is unchanged in 10.3.17, the newest release: sameImage and addImages in RNMBXImages.swift are the same, and RNMBXMapView.swift still has no styleImageRemoveUnused subscriber. 10.3.17's source was read but not run.

Standalone component to reproduce

import React, {useState} from 'react';
import {Button, View} from 'react-native';
import {MapView, Camera, Images, ShapeSource, SymbolLayer} from '@rnmapbox/maps';

const N = 30;
const points = {
  type: 'FeatureCollection',
  features: Array.from({length: N}, (_, i) => ({
    type: 'Feature',
    id: `p${i}`,
    properties: {id: `p${i}`},
    geometry: {
      type: 'Point',
      coordinates: [-74.0 + (i % 6) * 0.01, 40.7 + Math.floor(i / 6) * 0.01],
    },
  })),
};

const imagesFor = count =>
  Object.fromEntries(
    Array.from({length: count}, (_, i) => [
      `p${i}`,
      {uri: `https://picsum.photos/seed/p${i}/96`},
    ]),
  );

export default function BugReportExample() {
  const [count, setCount] = useState(N);
  return (
    <View style={{flex: 1}}>
      <MapView style={{flex: 1}}>
        <Camera centerCoordinate={[-73.975, 40.72]} zoomLevel={13} />
        <Images
          images={imagesFor(count)}
          onImageMissing={id => console.log('imageMissing', id)}
        />
        <ShapeSource id="points" shape={points}>
          <SymbolLayer
            id="photos"
            style={{iconImage: ['get', 'id'], iconAllowOverlap: true}}
          />
        </ShapeSource>
      </MapView>
      <Button title="Add one image" onPress={() => setCount(c => c + 1)} />
    </View>
  );
}

This is reduced from the app where both defects reproduce; it has not been run as written. Remote images are the subject of the bug, so it loads them from a public image host.

Observed behavior and steps to reproduce

Two defects, both in the iOS <Images> path for remote { uri } images.

1. Let the photos load, pan until those points leave the rendered area, then pan back. onImageMissing fires again for each returning key, and the symbols are drawn without their photos until the images download again. With 48 to 120 KB images served with some latency, the gap was 4 to 5 seconds.
2. Tap "Add one image". Every existing key is downloaded and added to the style again, not only the new one (watch the image host's requests through a proxy).

Expected behavior

1. An image that <Images> still lists stays in the style while its features are off screen, so returning features draw at once, as on Android.
2. Changing images downloads only the keys that are new or whose uri changed, as on Android (RNMBXImages.kt setImages compares value.uri != oldValue.uri).

Notes / preliminary analysis

1. Nothing subscribes to StyleImageRemoveUnused, so the map deletes requested images once no tile uses them. MBMStyleImageRemoveUnused.h (MapboxCoreMaps 11.23.1):

An image requested by the map through the StyleImageMissing event is no longer needed and can be removed using the removeStyleImage method. The application can decide whether the image should be kept or removed.

Note: If there are no subscribers for the StyleImageRemoveUnused event, the map will automatically remove the unused image.

RNMBXMapView.swift declares styleImageRemoveUnused next to styleImageMissing but never subscribes to it. A remote image whose symbol was laid out before its download landed is a requested image (onEvery(event: .styleImageMissing) calls addMissingImageToStyle), so the core removes it when its feature leaves the rendered tiles. Android doesn't hit this because addRemoteImages adds a placeholder before layout. iOS's addImages has the same intent ("Add image placeholder for images that are not yet available in the style"), but the placeholder branch is only reached when sameImage(oldValue:newValue:) is true, which it never is for a new key.

2. sameImage is never true for { uri } entries.

func sameImage(oldValue: Any?, newValue: Any?) -> Bool {
  if let oldS = oldValue as? String, let newS = newValue as? String {
    return oldS == newS
  } else {
    return false
  }
}

A { uri } entry arrives as a dictionary, so every key present in both the old and new images goes into missingImages, and RNMBXUtils.fetchImages(..., forceUpdate: true) downloads and addImages it again.

The fix running in the app, as a patch on 10.3.8:

1. Subscribe, and keep what an <Images> still lists, in RNMBXMapView.swift next to the styleImageMissing handler:

self.onEvery(event: .styleImageRemoveUnused) { (self, event) in
  let imageId = event.imageId
  if self.images.contains(where: { $0.lists(imageName: imageId) }) {
    return
  }
  self.withMapboxMap { map in
    logged("RNMBXMapView", "removeUnusedImage") {
      try map.removeImage(withId: imageId)
    }
  }
}

with, in RNMBXImages:

func lists(imageName: String) -> Bool {
  return images[imageName] != nil || nativeImageInfos.contains { $0.name == imageName }
}

2. Compare dictionary entries by value in sameImage:

} else if let oldD = oldValue as? NSDictionary, let newD = newValue as? NSDictionary {
  return oldD.isEqual(newD)
}

Two consequences an upstream version would need to handle, both covered in the patch:

• Once the subscriber keeps listed images, nothing removes an image the app stops listing (before, the core removed requested ones). The patch removes images that leave images (unless another <Images> on the map lists them), and drops a download that lands after its key was de-listed.
• Comparing by value removes the accidental retry defect 2 gave: a failed download used to be fetched again on the next images change, and the map reports a missing image only once, so nothing else retries. The patch retries a failed download after 2, 4 and 8 seconds while the image is still listed.

Additional links and references

• MBMStyleImageRemoveUnused.h in MapboxCoreMaps 11.23.1 (quoted above).