Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add style#setSprite method #3662

Closed
wants to merge 1 commit into from
Closed

Add style#setSprite method #3662

wants to merge 1 commit into from

Conversation

anandthakker
Copy link
Contributor

⚠️ note the base branch of this PR is smart-set-style (#3621), not master ⚠️

Closes #2058

This turned out to be simpler than I thought (unless I'm missing something). Render tests, including the new set-style runtime styling tests, still pass, and the light/dark setStyle debug page works, now with changing sprite URIs.

@anandthakker
Copy link
Contributor Author

👋 @lucaswoj @jfirebaugh just flagging that this is a proposed amendment to #3621, which we talked about merging tomorrow.

@jfirebaugh jfirebaugh changed the base branch from smart-set-style to master November 22, 2016 00:34
@jfirebaugh
Copy link
Contributor

I'm seeing some visual artifacting when there are icons on screen while the transition occurs. I suspect there's a race condition where, prior to the tiles finishing their reload, buffers that contain texture coordinates relative to the prior sprite are drawn with the newer sprite.

@anandthakker
Copy link
Contributor Author

@jfirebaugh what do you think is the ideal behavior here? Should we essentially track/detect when a tile's got stale spritesheet data, and bail out of rendering the offending layer? Or do we want to try something more ambitious, like cross-fading between the old and new sprites?

@jfirebaugh
Copy link
Contributor

It's a hard problem, and I don't see any easy solution. The sprite texture is essentially a shared resource that safe to delete only when there are no buffers containing texture coordinates that refer to it. It's similar to why updating a single image in the sprite is hard.

@jfirebaugh
Copy link
Contributor

I think the solution here has to be something like: treat the texture itself as a persistent resource -- never delete it. Instead, reference count individual images within it, like we do for glyphs. We'll need to add a generation identifier to the user-specified image identifier, to allow setSprite to overwrite previous images, while still retaining the previous image until tiles that use it have been re-parsed.

@anandthakker
Copy link
Contributor Author

Closing here, since the issue it targets ( #2058 ) is closed in favor of #2059

@anandthakker anandthakker deleted the set-sprite branch January 20, 2017 02:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants