Consider using EGL instead of GLX with Mesa drivers

NEW
Unassigned

Status

()

defect
7 years ago
2 hours ago

People

(Reporter: marco, Unassigned)

Tracking

(Blocks 4 bugs)

18 Branch
All
Linux
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [leave open])

Attachments

(3 attachments, 2 obsolete attachments)

(Reporter)

Description

7 years ago
From http://www.phoronix.com/scan.php?page=news_item&px=MTE3MTI:

---
Also mentioned by Eric is "GLX is pretty much deprecated at this point." Intel is encouraging developers to use EGL rather than GLX. EGL is needed anyways for Wayland, Android, OS X, etc and the Mesa EGL support has matured quite well and still works fine with an X.Org Server. Eric said, "Please please please stop using GLX. There shouldn't be anything left that GLX has that EGL doesn't."
---
One useful piece of information here would be which version of Mesa has sufficiently "mature" EGL support.  Currently we require Mesa 7.10.3 (but 8.0 with nouveau).
(Reporter)

Updated

7 years ago
Blocks: wayland
This would also be an instant win for WebGL, as it runs better (faster, using less memory) on top of GLES than desktop GL.

Comment 4

7 years ago
For Intel Sandybridge and Ivybridge GPUs, we submitted OpenGL ES 2.0 conformance test results on Mesa 8.0.4.  I believe that the tests ran well on 7.11.something on Sandybridge, but we never submitted those results.  I assume Firefox doesn't do anything exotic with EGL, so it should be fine.  Most of the important infrastructure is shared with GLX.
Sounds like we definitely want to move to EGL for the new OpenGL library ABI if/when that happens.

The NVIDIA GeForce driver might be the last thing holding us back before then.
Moving to EGL opens up the ability to use EGLImage to share textures instead of using context sharing, which has the potential to improve performance slightly.
Well, I should say, we could use EGLImage where available. I don't know if Mesa supports or has any plan to support it, but it would be nice to have.

Comment 8

7 years ago
Mesa already supports it, and, I believe, Wayland uses it.  You may encounter issues, and we look forward to your bug reports. :)
Blocks: 822518
This would also be of great important to Raspberry Pi, as their drivers only support EGL and not GLX on X11.
Blocks: 835926
Posted patch initial wip (obsolete) — Splinter Review
I've had a go at looking into this the last couple of evenings, I didn't get very far. With the patch, you can add the following to your mozconfig:

ac_add_options --with-gl-provider=EGL
ac_add_options --enable-default-toolkit=cairo-gtk2-egl

(actually, you probably only need the second line) and Firefox will build and run without crashing. (I also had to build my own version of Mesa, but not sure if there might have been an easier way). But all we get rendered is a black screen - no warnings, no assertions.

I had assumed we'd need to do some work to get gtk and egl to play nice, but I was kind of hoping I would get some hints rather than just a black screen.

Anyone got any ideas?
Suggestion: run a debug build with MOZ_GL_DEBUG_ABORT_ON_ERROR=1 to see if you get GL errors, or use APItrace.
(In reply to Benoit Jacob [:bjacob] from comment #12)
> Suggestion: run a debug build with MOZ_GL_DEBUG_ABORT_ON_ERROR=1 to see if
> you get GL errors, or use APItrace.

So this gave me nothing.

I also tried disabling xrender, but that did nothing too.

AND THEN, for reasons I completely and utterly fail to understand, it spontaneously started working on me. As far as I can tell it is using EGL and doing all it should do without any errors. Which, I'm suspicious. I'll do some more testing. (btw I re-enabled xrender and turned off GL_DEBUG and it still worked, then I rebooted and it still worked).
A little more testing and it mostly works. OMTC is totally borked - just a big black screen, but main thread is OK. WebGL doesn't seem to work. And when using an intermediate surface for container layers, it renders OK, but I get some kind of GL abort when using MOZ_GL_DEBUG_ABORT_ON_ERROR. So, a few things to fix up, but at least it is not a mysterious blank screen.
Turns out the intermediate surface thing is nothing to do with EGL. It is fixed in bug 863968.
I got webgl working, but only with readback. I'll look at OMTC next, but not tonight. Patches coming soon-ish.

jgilbert - I seem to be having problems with streaming buffers stuff, but this stuff must work on Android right? Does anything come to mind where there are assumptions that EGL => Android or OMTC? Sorry this is a bit vague, when I get some more time, I'll figure out a real question :-p
Flags: needinfo?(jgilbert)
You may want to take a quick look at bug 863397. In particular, I'm removing the eglLockSurface-based optimizations there because they belong in dedicated TextureClient/TextureHosts rather than in GLContextProviderEGL.cpp. I don't know if eglLockSurface is available in Mesa.
Comment on attachment 738390 [details] [diff] [review]
initial wip


>     mNeedsYFlip = false;
>-#if defined(MOZ_X11) && !defined(MOZ_PLATFORM_MAEMO)
>+#if defined(GL_PROVIDER_GLX) && !defined(MOZ_PLATFORM_MAEMO)
>     if (aData.mSurface->GetType() == gfxASurface::SurfaceTypeXlib) {
>         gfxXlibSurface *xsurf = static_cast<gfxXlibSurface*>(aData.mSurface);

keep it as just 
+#if defined(GL_PROVIDER_GLX)
Working for OMTC too! Turns out we should not tell CompositorParent we are using EGL unless we are on Android and therefore intend to send surface size info.
(In reply to Benoit Jacob [:bjacob] from comment #17)
> You may want to take a quick look at bug 863397. In particular, I'm removing
> the eglLockSurface-based optimizations there because they belong in
> dedicated TextureClient/TextureHosts rather than in
> GLContextProviderEGL.cpp. I don't know if eglLockSurface is available in
> Mesa.

Thanks for the heads up! I think we can get something working without any of that. I guess if we want to pursue using EGL more in the future we can put it back in as a texture client/host.
Posted patch basic fixupsSplinter Review
This version just requires |ac_add_options --with-gl-provider=EGL| in your mozconfig.
Attachment #738390 - Attachment is obsolete: true
Attachment #740058 - Flags: review?(bjacob)
Posted patch WIP webgl (obsolete) — Splinter Review
WIP, in case anyone wants to test this stuff. Just using forced readback for now.
Posted patch WIP webglSplinter Review
Attachment #740059 - Attachment is obsolete: true
(In reply to Nick Cameron [:nrc] from comment #19)
> Working for OMTC too! Turns out we should not tell CompositorParent we are
> using EGL unless we are on Android and therefore intend to send surface size
> info.

Sounds like we should fix that. None of the streaming stuff should assume that EGL is android.
Flags: needinfo?(jgilbert)
Attachment #740058 - Flags: review?(bjacob) → review+
Comment on attachment 740058 [details] [diff] [review]
basic fixups

Review of attachment 740058 [details] [diff] [review]:
-----------------------------------------------------------------

::: widget/xpwidgets/nsBaseWidget.cpp
@@ +876,5 @@
>                                                      int aSurfaceHeight)
>  {
> +  // Even if we are using EGL, unless we are on Android, we don't want
> +  // CompositorParent to think we are, so we pass aRenderToEGLSurface = false.
> +  return new CompositorParent(this, false, aSurfaceWidth, aSurfaceHeight);

I would like to know how hard this is to fix, since this just screams 'hack'.
Attachment #740058 - Flags: feedback?(ncameron)
(In reply to Jeff Gilbert [:jgilbert] from comment #25)
> Comment on attachment 740058 [details] [diff] [review]
> basic fixups
> 
> Review of attachment 740058 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: widget/xpwidgets/nsBaseWidget.cpp
> @@ +876,5 @@
> >                                                      int aSurfaceHeight)
> >  {
> > +  // Even if we are using EGL, unless we are on Android, we don't want
> > +  // CompositorParent to think we are, so we pass aRenderToEGLSurface = false.
> > +  return new CompositorParent(this, false, aSurfaceWidth, aSurfaceHeight);
> 
> I would like to know how hard this is to fix, since this just screams 'hack'.

Well, I think it is just that aRenderToEGLSurface really means aExternallyProvidedSurfaceSize or something, so the fix is just to rename the argument and corresponding member. I don't think that this flag is used for anything actually related to EGL.
(In reply to Nick Cameron [:nrc] from comment #26)
> (In reply to Jeff Gilbert [:jgilbert] from comment #25)
> > Comment on attachment 740058 [details] [diff] [review]
> > basic fixups
> > 
> > Review of attachment 740058 [details] [diff] [review]:
> > -----------------------------------------------------------------
> > 
> > ::: widget/xpwidgets/nsBaseWidget.cpp
> > @@ +876,5 @@
> > >                                                      int aSurfaceHeight)
> > >  {
> > > +  // Even if we are using EGL, unless we are on Android, we don't want
> > > +  // CompositorParent to think we are, so we pass aRenderToEGLSurface = false.
> > > +  return new CompositorParent(this, false, aSurfaceWidth, aSurfaceHeight);
> > 
> > I would like to know how hard this is to fix, since this just screams 'hack'.
> 
> Well, I think it is just that aRenderToEGLSurface really means
> aExternallyProvidedSurfaceSize or something, so the fix is just to rename
> the argument and corresponding member. I don't think that this flag is used
> for anything actually related to EGL.

Can we please-please-please rename it then? I would much rather prefer that to the `// Hack` comment we have there right now.

Updated

6 years ago
Blocks: 865019
(In reply to Jeff Gilbert [:jgilbert] from comment #27)
> (In reply to Nick Cameron [:nrc] from comment #26)
> > (In reply to Jeff Gilbert [:jgilbert] from comment #25)
> > > Comment on attachment 740058 [details] [diff] [review]
> > > basic fixups
> > > 
> > > Review of attachment 740058 [details] [diff] [review]:
> > > -----------------------------------------------------------------
> > > 
> > > ::: widget/xpwidgets/nsBaseWidget.cpp
> > > @@ +876,5 @@
> > > >                                                      int aSurfaceHeight)
> > > >  {
> > > > +  // Even if we are using EGL, unless we are on Android, we don't want
> > > > +  // CompositorParent to think we are, so we pass aRenderToEGLSurface = false.
> > > > +  return new CompositorParent(this, false, aSurfaceWidth, aSurfaceHeight);
> > > 
> > > I would like to know how hard this is to fix, since this just screams 'hack'.
> > 
> > Well, I think it is just that aRenderToEGLSurface really means
> > aExternallyProvidedSurfaceSize or something, so the fix is just to rename
> > the argument and corresponding member. I don't think that this flag is used
> > for anything actually related to EGL.
> 
> Can we please-please-please rename it then? I would much rather prefer that
> to the `// Hack` comment we have there right now.

We certainly can, I'll make a separate patch

Updated

6 years ago
Attachment #740058 - Flags: feedback?(ncameron)
Attachment #742876 - Flags: review?(bugmail.mozilla)
Comment on attachment 742876 [details] [diff] [review]
rename RenderToEGLSurface

Review of attachment 742876 [details] [diff] [review]:
-----------------------------------------------------------------

LGTM with the one fix below.

::: gfx/layers/ipc/CompositorParent.h
@@ +43,5 @@
>    NS_INLINE_DECL_THREADSAFE_REFCOUNTING(CompositorParent)
>  
>  public:
>    CompositorParent(nsIWidget* aWidget,
> +                   bool s = false,

s/s/aUseExternalSurfaceSize/
Attachment #742876 - Flags: review?(bugmail.mozilla) → review+
So, as far as I can tell, everything except WebGL works with EGL now. We should fix WebGL, and then leave this bug open to discuss if we want to support EGL in any capacity.

Comment 34

6 years ago
(In reply to Benoit Jacob [:bjacob] from comment #17)
> You may want to take a quick look at bug 863397. In particular, I'm removing
> the eglLockSurface-based optimizations there because they belong in
> dedicated TextureClient/TextureHosts rather than in
> GLContextProviderEGL.cpp. I don't know if eglLockSurface is available in
> Mesa.

It is not.  Since the spec requires the surface not be bound to a context, it will be difficult to impossible for us to implement this extension in general.
Depends on: 868682

Comment 35

6 years ago
(In reply to Karl Tomlinson (:karlt) from comment #5)
> Sounds like we definitely want to move to EGL for the new OpenGL library ABI
> if/when that happens.
> 
> The NVIDIA GeForce driver might be the last thing holding us back before
> then.

NVIDIA Driver Soon Likely To Support EGL, Mir http://www.phoronix.com/scan.php?page=news_item&px=MTM5MjE
(Reporter)

Updated

6 years ago
Attachment #742876 - Flags: checkin+
(Reporter)

Updated

6 years ago
Attachment #740058 - Flags: checkin+

Comment 36

5 years ago
"NVIDIA 334.16 Beta Supports 64-bit EGL / OpenGL ES"

http://www.phoronix.com/scan.php?page=news_item&px=MTU5NjI

Comment 37

4 years ago
i'm sorta confused as to what is happening with this issue. Will EGL be used? If so, when?
(In reply to Johnny Robeson from comment #37)
> i'm sorta confused as to what is happening with this issue. Will EGL be
> used? If so, when?

EGL will not be used by default.

Comment 39

4 years ago
(In reply to Jeff Gilbert [:jgilbert] from comment #38)
> (In reply to Johnny Robeson from comment #37)
> > i'm sorta confused as to what is happening with this issue. Will EGL be
> > used? If so, when?
> 
> EGL will not be used by default.

Any reason for that? Isn't it required for wayland support?
(In reply to AnAkkk from comment #39)
> (In reply to Jeff Gilbert [:jgilbert] from comment #38)
> > (In reply to Johnny Robeson from comment #37)
> > > i'm sorta confused as to what is happening with this issue. Will EGL be
> > > used? If so, when?
> > 
> > EGL will not be used by default.
> 
> Any reason for that? Isn't it required for wayland support?

I do not believe we have Wayland support by default.
Depends on: 1202175
Is it possible to disable GLX at build time?
I tried with --with-gl-providel=EGL but this changeset[1] probably broke that option.

[1] <https://hg.mozilla.org/mozilla-central/rev/f84aedf7a62d>
are there any wayland support? I observed that in plasma 5 with arch intel has improved much the egl backed, and with amd open source drivers is very good too, in the other hand nvidia privative drivers hasn't got any type of support yet, or not?
How about using the Vulkan API instead?
Comment hidden (admin-reviewed)

Updated

2 years ago
Blocks: 1215085

Comment 45

a year ago
> Blocks: 1215085

Shouldn't that be "depends on"?
And anyway it should be bug 1433081 now.
(Reporter)

Comment 46

a year ago
With Wayland, we're going to use EGL (see bug 1434574).
Maybe this bug should just be duplicated to bug 1434574.
No longer blocks: 1215085
See Also: → 1434574
We'd need to move forward with this bug as it's last serious Wayland blocker. When building for Wayland target we have to select build-time GL which is EGL. In that case we need to also use EGL on both X11 and Wayland as it's build time switch.

In order to make Wayland builds usable, is it better to use EGL for both X11 and Wayland (and fix/enable EGL for X11) or implement run-time GL context switch to use GLX on X11 and EGL on Wayland?
Flags: needinfo?(jgilbert)
EGL/Mesa performance on X11 is poor relatively to GLX implementation so it does not make sense to ship EGL/X11 builds on Linux.
Flags: needinfo?(jgilbert)

Comment 49

11 months ago
(In reply to Martin Stránský [:stransky] from comment #48)
> EGL/Mesa performance on X11 is poor relatively to GLX implementation so it
> does not make sense to ship EGL/X11 builds on Linux.

Do you have some data to back this claim up? Per my understanding EGL should be mature enough with Mesa drivers to have same or even better performance. I'm just speculating, of course, but so you do.
(In reply to Hi-Angel from comment #49)
> (In reply to Martin Stránský [:stransky] from comment #48)
> > EGL/Mesa performance on X11 is poor relatively to GLX implementation so it
> > does not make sense to ship EGL/X11 builds on Linux.
> 
> Do you have some data to back this claim up? Per my understanding EGL should
> be mature enough with Mesa drivers to have same or even better performance.
> I'm just speculating, of course, but so you do.

You can compare that by yourself, build Firefox with Wayland backend and then run it on X11.

On my HW / Fedora 28 / kernel-4.16.9-300.fc28.x86_64
EGL_CLIENT_APIS: OpenGL OpenGL_ES 
GL_VERSION: OpenGL ES 3.2 Mesa 18.0.2
GL_RENDERER: Mesa DRI Intel(R) HD Graphics 530 (Skylake GT2) 

I use http://webglsamples.org/ for testing and the speed difference is enormous.

Beside the performance the GLX backend is fine tuned for X11 so better use it there. I don't say we can't use EGL on X11 but it means lots of work IMHO.

Comment 51

11 months ago
(In reply to Martin Stránský [:stransky] from comment #50)
> (In reply to Hi-Angel from comment #49)
> > (In reply to Martin Stránský [:stransky] from comment #48)
> > > EGL/Mesa performance on X11 is poor relatively to GLX implementation so it
> > > does not make sense to ship EGL/X11 builds on Linux.
> > 
> > Do you have some data to back this claim up? Per my understanding EGL should
> > be mature enough with Mesa drivers to have same or even better performance.
> > I'm just speculating, of course, but so you do.
> 
> You can compare that by yourself, build Firefox with Wayland backend and
> then run it on X11.
> 
> On my HW / Fedora 28 / kernel-4.16.9-300.fc28.x86_64
> EGL_CLIENT_APIS: OpenGL OpenGL_ES 
> GL_VERSION: OpenGL ES 3.2 Mesa 18.0.2
> GL_RENDERER: Mesa DRI Intel(R) HD Graphics 530 (Skylake GT2) 
> 
> I use http://webglsamples.org/ for testing and the speed difference is
> enormous.
> 
> Beside the performance the GLX backend is fine tuned for X11 so better use
> it there. I don't say we can't use EGL on X11 but it means lots of work IMHO.

FTR: I asked on #radeon channel of Freenode, and I've got the following answers (picked out as there was an irrelevant discussion too). The TL;DR being that Firefox could to be using EGL wrong.

--------

[15:44:53] <Hi-Angel> Does anybody know how GLX to EGL compares in performance? I couldn't find benchmarks, and I saw a message where Firefox devs considering to not enable EGL on X11 by default, allegedly because Mesa/EGL performance is worse than GLX. I wonder, how true is that?
[15:59:25] <iive> Hi-Angel, i'm not specialist, but I think that the Xorg 2D acceleration for all new cards is done through glamor, that is using EGL.
[16:04:04] <Hi-Angel> Hah, good point! Judging by their reply https://bugzilla.mozilla.org/show_bug.cgi?id=788319#c50 could be because they're using Intel GPU, whose DDX driver doesn't use Glamor.
[17:32:19] <MrCooper> Hi-Angel: there's no inherent reason why performance would be any different between EGL and GLX; it sounds like maybe Firefox ends up using different functionality in each case
[17:33:17] <Hi-Angel> MrCooper, is it the same on Intel GPU? The Firefox dev answered, and he turns out to have tested with Intel GPU.
[17:34:15] <iive> i think glamor has been developed on intel gpu, it is just a matter if they do use it.
[17:34:39] <MrCooper> Hi-Angel: should be the same; I think he really needs to provide more information about his findings
[17:34:56] <MrCooper> iive: the modesetting driver uses glamor, xf86-video-intel doesn't
[17:35:50] <MrCooper> iive: anyway, there could theoretically be a difference between EGL as used by glamor, and EGL on X11 as used by Firefox
Comment hidden (advocacy)
Comment hidden (advocacy)

Comment 54

11 months ago
(In reply to Martin Stránský [:stransky] from comment #48)
> EGL/Mesa performance on X11 is poor relatively to GLX implementation so it
> does not make sense to ship EGL/X11 builds on Linux.

A guy on phoronix said that on when EGL is enabled, Firefox is using GLES rather than 'desktop' OpenGL: https://www.phoronix.com/forums/forum/phoronix/latest-phoronix-articles/1026675-firefox-developers-still-hesitant-about-using-egl-over-glx-on-x11-linux?p=1026709#post1026709 

This could explain the big performance difference.
(In reply to ame from comment #54)
> (In reply to Martin Stránský [:stransky] from comment #48)
> > EGL/Mesa performance on X11 is poor relatively to GLX implementation so it
> > does not make sense to ship EGL/X11 builds on Linux.
> 
> A guy on phoronix said that on when EGL is enabled, Firefox is using GLES
> rather than 'desktop' OpenGL:
> https://www.phoronix.com/forums/forum/phoronix/latest-phoronix-articles/
> 1026675-firefox-developers-still-hesitant-about-using-egl-over-glx-on-x11-
> linux?p=1026709#post1026709 
> 
> This could explain the big performance difference.

Sure, I didn't spend much time with that.

*anybody* can compile firefox with "ac_add_options --enable-default-toolkit=cairo-gtk3-wayland" and investigate why the EGL is slow/broken on X11/Wayland. I may be actually wrong and EGL works fine (you may still need to set webgl.force-enabled).

As I said I din't spend much time with that as my primary goal is to have at least *some* WebGL support.
No longer blocks Wayland - that builds use GLX for X11 and EGL for Wayland.
No longer blocks: wayland

Comment 57

5 months ago
Can we make this bug dependent on bug 1474281 ?
It might fix the observed bad performance.
You need to log in before you can comment on or make changes to this bug.