Fix ImageSource not working in FF/Safari #10230
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
karimnaaji
approved these changes
Dec 19, 2020
There was a problem hiding this comment.
I wouldn't qualify this fix as hacky, it's fairly common to defer cpu resource deletion to a later frame to ensure graphics APIs can still retrieve the data. The concern is whether we are confident that a single frame is enough in that situation, and the fix does indeed work great on both firefox and safari.
|
In the announcement of V2, "Updates to the v1 releases will be made only for critical browser compatibility" is mentioned. Any chance for an emergency backport of this? The entire image source feature is broken in Firefox. |
1 task
|
@lights0123 yes, we're planning to do a v1.13 patch release with this fix in a few days. |
ryanhamley
pushed a commit
that referenced
this pull request
Feb 17, 2021
Merged
|
@lights0123 we released this as a patch v1.13.1 |
facebook-github-bot
pushed a commit
to terragraph/tgnms
that referenced
this pull request
Apr 30, 2021
Summary: upgrades mapbox to 1.13.1, node bearings were not showing in firefox/safari presumably due to: mapbox/mapbox-gl-js#10230 allow-large-files Reviewed By: cjminer505 Differential Revision: D28098026 fbshipit-source-id: 972b3df93f3b471f81c171f588500182e5a40822
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Closes #10226. Closes #10230. Turns out that since Safari/FF don't support offscreen canvas, ImageSource uses the old blob url image loading hack, however the image isn't bound to a GL texture immediately because this is deferred to a separate
preparestep. This leads to image sources trying to upload an image to GL that was already reset.This is a hacky workaround — wait until the next frame before releasing the image, so that
preparepasses can happen before. But it may be fragile — we should have a hard look at the architecture here and make it more resilient.Launch Checklist
mapbox-gl-jschangelog:<changelog>Fix ImageSource not working in some cases in Firefox & Safari.</changelog>