maplibre / maplibre-gl-js

MapLibre GL JS - Interactive vector tile maps in the browser
https://maplibre.org/maplibre-gl-js/docs/
Other
6.67k stars 716 forks source link

Firefox WebGL warning (Alpha-premult and y-flip are deprecated for non-DOM-Element uploads). #2030

Open jwoodwardtfx opened 1 year ago

jwoodwardtfx commented 1 year ago

When running in Firefox we get the following warning: WebGL warning: texImage: Alpha-premult and y-flip are deprecated for non-DOM-Element uploads.

Google shows up the following: https://bugzilla.mozilla.org/show_bug.cgi?id=1400077 https://github.com/mapbox/mapbox-gl-js/issues/5292

Is this an issue or a performance limitation we should be concerned about (as the Firefox developers say this path isn't optimized and should not be used)?

morgoth commented 1 year ago

Since the issue is known at least from 2017, vide https://github.com/mapbox/mapbox-gl-js/issues/5292 is there any plan to fix/workaround/address this problem?

HarelM commented 1 year ago

Can you please share a reproduction of this issue? Generally speaking, Firefox has a very low usage statistics when it comes to browsers, but if someone would like to invest in fixing this, please do. If this is a performance concern I might be able to assign a bounty to it, but I'll need a clear reproduction example first

morgoth commented 1 year ago

You can just open this page in Firefox https://maplibre.org/maplibre-gl-js-docs/example/simple-map/ and look into the console.

I'm using Firefox 110.0 (64-bit) on Ubuntu

HarelM commented 1 year ago

Would you be interested in working on this issue?

HarelM commented 1 year ago

Thanks for the info!

Assigned L bounty. Boundy direction: https://github.com/maplibre/maplibre/issues/192

Here's a screenshot of the warning: image

morgoth commented 1 year ago

@HarelM I'm not even an user yet ;-) - just investigating future possibilities for the integration and that was the first "warning" to see the known issue opened since 2017 without addressing.

IvanSanchez commented 1 year ago

I can have a look at this.

I'm having a look at the code paths triggering the warnings, and am currently considering skipping the creation of textures when the atlases are empty. This won't get rid of all the warnings, but it should avoid telling the GPU to allocate 1x1 px textures needlessly.

HarelM commented 1 year ago

Would be very nice if you could push this forward. Let me know if you would like me to assign this issue to you.

IvanSanchez commented 1 year ago

@HarelM Sure thing!

Have a look at #2297 - it gets rid of all warnings except for the first one. I'll have to look at the root cause of that one.

WebMechanic commented 1 year ago

@IvanSanchez thanks for working on this!

Is this on the 3.0 roadtrack only or can it be "backported" to the 2.4 branch? Firefox userbase in Europe is at least on par with Edge (varies by stats). These warnings are super annoying poluting the console and Mozilla apparently doesn't care.

They show up with the basic-preview or osm-bright styles (local tileserver.org 8.x instance) also with https://demotiles.maplibre.org/style.json

Is there something a "map server admin" can do to fix these missing sprites/textures? Hints welcome.

thank you.

IvanSanchez commented 1 year ago

@WebMechanic The #2297 & #2299 PRs are targeting main, which means it'd be into the 3.0 roadmap.

The actual changes are rather minimal, so I guess it'd be possible to backport them. I'd rather not do so unless @HarelM asks me to.

HarelM commented 1 year ago

I can't think of a strong argument to backport this into 2.4. we should be releasing 3.0 shortly (hopefully).

WebMechanic commented 1 year ago

I can't think of a strong argument to backport this into 2.4. we should be releasing 3.0 shortly (hopefully).

If 3.0 is virtually a seamless upgrade from 2.4 w/o any BC breaks, then sure.

However, if 3.0 introduces any significant API changes, we (and likely other "users") might neither have the time nor resources to make our current component code work with this major release.

Looking forward to the 3.0 release. Thanks.

IvanSanchez commented 1 year ago

@WebMechanic You can check the current changelog for yourself: https://github.com/maplibre/maplibre-gl-js/blob/main/CHANGELOG.md - see changes marked with [breaking]. Those shouldn't have a big impact on the userbase.

birkskyum commented 1 year ago

Checked the debug pages to make sure - This console warning still appear after the move to webgl2

WebGL warning: texImage: Alpha-premult and y-flip are deprecated for non-DOM-Element uploads.

HarelM commented 11 months ago

There is still an open PR which was not merged unfortunately. If this is something that interests you feel free to push it forward. Backporting stuff is very time consuming, and we are focusing on latest and greatest main. If someone from the community is interested in spending the time to backport something I'll be the last one to object. :-)

WebMechanic commented 9 months ago

We eventually upgraded to v3 which went very smooth! 👍🏻

However, I still get these warnings with v3.6.2 (current as of this writing) and the amount of warnings per identical request appears to be pretty random too.

Thanks.

garma83 commented 7 months ago

confirming that this is not fixed in 3.0.1. Im a bit surprised this is left unfixed. Imho map box should not throw warnings when a user uses it correctly and as documented.

HarelM commented 7 months ago

@garma83 the PR is still open and you are welcome to push it forward if this is something that is truly important to you.

garma83 commented 7 months ago

@garma83 the PR is still open and you are welcome to push it forward if this is something that is truly important to you.

TBH an answer like that is not very helpful IMHO. I have no idea about the code base, nor would it be feasible for me to dedicate time during work hours. Im just flagging it here because it was said that it would be fixed in 3.0

sbachinin commented 7 months ago

Tried to fix these warnings but failed. I'll share some insights, in case someone tries to solve this in the future.

1) I wouldn't recommend to proceed with PR #2297 because it's only a partial fix and it's kind of misleading because empty atlases aren't the real cause of warnings. I tried this solution on several example pages. Warnings disappeared on some pages, simple-map for example. On many other pages console was still full of warnings. Apparently the problem is not only with empty atlases but with any atlases.

2) The actual problem is experssed in the warning itself and I think the only way to fix the problem is to do what Firefox tells us to do. Currenly FF is unhappy because, when updating a texture, we call:

gl.pixelStorei(gl.UNPACK_PREMULTIPLY_ALPHA_WEBGL, true);

and then provide a non-DOM-Element source to gl.texImage2D. Firefox assumes that non-DOM-Element image data should be premultiplied manually by a consuming program instead of delegating it to webgl. FF states that they "do not optimize this path". That is, doing it manually in theory mustn't lead to performance degradation in FF. Premult algorithm is pretty simple and fast. And it won't be called very often anyway.

So basically a proper solution must be something like (pseudocode):

if (firefox && !isDOMElement(imageData)) {
    imageData = getPremultiplied(imageData); // premultiply manually
} else {
        // current behaviour: delegate premult to webgl
    gl.pixelStorei(gl.UNPACK_PREMULTIPLY_ALPHA_WEBGL, true);
}

Problems with this approach:

a) doing special stuff for Firefox alone is problematic from testing pov. In such case we need a dedicated render tests for FF.

b) real implementation turns out quite messy. It's mostly because there are different kinds of non-DOM-Element data. It's easy to premultiply "plain data" (an array of numbers). But then there is an interesting case of ImageBitmaps: sometimes ImageBitmaps cause warnings and sometimes they don't. And premultiplication of ImageBitmaps is a different algorithm. Everything is doable but that will be too much code.

HarelM commented 7 months ago

Thanks for looking into this @sbachinin! What other alternatives do we have? Can we change the gl code somehow to use an optimized path in FF using a different API maybe?

sbachinin commented 7 months ago

At the moment I have no other ideas. Asked them to issue this warning only once. https://bugzilla.mozilla.org/show_bug.cgi?id=1889315 (because the problem is the amount of warnings first of all, afaics)