Closed Bug 1474281 Opened 6 years ago Closed 4 years ago

Make EGL-provider support OGL (additional to GLES)

Categories

(Core :: Graphics: CanvasWebGL, enhancement, P3)

Unspecified
Linux
enhancement

Tracking

()

RESOLVED FIXED
mozilla77
Tracking Status
firefox77 --- fixed

People

(Reporter: rmader, Assigned: rmader)

References

(Blocks 2 open bugs)

Details

(Whiteboard: [gfx-noted])

Attachments

(2 files, 10 obsolete files)

User Agent: Mozilla/5.0 (X11; Fedora; Linux x86_64; rv:61.0) Gecko/20100101 Firefox/61.0
Build ID: 20180625102006

Steps to reproduce:

Start the flatpak nightly on wayland, go to about:support

System:
Fedora 28, Gnome 3.28 - Wayland , Mesa 18.0.5, Intel Ivy-Bridge


Actual results:

 - WebGL 1/2 Driver Version report OpenGL ES (3.0)
 - slower performance in webGL-demos than with desktop OpenGL


Expected results:

 - reported driver version should be desktop OpenGL versions (3.0 or core 4.2)
 - similar performance in webGL-demos

This issue has been raised in several places from time to time, it might be worth looking into it for good firefox wayland support.

See:
https://bugzilla.mozilla.org/show_bug.cgi?id=788319 comments 50-54
Component: General → Canvas: WebGL
I'm not a hundred percent sure, but I think the OpenGL version is not only about WebGL but also about accelerated layers and webrender, making this whole topic much more important if we ever want them to be enabled on linux by default.
I' forgot to add: this is not only true for the flatpak version but also for the fedora firefox-wayland package. AFAIK it's always been like this with the EGL backend.
Are there any benchmarks? I get good performance with WebGL EGL OpenGL ES.
I just tried to make some benchmarks and can confirm that I couldn't find any performance differences for WebGL.
That said, I'm actually more concerned about the Gecko OpenGL render backend and Webrender, as they are not limited to OpenGL ES as WebGL is.

He are some numbers for the CanvasMark 2013 benchmark, although I'm not sure if that's the best benchmark for 2D performance (any suggestions?). Unfortunately, firefox 61 wayland on fedora 28 didn't work with layers acceleration forced on (Failed to create EGLSurface!), while it worked well in the flatpak nighly. But that would have been exactly the numbers we'd be interested in.


Therefore some incomplete numbers already just for reference:

stable 61 x11:
opengl: 11374
basic:  11560

stable 61 wayland:
basic:  11576

nighly 63 flatpak - wayland:
webrender: 6501
opengl:    6938
basic:     7377


Concerning the bad numbers in nighly, I suppose there're some debug options set.
I forgot: that said I'd like to remove my claims from the beginning of bad WebGL performance. That was seemingly just a wrong impression. So the point is purely about having desktop OpenGL for the render backends.
Wow! This will be quite the coup if Mozilla releases native support for Wayland before Chromium.
Whiteboard: [gfx-noted]
Priority: -- → P3
Attached patch use_desktop_gl_in_egl.patch (obsolete) — Splinter Review
This makes the egl-backend create desktop-gl contexts on non-android plattforms.
I successfully tested several WebGL examples, accelerated layers and webrender under wayland/egl.

I guess the most important remaining question is on which plattform we want desktop GL by default and on which ones GLES
:stransky , this one fixes bug 1500415 for me, could you have a look on it?
In order to make this save to land, can someone from the gfx-team clarify where we need GLES support?

Three questions come to my mind:
 - apart from android, are there platforms we know which need GLES?
 - should we have a fallback? Check first for desktop GL, then try GLES?
 - should we maybe just set a compile flag which controls this?
Flags: needinfo?(jnicol)
Summary: EGL forces OpenGL ES → Make EGL-provider support OGL (additional to GLES)
Flags: needinfo?(jgilbert)
jnicol said on IRC that jgilbert is the right person to talk to
Flags: needinfo?(jnicol)
(In reply to robert.mader from comment #9)
> In order to make this save to land, can someone from the gfx-team clarify
> where we need GLES support?
> 
> Three questions come to my mind:
>  - apart from android, are there platforms we know which need GLES?

Not really no. 

>  - should we have a fallback? Check first for desktop GL, then try GLES?

It's probably ok to not.

>  - should we maybe just set a compile flag which controls this?

That seems ok to me.

It's worth checking these answers with jgilbert though.
Applied the use_desktop_gl_in_egl patch, now WebRender doesn't start (and WebGL1 failed too? because of it?)

(gfx.webrender.all=true, Weston master, Mesa master ~1 month old, Radeon RX 480, FreeBSD -CURRENT)

Compositing:	OpenGL

WebGL 1 Driver Renderer:	WebGL creation failed: 
* tryNativeGL
* Exhausted GL driver options.

(#0) Error	Failed to compile shader: ps_text_run 0:21(12): error: extension `GL_ARB_explicit_attrib_location' unsupported in vertex shader
(#1) Error	wr_renderer_render: Shader(Compilation("ps_text_run", "0:21(12): error: extension `GL_ARB_explicit_attrib_location\' unsupported in vertex shader\n"))
(#2) Error	Compositors might be mixed (5,2)


GL_ARB_explicit_attrib_location *is* present in "WebGL 2 Driver Extensions" though.
X11/GLX backend works perfectly (on the same Firefox build) with WebRender and WebGL and everything.

The WebGL 1 issue doesn't seem WebRender related (same result with gfx.webrender.all=false).

WebRender and WebGL 1 worked fine on GLES (until the "everything is squares" WebRender bug appeared).
Just rebased to central tip and see the very same issue (no WebGL, Webrender not starting). Will look into it
(In reply to Jeff Muizelaar [:jrmuizel] from comment #11)
> (In reply to robert.mader from comment #9)
> > In order to make this save to land, can someone from the gfx-team clarify
> > where we need GLES support?
> > 
> > Three questions come to my mind:
> >  - apart from android, are there platforms we know which need GLES?

Android and ANGLE.

> >  - should we have a fallback? Check first for desktop GL, then try GLES?

We *must* fallback to ES.
Flags: needinfo?(jgilbert)
Thanks for the clarification @jrmuizel and @jgilbert!

@grep: I think there are two bugs here. First of all we seem to hit bug 1489902, but that should also happen without this patch. Secondly, something has happened to the WebGL 1 path...or maybe it is even the same.

I'll wait for stransky to investigate that bug again and try to change this patch to have a proper fallback in the meantime.
Another rebase and looks like the issue is gone (with the desktop GL patch at least). WebGL works, even WebRender works under Wayland again.

Except there's a new (?) issue: with WebRender, I can only have one browser window working. Windows other than the first are just transparent, only the gtk shadow is visible, no content.
Attached patch patch rebased to latest trunk (obsolete) — Splinter Review
Rebased the patch to latest trunk but WebRender does not work for me with it.
Attached patch use_desktop_gl_in_egl.patch (obsolete) — Splinter Review

Rebased to tip and made some small changes now the context creation matches that of the GLX provider in all three cases: basic, opengl and webrender (works now for webgl 1 in all cases, which previously didn't)

Unfortunately with webrender there's a blue/red swap which I have to figure out. Otherwise it seems to work reasonably well. It also seems to work around bug 1492774, although we should fix that one for GLES.

Attachment #9024314 - Attachment is obsolete: true
Type: defect → enhancement
OS: Unspecified → Linux
Version: 63 Branch → Trunk
See Also: → linux-egl

(In reply to robert.mader from comment #19)

Created attachment 9075220 [details] [diff] [review]
use_desktop_gl_in_egl.patch

Rebased to tip

This does not differentiate between Android (GLES) and desktop (LOCAL_EGL_OPENGL_BIT) like comment 18 does. Also, current master looks different.

(In reply to Jan Andre Ikenmeyer [:darkspirit] from comment #20)

(In reply to robert.mader from comment #19)

Created attachment 9075220 [details] [diff] [review]
use_desktop_gl_in_egl.patch

Rebased to tip

This does not differentiate between Android (GLES) and desktop (LOCAL_EGL_OPENGL_BIT) like comment 18 does. Also, current master looks different.

Yes I left that out again as it currently has no visible effect on my mashine and the patch does not yet implement the fallback to GLES as required in comment 15. Sorry, I should maybe outline this is WIP.
As soon as I have webrender working correctly, I'll try to add a reasonable fallback code, which will also take care of that bits.
As of current master: well it's gecko-dev master from today :/

@greg: just tested multiple windows, works here

FTR: I think I found the issue around the colors: GLES has GL_EXT_texture_format_BGRA8888, but at least on my intel card I don't get the equivalent GL_EXT_bgra in a core context (but in a compatibility context, which again doesn't work with webrender).
So around the lines in https://searchfox.org/mozilla-central/rev/8a990595ce6d5ed07ace2d4d5d86cc69aec90bde/gfx/wr/webrender/src/device/gl.rs#1246 webrender falls back to RGBA now, which matches exactly the observed behaviour. Even more, WebGL 1, which uses the compatibility profile, does not have the issue, while WebGL 2, which uses the core profile, does.

I suspect in the GLX-backend that is handled somewhere, but the Wayland code probably expects BRGA. Now I have to figure out what needs fixing here :) Maybe even mesa not exposing GL_EXT_bgra in core contexts? Apparently NVIDIA does.

Attached patch force-wr-to-bgra.patch (obsolete) — Splinter Review

Turns out that Webrender fails to properly detect the context as GL instead of GLES. If we force the GL behaviour (which should match what happens with GLX), colors are right again. So here's a little hack until for that until a proper fix lands in WR.

Attached patch use_desktop_gl_in_egl.patch (obsolete) — Splinter Review

Patch was in the wrong direction :)

Attachment #9075220 - Attachment is obsolete: true
Attached patch force-wr-to-bgra.patch (obsolete) — Splinter Review

This one, too

Attachment #9075260 - Attachment is obsolete: true

(In reply to Jan Andre Ikenmeyer [:darkspirit] from comment #27)

These lines seem to assume EGL means GLES:
https://searchfox.org/mozilla-central/rev/8a990595ce6d5ed07ace2d4d5d86cc69aec90bde/gfx/webrender_bindings/src/bindings.rs#1113
https://searchfox.org/mozilla-central/rev/8a990595ce6d5ed07ace2d4d5d86cc69aec90bde/gfx/webrender_bindings/src/bindings.rs#1178

Thanks for that!

So to sum up what we need to do for this:

In order to just work

  • fix WR context detection, so GL on EGL is properly detected and GL on EGL takes the same path as GL on GLX

For best performance, which I'd assume does not involve any texture swizzling therefore would also benefit the GLX path:

  • make WR use GL_EXT_bgra additionally to GL_EXT_texture_format_BGRA8888, which is GLES only
  • make mesa export GL_EXT_bgra in the core profile

Is that right?

Hi Robert,

(In reply to robert.mader from comment #28)

In order to just work

  • fix WR context detection, so GL on EGL is properly detected and GL on EGL takes the same path as GL on GLX

Yes, this sounds like it will fix the red and blue inversion. I should probably have added a warning to that fallback code so it was immediately obvious why the inversion was happening.

For best performance, which I'd assume does not involve any texture swizzling therefore would also benefit the GLX path:

  • make WR use GL_EXT_bgra additionally to GL_EXT_texture_format_BGRA8888, which is GLES only
  • make mesa export GL_EXT_bgra in the core profile

Is that right?

I don't think this is required, nor correct. GL_EXT_texture_format_BGRA8888 is about allowing BGRA as an internal format. Whereas GL_EXT_BGRA allows BGRA as the external format (with automatic swizzling if required). EXT_BGRA is an ancient extension, which is included in GL for a long time, and we currently rely on that behaviour. As for performance, the driver is hopefully smart enough to use BGRA for the internal format too, but if it is not I don't believe there is any way for us to ask it to.

So just fixing the GL/GLES detection should be enough.

I filed bug 1562817 about switching away from BGRA, since it's an ongoing source of pain.

See Also: → 1562817

(In reply to Jan Andre Ikenmeyer [:darkspirit] from comment #27)

These lines seem to assume EGL means GLES:
https://searchfox.org/mozilla-central/rev/8a990595ce6d5ed07ace2d4d5d86cc69aec90bde/gfx/webrender_bindings/src/bindings.rs#1113
https://searchfox.org/mozilla-central/rev/8a990595ce6d5ed07ace2d4d5d86cc69aec90bde/gfx/webrender_bindings/src/bindings.rs#1178

Should they be changed to one of these?

if cfg!(target_os = "android") || unsafe { is_glcontext_angle(gl_context) } {
    gl = unsafe { gl::GlesFns::load_with(|symbol| get_proc_address(gl_context, symbol)) };
} else {
    gl = unsafe { gl::GlFns::load_with(|symbol| get_proc_address(gl_context, symbol)) };
}
#[cfg(target_os = "android")]
gl = unsafe { gl::GlesFns::load_with(|symbol| get_proc_address(gl_context, symbol)) };

#[cfg(not(target_os = "android"))]
if unsafe { is_glcontext_angle(gl_context) } {
    gl = unsafe { gl::GlesFns::load_with(|symbol| get_proc_address(gl_context, symbol)) };
} else {
    gl = unsafe { gl::GlFns::load_with(|symbol| get_proc_address(gl_context, symbol)) };
}

(In reply to Jan Andre Ikenmeyer [:darkspirit] from comment #31)

Well if we want the EGL backend to be able to use both, GLES and GL, and choose between at runtime (try GL, fall back to GLES for example), it would be nicer if WR would properly detect what kind of context it gets. At runtime.

Blocks: linux-egl
See Also: linux-egl
See Also: → 1548339
Depends on: 1563035
Attached patch use_desktop_gl_in_egl.patch (obsolete) — Splinter Review

Rebased on bug 1347442, which implements proper gl type detection. It still chooses the type during compile time, though, so WIP

Attachment #9075264 - Attachment is obsolete: true
Attachment #9075265 - Attachment is obsolete: true

Ah I meant bug 1563035 of course!

Status: UNCONFIRMED → NEW
Ever confirmed: true

I tinkered a bit with this but before going forward I like again to hear an opinion how to implement this best. I see the following options how to choose which GL type to use:

  1. Runtime fallback: where supported, first try GL, then GLES. In comment #15 :jgilbert says we must do that, but it's definitely the hardest and most error prone option. Requires quite a bit of restructuring if I get things right.
  2. At compile time
  3. Via an environment variable

I'd personally prefer a mixture of 2 and 3, because the GL variant targets only Wayland and X11 clients, both cases where can reasonably assume GL support to be present. The only reason for me to go for option 1 would be if we need to support the case where clients only support GLES and we don't know beforehand / at compile time. Is there such a scenario?

To make it clear, I'd like to:

  • have a build variable which controls if GL support should be added to EGL, something like MOZ_EGL_GL. If not set, always use GLES
  • if compiled with MOZ_EGL_GL, have an env variable like MOZ_EGL_USE_GLES which makes the EGL provider use GLES, otherwise default to GL

The main reason for the env variable would be to make it easy to compare the two backends, e.g. compare performance or work around bugs like bug 1492774

What do you think?

Any time we can get Desktop GL, that's what we want. (though it's nice to have a pref for experimentation)

Requiring a re-compile isn't something we want.
Env vars are really very similar to a pref, but prefs are documented and env-vars are more error-prone.

Do not make this a build-variable. It's such a pain to tie things tightly to our build system.

(In reply to Jeff Gilbert [:jgilbert] from comment #36)

Any time we can get Desktop GL, that's what we want. (though it's nice to have a pref for experimentation)

Requiring a re-compile isn't something we want.
Env vars are really very similar to a pref, but prefs are documented and env-vars are more error-prone.

Do not make this a build-variable. It's such a pain to tie things tightly to our build system.

Ok, but that does not yet answer the question about option 1 completely. According to https://wiki.mozilla.org/Platform/GFX/WebGL/Backends (which might be a little out-dated), on Windows we just fail if loading ANGLE fails, instead of falling back to native GL. Would the same be ok here, just trying GL and not falling back to GLES? AFAIK FF currently only supports GLX/GL on Linux, so we wouldn't regress.
I know it sounds a bit lazy, but things would be really much easier that way :)

I own this code and I'm saying I'd prefer to load desktop GL, and fall back to GLES.

(In reply to Jeff Gilbert [:jgilbert] from comment #38)

I own this code and I'm saying I'd prefer to load desktop GL, and fall back to GLES.

Alright, just wanted to be sure. Will try to figure it out.

Awesome, thanks!

Assignee: nobody → robert.mader
Attached patch use_desktop_gl_in_egl.patch (obsolete) — Splinter Review

First draft of a solution with fallback for interested parties. Now I'm go figure out how to use phabricator for a proper review etc.

Attachment #9075514 - Attachment is obsolete: true

In the past EGL only supported GLES, not OGL. This has not been true
for a very long time, so lets support OGL context creation in the EGL
backend.

This allows e.g. the Wayland backend to use OGL contexts, which brings
it on par with the X11/GLX backend.

Attachment #9078277 - Attachment is obsolete: true
Attachment #9098605 - Attachment is obsolete: true

Ok, finally got around to setup moz-phab and friends. Turned out to be so much more straight forward than I thought all the time when looking at text walls. Maybe I should write a blog post, 'how to setup moz-phab with git - for stupid people with low frustration tolerance' or so.

Jokes aside, this is still the same version as I posted earlier. It has a 'MOZ_PREFER_GLES' env variable which allows easy comparison of GL vs GLES with the Wayland backend. Any feedback is welcome.

Attachment #9098605 - Attachment description: Bug 1474281 - Make EGL-provider support OGL → Bug 1474281 - Make EGL-provider support OGL. r=jgilbert
Attachment #9098605 - Attachment is obsolete: false
Attachment #9098627 - Attachment description: Bug 1474281 - Remove unneded debug print. r=jgilbert → Bug 1474281 - Remove unneeded debug print. r=jgilbert

I tried the patch while I was investigating shader Bug 1583731 and found out that the desktop GL does not provide GL_OES_EGL_image_external_essl3 extension. Not sure if EGL_image_external (provided by desktop GL) is enough here.

:stransky was it necessary to use GL_OES_EGL_image_external_essl3 for bug 1583731 ? Or should it work with OGL, too? :)

GL_OES_EGL_image_external_essl3 is recently needed for WebRender to work with dmabuf backend unless shaders are updated to avoid textureSize calls...so I'm fine with the patch if the desktop GL / GL ES can be switched run-time for webrender.

Ah you're referring to https://github.com/servo/webrender/issues/3310#issuecomment-452433759, right? To use GL_OES_EGL_image_external and avoid calls to textureSize? I'll have a look if I can get that to work.

Actually mesa exposes GL_OES_EGL_image in the core profile, so there's a good chance that it also works for GL_OES_EGL_image_external and GL_OES_EGL_image_external_essl3. Doing that would also help other projects like skia, which also relies on GL_OES_EGL_image_external_essl3 (https://chromium.googlesource.com/skia/+/chrome/m49/src/gpu/gl/GrGLCaps.cpp#219)

Quote from https://gitlab.freedesktop.org/mesa/mesa/issues/2119:

TEXTURE_EXTERNAL was introduced (in ES1.1 days) for GPUs which had strict limitations when external or YUV buffers were used: it is TEXTURE_2D with far more limitations. As time has moved on, these limitations have become unnecessary, and TEXTURE_EXTERNAL is more or less a relic of the past.

I see, TEXTURE_2D can be used in Firefox then, there's no need of TEXTURE_EXTERNAL there.

Depends on: 1599016

This needs to be updated to work with webrender after bug 1556301

This patch works for me now as WebRender/GL compositor does not use TEXTURE_EXTERNAL any more. With this patch the GL backend on Wayland works better than GLES for me, for instance I can debug gl frames by apitrace. Jeff, can we move here?
Thanks.

Flags: needinfo?(jgilbert)
Attachment #9098607 - Attachment is obsolete: true

I have to say that I'm not super happy with structure of the patch at the moment. So if you have some suggestions how to better split up the code that would be greatly appreciated. I've tried it already but didn't come up with a great solution thus far :/

Flags: needinfo?(jgilbert)
Keywords: leave-open
Pushed by apavel@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/76ee1827c820
Remove unneeded debug print. r=jgilbert

Martin, as I don't have commit access yet, could you do a try push for the revision? It looks like this version is so small and simple now that we could land it (IMHO).

Flags: needinfo?(stransky)

(In reply to robert.mader from comment #57)

Martin, as I don't have commit access yet

Can you please apply for it? I can vouch you. How-to is here:

https://www.mozilla.org/en-US/about/governance/policies/commit/
https://www.mozilla.org/en-US/about/governance/policies/commit/access-policy/

could you do a try push for the revision? It looks like this version is so small and simple now that we could land it (IMHO).

Try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=c5fa4a9995ff6a4b1811328cf6f9ca31927e5453

Thanks.

Flags: needinfo?(stransky)

(In reply to Martin Stránský [:stransky] from comment #58)

(In reply to robert.mader from comment #57)

Martin, as I don't have commit access yet

Can you please apply for it? I can vouch you. How-to is here:

Will do!

Try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=c5fa4a9995ff6a4b1811328cf6f9ca31927e5453

If I interpret the result right this is as expected - no effect, as none of the tests runs on Wayland (AFAIK). But as the EGL provider is used on Android and Windows (angle), too, we should include those platforms (that's where I suspect breakage). I can do so as soon as I get commit permissions.

This workaround was added more than eight years ago and likely has
been fixed by now. More importantly, there is still hardware around
that supports OGL 1.4 / GLES 2.0 (intel gen3). With GLES fallback in
place, these now support WebGL 1 and OpenGL compositor.

Tested with MESA_GL_VERSION_OVERRIDE=1.4 MESA_GLES_VERSION_OVERRIDE=2.0

Depends on D48096

Keywords: leave-open

Jeff, mind another round of review? Should be quick (as there's not much code) and it would be great if we carried on to bug 788319

Flags: needinfo?(jgilbert)
Attachment #9132925 - Attachment is obsolete: true
Flags: needinfo?(jgilbert)
Attachment #9098605 - Attachment description: Bug 1474281 - Make EGL-provider support OGL. r=jgilbert → Bug 1474281 - Make EGL-provider support OGL. r?jgilbert
Attachment #9030232 - Attachment is obsolete: true

Updated, Thanks. When I use this patch on Wayland, apitrace works for me.

Pushed by nbeleuzu@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/2a8c06104c40
Make EGL-provider support OGL. r=jgilbert
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla77
Regressions: 1632095
Regressions: 1632137
Regressions: 1633273
Regressions: 1634268
No longer regressions: 1634268
Regressions: 1647172
See Also: → 1865601
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: