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

Fix ImageSource breaking in FF/Safari if it's not immediately displayed #10698

Merged
merged 1 commit into from
May 27, 2021

Conversation

mourner
Copy link
Member

@mourner mourner commented May 21, 2021

Closes #10685 by immediately decoding an image of an ImageSource on load for use later so that we don't hit a race condition where the memory for it is freed preemptively.

Launch Checklist

  • briefly describe the changes in this PR
  • write tests for all new functionality
  • manually test the debug page
  • apply changelog label ('bug', 'feature', 'docs', etc) or use the label 'skip changelog'
  • add an entry inside this element for inclusion in the mapbox-gl-js changelog: <changelog>Fix ImageSource breaking in FF/Safari if it's not immediately visible.</changelog>

Copy link
Contributor

@rreusser rreusser left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 👍

@@ -78,7 +79,7 @@ class ImageSource extends Evented implements Source {
dispatcher: Dispatcher;
map: Map;
texture: Texture | null;
image: HTMLImageElement | ImageBitmap;
image: ImageData;
Copy link
Contributor

@rreusser rreusser May 24, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Question: Makes perfect sense. Just out of curiosity, it seems this is not set until loaded. As far as flow goes, is ImageData | null just not worth the trouble in this case?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just didn't change the status quo in this particular PR, but we do need to make a sweep to clean this up — sloppy typing like this is everywhere both in this class and all over the codebase, and Flow isn't complaining. :(

@mourner mourner merged commit f47f69d into main May 27, 2021
@mourner mourner deleted the fix-delayed-image-source branch May 27, 2021 18:15
SnailBones pushed a commit to SnailBones/mapbox-gl-js that referenced this pull request Jul 15, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Image source content is not loaded properly if not utilized 'quickly enough' on Safari/FF
3 participants