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

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

Closed
hf-farmqa opened this issue May 17, 2021 · 6 comments · Fixed by #10698
Closed
Assignees
Labels

Comments

@hf-farmqa
Copy link

hf-farmqa commented May 17, 2021

mapbox-gl-js version:
1.13.1 -> 2.2.0 (latest)

browser
Safari/Firefox (Mac OS's only: IOS/macOS/etc)

Description

When adding a new 'Image' source to the map there appears to be a loading/timing issue with the initialization of the underlying content that prevents the image content from ever being displayed unless it is utilized somewhat 'immediately'.
This means that it appears to require that the source be referenced by a 'Raster' layer that is currently 'visible'.

It almost looks like an extension or regression of an earlier issue in Dec 2020: ImageSource not working in FF/Safari (#10230).

Steps to Trigger Behavior

Repro scenario 1:
An 'Image' source and corresponding 'Raster' layer are added to the map in quick succession but the layer's visibility layout property is set to 'none'. The underlying content will then never be displayed in this case even if the visibility property is later toggled to 'visible'

Repro scenario 2:
An 'Image' source is added to the map. Then at some point later a layer is added that references the source and is either initialized with the visibility layout property set to true or leaves it unset (true is the default).
Similarly, the content is never displayed on the map, however this shows that the loading problem is disjointed from the referencing layer and suggests some kind of timing issue where the source content has to be 'used' by some time period.

Console Logs

There are no warnings/errors within the console log and a cursory inspection of the map style state suggests that the map thinks everything is 'ok'. I.e SourceCache and linked Source properties within the map instance indicate that they believe the themselves to be 'loaded' (though I am not an expert on the internal structures of the library)

Workaround

I discovered that this issue is bypassed by way of the utilizing the 'raster-opacity' paint property as the means by which to show/hide the layer instead of the layout 'visibility' property. I'm not familiar enough with the implementation to know the implications of this finding.
For my particular scenario, this workaround is a valid and simple approach as my usage of 'raster-opacity' property is simplistic (0 or 1). However, this could become tricky if someone required more complex logic or expressions for this property.

@mourner
Copy link
Member

mourner commented May 21, 2021

@hf-farmqa please check #10698 — this should fix the issue.

@hf-farmqa
Copy link
Author

Sounds good. Thanks for the quick turn-around.

@anoopnair
Copy link

Hello - we are seeing some similar strange raster layer visibility issues with 1.13.1 as well.

  1. Does this same issue affect v1.13.1 as well?
  2. If yes, is there a patch release planned?

Thanks.

@anoopnair
Copy link

anoopnair commented Jun 5, 2021

@mourner - I am now increasingly confident that the issue we are seeing on v1.13.1 is the same issue described here since the suggested workaround involving raster-opacity works. Could you kindly comment on the question related to a potential patch version release?

Much appreciated, thanks!

@mourner
Copy link
Member

mourner commented Jun 7, 2021

Yes, we might want to backport this to v1, although it's not an urgent matter because there's an easy workaround.

@sblum51
Copy link

sblum51 commented Aug 4, 2021

thanks for the fix :)

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 a pull request may close this issue.

4 participants