Open Bug 1474281 Opened Last year Updated 2 months ago

Make EGL-provider support OGL (additional to GLES)

Categories

(Core :: Canvas: WebGL, enhancement, P3)

Unspecified
Linux
enhancement

Tracking

()

People

(Reporter: robert.mader, Assigned: robert.mader)

References

(Blocks 2 open bugs)

Details

(Whiteboard: [gfx-noted])

Attachments

(2 files, 6 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.
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: → 788319

(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: 788319
See Also: 788319
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

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
You need to log in before you can comment on or make changes to this bug.