Closed
Bug 958368
Opened 11 years ago
Closed 11 years ago
Remove the code paths to load a custom Mesa/llvmpipe build instead of system OpenGL (sadface)
Categories
(Core :: Graphics, defect)
Core
Graphics
Tracking
()
RESOLVED
FIXED
mozilla29
People
(Reporter: bjacob, Assigned: bjacob)
References
Details
(Whiteboard: [qa-])
Attachments
(1 file)
(deleted),
patch
|
jgilbert
:
review+
|
Details | Diff | Splinter Review |
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.
Attachment #8358118 -
Flags: review?(jgilbert)
Assignee | ||
Comment 1•11 years ago
|
||
$ 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•11 years ago
|
||
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.
Attachment #8358118 -
Flags: review?(jgilbert)
Assignee | ||
Comment 3•11 years ago
|
||
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.
Assignee | ||
Comment 4•11 years ago
|
||
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•11 years ago
|
||
Oh, ok.
Comment 6•11 years ago
|
||
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]
Attachment #8358118 -
Flags: review+
Assignee | ||
Comment 7•11 years ago
|
||
Assignee: nobody → bjacob
Target Milestone: --- → mozilla29
Comment 8•11 years ago
|
||
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Updated•11 years ago
|
Whiteboard: [qa-]
Comment 9•9 years ago
|
||
(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).
You need to log in
before you can comment on or make changes to this bug.
Description
•