mapbox / mapbox-gl-native

Interactive, thoroughly customizable maps in native Android, iOS, macOS, Node.js, and Qt applications, powered by vector tiles and OpenGL
https://mapbox.com/mobile
Other
4.35k stars 1.33k forks source link

Images set to style are removed when reducing memory use and causing crash #16534

Open sofla2xper opened 3 years ago

sofla2xper commented 3 years ago

Steps to reproduce

Given a style with an iconLayer and manually added and removed images. The iconImageName is declared as: iconLayer.iconImageName = NSExpression(forKeyPath: "iconImageName") It appears many, or large, images are needed to reproduce and that triggering ImageManager::reduceMemoryUse() causes the issue.

  1. Add images to style using setImage(_ image: UIImage, forName name: String)
  2. Remove a subset of the images from the style, let's call the subsets A for the images present and B for the ones removed.
  3. Re-add the removed subset (B), without any other changes to the images (no removed or edited images)
  4. Once again remove the subset (B)

Expected behavior

The images are expected to be toggled present, not present in the map as they are added/removed

Actual behavior

  1. All images are present as expected (A+B)
  2. Only the non-removed images are present as expected (A)
  3. The readded images are missing, only set A is present, expected (A+B)
  4. The application crashes

My theory about what causes the issue of the missing images: When calculating the image diff and images are added, hasImageDiff is never set to true unless images are also removed or has changed size. Adding hasImageDiff = true; after this line appears to solve the isse: https://github.com/mapbox/mapbox-gl-native/blob/master/src/mbgl/renderer/render_orchestrator.cpp#L205 Then the images are not considered unused and won't be removed in ImageManager::reduceMemoryUse()

The crash in the next step appears fixed in https://github.com/mapbox/mapbox-gl-native/commit/f79334c12440f5a8a52bcbe4e5978a4f13381aec

Configuration

Mapbox SDK versions: maps-v1.5.1 master(77b883d8) iOS/macOS versions: iOS 13.4 Device/simulator models: iPadPro 11 inch second generation Xcode version: 11.5