958368 - Remove the code paths to load a custom Mesa/llvmpipe build instead of system OpenGL (sadface)

Status: RESOLVED FIXED

(Core :: Graphics, defect)

Description

[Benoit Jacob [:bjacob] (mostly away)]

Attachment: Drop the Mesa llvmpipe switch

A while ago we gained the ability to use a custom build of Mesa/llvmpipe instead of system OpenGL libraries. By itself, that wouldn't have required any special code on our side, as the same could be achieved by LD_PRELOAD'ing a libGL. But the idea was that we would then develop the ability to download such a Mesa/llvmpipe build and switch to it at runtime in order to have a software fallback for WebGL, similar to what Chrome does with Swiftshader.

These plans never concretized, unfortunately. They might still be useful, but GLContext has been significantly simplified since, and from where we are now, further simplifications could easily give us a better way of achieving the same thing with less added complexity, if we wanted to.

The other problem with the current code, which affects WGL and GLX, is that only the GLX paths could ever be tested, as Windows is not currently a supported configuration for Mesa/llvmpipe. In other words, if we wanted to get back to this, before it would make sense to accept added complexity around GLContext, we would first have to do the work to fix the Windows port of Mesa/llvmpipe.

Removing this code will also allow us to remove ContextFlags, which will be another welcome conceptual simplification.

Comment 1

[Benoit Jacob [:bjacob] (mostly away)]

$ hg diff --stat -c drop-mesa-llvmpipe-switch
 content/canvas/src/WebGLContext.cpp |   11 +-
 gfx/gl/GLContextGLX.h               |    6 +-
 gfx/gl/GLContextProviderGLX.cpp     |  120 +++++++++-----------------
 gfx/gl/GLContextProviderWGL.cpp     |  140 ++++++++++++-------------------
 gfx/gl/GLContextTypes.h             |    3 +-
 gfx/gl/GLContextWGL.h               |    7 +-
 gfx/gl/GLXLibrary.h                 |   16 +--
 gfx/gl/WGLLibrary.h                 |   18 +---
 gfx/thebes/gfxXlibSurface.cpp       |    6 +-
 9 files changed, 112 insertions(+), 215 deletions(-)

Comment 2

[Kelsey Gilbert [:jgilbert]]

Comment on attachment 8358118 [details] [diff] [review]
Drop the Mesa llvmpipe switch

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

Let's talk first. I believe we either are currently, or are planning to use llvmpipe to run WebGL tests on EC2. llvmpipe is also used for WebGL fuzzing. I think we do have users of this code, if not the WGL portion. Ideally we would get all backends for this standing up, and have automated tests of some sort for this.

Comment 3

[Benoit Jacob [:bjacob] (mostly away)]

That's totally unrelated: if we run on Ubuntu or other linux slaves where we use llvmpipe, that is as the default system OpenGL libraries. If you install the llvmpipe packages on a linux distro, it becomes what you get when you load libGL.so.1. Likewise, end-users on linux who install their distro's llvmpipe packages are using it in Firefox implicitly everytime we load libGL.so.1. If you look at the code being removed by the present patch, you'll see that the option being removed here caused us to try to load a mesallvmpipe.so library. There is no such library name on any standard linux distro. The plan was for us to distribute copies of llvmpipe with that file name, installing them in a firefox directory.

Comment 4

[Benoit Jacob [:bjacob] (mostly away)]

I'll add that the fact that you were (apparently; forgive me if I'm misunderstanding things) confused about that is evidence of the complexity of this code.

Comment 5

[Kelsey Gilbert [:jgilbert]]

Oh, ok.

Comment 6

[Kelsey Gilbert [:jgilbert]]

Comment on attachment 8358118 [details] [diff] [review]
Drop the Mesa llvmpipe switch

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

::: gfx/gl/GLContextProviderGLX.cpp
@@ +373,1 @@
>                                                       LOCAL_GLX_ALPHA_SIZE, &size);

misalignment due to change ([1])

@@ +483,5 @@
>      }
>  }
>  
>  #define BEFORE_GLX_CALL do {                     \
> +    sGLXLibrary.BeforeGLXCall();       \

[1]

@@ +488,4 @@
>  } while (0)
>  
>  #define AFTER_GLX_CALL do {                      \
> +    sGLXLibrary.AfterGLXCall();        \

[1]

::: gfx/gl/GLContextProviderWGL.cpp
@@ +664,1 @@
>                                                      nullptr, true,

[1]

Comment 7

[Benoit Jacob [:bjacob] (mostly away)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/70c49b6342bd

Comment 8

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/70c49b6342bd

Comment 9

[Lydia Franck]

(In reply to Benoit Jacob [:bjacob] (mostly away) from comment #0)

Dear Benoit

I'd like to ask a question:

> The other problem with the current code, which affects WGL and GLX, is that
> only the GLX paths could ever be tested, as Windows is not currently a
> supported configuration for Mesa/llvmpipe. 

Why is Windows currently not supported? (Or maybe this has changed in the meantime?)
I wonder because in Bug 731836 there was shown how to get Firefox to run WebGL with Mesa llvmpipe on Windows (Part 2).