Skip to content
This repository has been archived by the owner on Aug 8, 2023. It is now read-only.

allow for user-provided symbol imagery (aka Sprites) #941

Merged
merged 41 commits into from
Jul 8, 2015

Conversation

kkaefer
Copy link
Contributor

@kkaefer kkaefer commented Jul 2, 2015

As part of #877, we need to take user-provided marker imagery and render it in the GL layer. We also support standard Maki icons, and beyond the iOS beta, we will support native UIView / Android views tracking atop the GL view.

We need an API for a user to provide a bitmap into the system and have it become part of our sprite sheet, independent of style currently in play.

/cc @kkaefer @1ec5 @bleege

@incanus incanus added the feature label Mar 4, 2015
@incanus incanus added this to the iOS Beta milestone Mar 4, 2015
@incanus incanus mentioned this pull request Mar 4, 2015
@twaysive
Copy link

twaysive commented Mar 4, 2015

Are there plans for NSView for OS X?

@incanus
Copy link
Contributor Author

incanus commented Jun 1, 2015

@kkaefer is starting to dig into this as part of the b3+ focus.

@incanus incanus modified the milestones: iOS Beta 3, iOS Beta 2 Jun 1, 2015
@bleege bleege changed the title allow for user-provided symbol imagery allow for user-provided symbol imagery (aka Sprites) Jun 1, 2015
@kkaefer
Copy link
Contributor

kkaefer commented Jun 10, 2015

A few observations:

  • The sprite atlas needs to be aware of what images are used so we can remove from from the sprite atlas when they're no longer used. This means buckets have to track the sprite images they're using and release them when they get destructed
  • Currently unused sprites should be stored in a SpriteStore object (like GlyphStore).
  • Some sprites are queried at run-time because they're based on the outcome of a style sheet function. When rendering a frame, we should first make sure that we have all sprites required for the frame before binding/updating the sprite atlas.
  • When a sprite in the sprite store gets updated, its dimensions can change. In that case, we'd need to reencode all buckets that depend on this sprite. Alternatively, we could prohibit changing the sprite dimensions
  • When parsing a symbol bucket, we query the atlas for an image, which in turn queries the sprite store if it doesn't have the image yet
  • When removing an image from the sprite atlas, we should not remove the image immediately, but move it to a free list. When adding a new image and there is no space in the atlas, we delete the image from the free list and reuse the space in the texture. When the image that is being added was the same that was previously removed, we don't need to do anything (this may save some texture uploads).
  • Most of this infrastructure has the same requirements as Glyphs, and we should merge their code bases
  • We should support unnamed sprite images that can be added programmatically to avoid having to copy around the sprite image name everywhere

Open questions:

  • When a sprite image can't be found, what do we do? Right now, we're not processing the bucket when the Sprite hasn't loaded, but when adding/removing custom markers, we don't have a clear definition of when sprites have been loaded anymore. E.g. the user could remove sprite images at any point, or create annotations, but not adding the corresponding sprite image at the same time (e.g. because the user still has to wait for the custom sprite image to be created/load). We could process the bucket nonetheless with only the available icons, then reprocess as more sprite images become available, but that means we'd have to get our reprocess workflow to function correctly. Alternatively, we could use a generic icon, but that means tiles encoded before the sprite was loaded will contain the generic icons (and we'd need to trigger a reparse).

@mb12
Copy link

mb12 commented Jun 10, 2015

Can we check for the existence/validity of the image at the time of creating the PinAnnotation object that conforms to MGLAnnotation and return nil for error cases? This way the image is guaranteed to be there whenever it needs to be processed for use in SymbolBucket.

@incanus
Copy link
Contributor Author

incanus commented Jun 10, 2015

Can we check for the existence/validity of the image at the time of creating the PinAnnotation object that conforms to MGLAnnotation and return nil for error cases?

This becomes a bottleneck when compared to Apple's model, since images are only checked at point of need and not when added. Consider adding thousands of annotations, not necessarily yet visible, which could spawn thousands of disk checks for imagery by way of delegate callbacks...

@mb12
Copy link

mb12 commented Jun 10, 2015

1.) You are right this check can be expensive in several cases e.g. if the image is an http purl. In most cases, even if the number of annotations is large, the number of unique images would be less.

2.) Maybe if the image cannot be found or loaded, we can just use a default image like "?" or any other default image.

@kkaefer kkaefer self-assigned this Jul 2, 2015
@kkaefer kkaefer added the ⚠️ DO NOT MERGE Work in progress, proof of concept, or on hold label Jul 2, 2015
@kkaefer
Copy link
Contributor

kkaefer commented Jul 2, 2015

  • add an example/shortcut in the desktop + iOS test app
  • add test that checks rendering images with custom sprite images
  • Add tests for SpriteAtlas
    • Add a few images to the Sprite atlas, check that correct texture results
    • Add images that don't exist in the SpriteStore
    • Update SpriteStore, make sure that images in the atlas get updated
    • same texture, but different wrap values return different positions
  • Expose SpriteImage object, so users can construct them from the public headers
  • Cocoa wrapper

@kkaefer kkaefer force-pushed the 941-custom-sprite-imagery branch from 3293162 to 5f8e5ad Compare July 6, 2015 11:13
@kkaefer
Copy link
Contributor

kkaefer commented Jul 6, 2015

@incanus All of the C++ APIs should now be here. Do you want to sync up later on how the Cocoa APIs should look like?

@kkaefer
Copy link
Contributor

kkaefer commented Jul 6, 2015

Open tasks that can become separate issues:

  • Remove icons from sprite atlas once we're done with them (like GlyphAtlas does already)
  • Merge GlyphAtlas and SpriteAtlas (and LineAtlas?) implementation
  • When removing an image from the sprite atlas, we should not remove the image immediately, but move it to a free list. When adding a new image and there is no space in the atlas, we delete the image from the free list and reuse the space in the texture. When the image that is being added was the same that was previously removed, we don't need to do anything (this may save some texture uploads).

@incanus
Copy link
Contributor Author

incanus commented Jul 6, 2015

Per voice with @kkaefer, I am going to grab the Cocoa wrapper for this around passing user UIImage objects as pixel arrays to C++ as sprites to add to core.

@incanus
Copy link
Contributor Author

incanus commented Jul 7, 2015

b2d2690 has a first cut at a Cocoa API for this.

  • HACK: Currently sending back a random-colored pixel array much like the OS X P key demo since I'm having trouble turning a runtime UIImage into a pixel array — I think this is what you were seeing @kkaefer with non-premultiplied alpha?

Other notes:

  • Added -[MGLMapViewDelegate mapView:imageForAnnotation:], which can return nil to use the default marker for a style (e.g. default_marker) or an MGLAnnotationImage object.
  • An MGLAnnotationImage object has an associated UIImage and a reuseIdentifier string much like MKAnnotationView. You can either create a new MGLAnnotationImage with a runtime image or you can obtain an existing one to reuse efficiently with -[MGLMapView dequeueReusableAnnotationImageWithIdentifier:]. In the latter case, you can't change the imagery — the whole idea is that you reuse the imagery.
  • Custom imagery still properly queries the backend for the sprite offset so that the popup placement looks right.
  • Deprecated -[MGLMapViewDelegate mapView:symbolForAnnotation:]. Users no longer have to care about symbol names!
  • This currently logs a bit of info for debugging purposes so that you can see that sprites are properly being reused when indicated to have the same reuseIdentifier.
  • Still needs some cleanup around removal of the last instance of a custom sprite during annotation removal.
  • Currently doing some demo hackery around using different sprites in the iOS demo app.

@incanus
Copy link
Contributor Author

incanus commented Jul 7, 2015

@kkaefer 8520e71 stubs out annotation symbol cleanup inline during removeAnnotations() rather than some sort of periodic garbage collection. However, since neither setSprite() nor removeAnnotations() are called synchronously, this can crash as we still need to render annotations for at least a frame afterwards before removal.

I will dig at this a bit, but it's crashing in SpriteAtlas::copy() when holder.texture is NULL.

Any recommendations?

@incanus
Copy link
Contributor Author

incanus commented Jul 7, 2015

Pretend red_square is our custom-drawn UIImage, when actually it's a random little circle, but still drawn on the fly.

screen shot 2015-07-06 at 6 47 59 pm

/cc @1ec5 @friedbunny on b2d2690 for thoughts on the nascent Cocoa API.

@kkaefer kkaefer force-pushed the 941-custom-sprite-imagery branch from 4b62104 to fb230dd Compare July 8, 2015 17:46
@kkaefer kkaefer merged commit fb230dd into master Jul 8, 2015
@kkaefer kkaefer deleted the 941-custom-sprite-imagery branch July 8, 2015 18:08
@1ec5 1ec5 removed the ⚠️ DO NOT MERGE Work in progress, proof of concept, or on hold label Jul 8, 2015
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants