Closed Bug 731836 Opened 12 years ago Closed 12 years ago

Add a preference to use Mesa llvmpipe for WebGL software rendering (Windows-only for now)

Categories

(Core :: Graphics: CanvasWebGL, defect)

Product:
Core
Component:
Graphics: CanvasWebGL
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla15

People

(Reporter: bjacob, Assigned: drexler)

References

Details

(Whiteboard: [mentor=bjacob] webgl-next)

Attachments

(2 files, 5 obsolete files)

patch
38.64 KB, patch
bjacob
: review+
Details | Diff | Splinter Review
updated_patch
38.90 KB, patch
Details | Diff | Splinter Review 
Currently, due to our driver blacklisting, less than 50% of users have WebGL working:

  http://people.mozilla.org/~bjacob/gfx_features_stats/

A good way to improve that is to use a software renderer as a fallback.

In the present bug, we're discussing only doing this as an add-on. But once we have that, the next step will be to have the browser automatically download it when it's needed. So really, we're aiming for a full automatic solution.


*** Part 0. Get started with Mozilla hacking ***

This page has useful tips to get started, especially for Graphics hacking (tools, how to get source code, etc):

  https://wiki.mozilla.org/Platform/GFX/Contribute

Code search tool:

  http://mxr.mozilla.org/mozilla-central/


*** Part 1. Get Mesa llvmpipe up and running ***

Mesa is an open source OpenGL library.

  http://www.mesa3d.org

Mesa can use various renderers. Some are hardware-accelerated, some are software-only. The best software-only renderer is called 'llvmpipe' because it uses LLVM for compiling shaders.

 http://www.mesa3d.org/llvmpipe.html

This is available for x86 and x86-64 on Linux, Windows and Mac. Build instructions:

 http://www.mesa3d.org/install.html

IIUC, scons is the recommended way on Windows.

Your first step is to get a build of Mesa llvmpipe for your favorite OS. To be honest, the most interesting OS here is Windows, because that's where the largest proportion of users have outdated GPU drivers, but all OSes are interesting.


*** Part 2. Get Firefox to run WebGL with Mesa llvmpipe ***

Building Mesa llvmpipe should produce a OpenGL library that you could use as a drop-in replacement for the system OpenGL libraries. Notice that on Windows, to get Firefox to use OpenGL, you have to go to about:config and set these preferences:

  webgl.prefer-native-gl = true
  webgl.force-enabled = true

You can check in about:support, 'WebGL renderer', if llvmpipe is actually used.

Run the conformance tests to check that things are decently working:

  https://cvs.khronos.org/svn/repos/registry/trunk/public/webgl/sdk/tests/webgl-conformance-tests.html

Also, you can assess performance by first disabling compositing acceleration (we want to optimize for the case where hardware acceleration doesn't work, so better work in real conditions):

  layers.acceleration.disabled = true

and then play some WebGL demos such as

  http://webglsamples.googlecode.com/hg/aquarium/aquarium.html


*** Part 3. Get Firefox to dynamically switch to Mesa llvmpipe ***

Until now we haven't had to touch Mozilla code, but now we have to. It was nice to be able to test WebGL on llvmpipe, but for it to be practical, it must not require replacing system OpenGL libraries by llvmpipe, and Firefox must be able to decide at runtime whether to use llvmpipe.

Let's not automatically fall back to llvmpipe for now. Instead, let's introduce a new preference in about:config that allows to choose llvmpipe for WebGL rendering.

The file to edit, to add a new preference, is:
http://hg.mozilla.org/mozilla-central/file/tip/modules/libpref/src/init/all.js

Call it webgl.use-llvmpipe, boolean, default to false.

In the WebGL initialization code, where we choose our 'GL context provider' (e.g. WGL vs EGL vs GLX...). The problem is twofold. First, we want use-llvmpipe to override the OpenGL driver blacklist i.e. the same effect as webgl.force-enabled. Second, on Windows, switching to Mesa llvmpipe requires switching to WGL (the default is EGL) i.e. the same effect as webgl.prefer-native-gl.

So edit this file:

http://hg.mozilla.org/mozilla-central/file/tip/content/canvas/src/WebGLContext.cpp

Search for Preferences::GetBool in this file, in WebGLContext::SetDimensions, which is where we create the OpenGL context for a WebGL context.

Once this is done (so we're now using the correct GL context provider for llvmpipe and ignoring the blacklist) we can proceed to the code where we actually load the OpenGL library and create a OpenGL context. That code is in different files depending on the OS. If you can't test on all OSes, it's fine to only do yours.

Windows: http://hg.mozilla.org/mozilla-central/file/tip/gfx/gl/GLContextProviderWGL.cpp
Mac: http://hg.mozilla.org/mozilla-central/file/tip/gfx/gl/GLContextProviderCGL.mm
Linux: http://hg.mozilla.org/mozilla-central/file/tip/gfx/gl/GLContextProviderGLX.cpp

Look for where we're setting the filename of the library we're opening. For example, on Windows it's

  http://hg.mozilla.org/mozilla-central/file/1c3b291d0830/gfx/gl/GLContextProviderWGL.cpp#l141

There, you want to read the webgl.use-llvmpipe preference again (yes, that's not super efficient, if needed we'll optimize that later). Use again Preferences::GetBool; you might need to add a #include "mozilla/Preferences.h" for it.

The filename is up to you; it should probably be different from the system default (e.g. not opengl32.dll) otherwise AFAICS we won't be able to choose one over the other.


*** Part 4. Wrap this as a Firefox add-on ***

I'm not competent with that part. We'll have to find another mentor when we're there.


*** Next steps ***

Once we're there, we'll want to see how we can get Gecko to automatically download Mesa llvmpipe when needed. We don't want to ship it with the standard Firefox build, as that would inordinately increase the Firefox download size.
I volunteer as a mentor for part 4+.

That said, I'm not sure to understand what you meant by "Wrap this as a Firefox add-on" if "this" are the previous parts, modifying existing native built-in implementations, which is something an add-on cannot do afaik.

Otoh, the subsequent "automatically download llvmpipe if needed" would imho really fit as a feature of the add-on rather than of Gecko (the add-on possibly being bundled in Firefox at a later stage).


IMHO it could be nice (and perhaps even simpler/cleaner in the end) to expose a generic "add-on aware" mechanism to specify a fallback GL implementation at context creation time (ie. no 'llvmpipe' reference in Gecko, not even as a pref name).


An add-on could provide LLVMpipe fallback but another add-on could provide another alternative software implementation as easily (possibly Transgaming could even release a SwiftShader add-on).
The add-on would also have the possibility of forcing a reduced size drawing buffer as it sees fit.


It could work by implementing a simple WebGLContextProviderFallbackService XPCOM interface like :

interface nsIWebGLContextFallbackProviderService {
  registerFallbackProvider(in WebGLContextFallbackProvider provider);
  unregisterFallbackProvider(in WebGLContextFallbackProvider provider);
  [noscript] WebGLContextFallbackProvider tryFallbackProviders(in DOMCanvasElement canvas);
}

/* this is an interface declaration only, it's up to the fallback add-on(s) to implement this */ 
interface nsIWebGLContextFallbackProvider {
  bool canProvideFallbackForCanvas(in DOMCanvasElement canvas);
  readonly DOMString providerName;     // "wgl", "cgl", "glx" or "egl"
  readonly DOMString nativeLibraryPath;// eg. "c://win/system32/my_egl.dll"
  readonly DOMString glLibraryPath;    // eg. "c://win/system32/my_gles_v2.dll"
  readonly long drawingBufferWidth;    // eg. same as canvas.width
  readonly long drawingBufferHeight;   // eg. same as canvas.height
}


tryFallbackProviders implementation:
1) for each registered WebGLContextFallbackProvider, in reverse order of registration:
  2) call canProvideFallbackForContext with the given canvas argument
  3) if the call returned true, break the loop at 1 and return the current WebGLContextFallbackProvider
4) return null


In GLContextProvider interface, append 2 optional string arguments nativeLibraryPath and glLibraryPath to CreateOffscreen().

static GLContextProvider::GetInstance(providerLibraryPath = nsnull, glLibraryPath = nsnull)


In setDimensions(), append the following steps when context creation failed :
1) call WebGLContextFallbackProviderService.tryFallbackProviders(canvas) and store the return value in local "provider"
2) if "provider" is not null, attempt to create a context, using the GLContextProvider interface specified by provider.providerName (eg. GLContextProviderWGL if "wgl") and the provider desired attributes. eg:
''
gl::GLContextProviderWGL::CreateOffscreen(gfxIntSize(provider.drawingBufferWidth, drawingBufferHeight), format, provider.nativeLibraryPath, provider.glLibraryPath);
''  


Thoughts?
(In reply to Cedric Vivier [:cedricv] from comment #1)
> I volunteer as a mentor for part 4+.
> 
> That said, I'm not sure to understand what you meant by "Wrap this as a
> Firefox add-on" if "this" are the previous parts, modifying existing native
> built-in implementations, which is something an add-on cannot do afaik.

Indeed, I was a bit unclear. I think that parts 1--3 should ship directly in Gecko. What can't ship in Gecko, is llvmpipe itself, because that would be too big (I'm worried about the Firefox download size). So we would ship parts 1--3 in Gecko itself, and wrap llvmpipe in an add-on that also has the effect of turning on the webgl.use-llvmpipe preference. Does that make sense?

The rest of what you write is super interesting, but needs to be moved to a separate bug, depending on this one, "automatically download llvmpipe when needed for WebGL software rendering". I would like to keep the present mentored bug not too big and not too scary.

In fact, step 4. could already be a separate bug.
To be even more clear: only Part 3 is about changes to be made directly in Gecko.
Summary: Make a firefox add-on that wraps Mesa llvmpipe for WebGL software rendering → Add a preference to use Mesa llvmpipe for WebGL software rendering
Blocks: 731983
I filed bug 731983 about part 4, so the present bug is now focused on building llvmpipe and adding the preference to use it.
Willing to take a shot at this if just to have an excuse at building an add-on :-)
Assignee: nobody → andrew.quartey
Excellent! I'll try to assist you through step 3 (add preference) and then you'll be off to bug 731983 with Cedric for the add-on work. (And then if you're still alive we'll think about how to have Firefox automatically download it when needed.)
I have been able to build the llvmpipe and get Firefox to use it. However, in running the conformance tests, there were several failures-100s. Is that okay? Also, in getting the demos were a lot slower to run than using ANGLE 1.0.0.930. Is that to be expected as well?
(In reply to Andrew Quartey [:drexler] from comment #7)
> I have been able to build the llvmpipe and get Firefox to use it. However,
> in running the conformance tests, there were several failures-100s. Is that
> okay? Also, in getting the demos were a lot slower to run than using ANGLE
> 1.0.0.930. Is that to be expected as well?

Failures are unsurprising, though 'hundreds' is higher than I would guess. Could you detail how you got firefox to use it, so we could take a look?

llvmpipe should be considerable slower than ANGLE, since ANGLE is nearly at parity with native GL performance.
Regarding performance, as Jeff says of course a software renderer is slower than hardware-accelerated rendering (even with ANGLE) but we saw OpenArena running very smoothly on llvmpipe on a fast Intel CPU so we know it can be good enough to be worthwhile.

One thing though, is that software renderers will tend to be more fillrate-limited than GPUs. That means that they would benefit more from lowering the resolution, than GPUs would.
Sorry for the delay with my responses. I had to get a new laptop to work on. I will rebuild the llvmpipe and detail the steps accordingly.
Hit a snag getting firefox to work with this particular graphics card although after following the exact same steps i got it working previously on a different machine. With this particular card, it defaults to a different webgl renderer: 
Intel -- Intel(R) HD Graphics Family -- 3.1.0 - Build 8.15.10.2538.



//=====Current Graphics Card===========
Adapter Description  Intel(R) HD Graphics Family
Vendor ID  8086
Device ID  0126
Adapter RAM Unknown 
Adapter Drivers igdumd64 igd10umd64 igd10umd64 igdumdx32 igd10umd32 igd10umd32
Driver Version 8.15.10.2538
Driver Date9-26-2011 
Direct2D Enabled true
Direct Write Enabled true(6.1.7601.17776)
ClearType Parameters ClearType parameters not found
WebGL Renderer Google Inc. -- ANGLE (Intel(R) HD Graphics Family) -- OpenGL ES 2.0 (ANGLE 1.0.0.930) 
GPU Accelerated Windows 1/1 Direct3D 10
AzureBackend direct2d


>Could you detail how you got firefox to use it, so we could take a look?

Steps to Build llvmpipe:
========================
Download LLVM 2.9 source
Download CMake and install. Ensure it's added to PATH

Create a new build folder for llvm. 
c:\llvm_build>cmake -DVARIABLE=value ..\llvm-2.9 -DLLVM_USE_CRT_DEBUG=MTd  -DLLVM_USE_CRT_RELEASE=MTd -DLLVM_TARGETS_TO_BUILD=X86
G option not needed as it recognizes installed compiler.

Alternatively you use the CMake-Gui to make changes to entry variables:
LLVM_USE_CRT_RELEASE=MTd
LLVM_USE_CRT_DEBUG=MTd
Configure and generate files. 


Add environment variable to point to LLVM installation and add to PATH.
This should be: C:\Program Files (x86)\LLVM
Open Visual Studio, Go to Configuration Manager for the LLVM.sln and check that INSTALL is built for both release and debug options.
Build the solution with INSTALL set as startup project.

Download scons(zip file). unzip to C:
run these commands:
# cd scons-2.1.0
# python setup.py install
 

Download and extract Mesa3d.

Mingw installation required for this:
Install msys-flex and msys-bison using this command: mingw-get install msys-flex msys-bison.

Add mingw\bin and msys\bin to PATH


change to the folder for mesa 3d
run either:
c:\Mesa-8.0.2>scons llvm=yes build=debug machine=x86 opengl32
  OR 
c:\Mesa-8.0.2>scons build=debug machine=x86 libgl-gdi

[Important: make a copy of the existing opengl32.dll in the system folder]
 
Copy the opengl32.dll from Mesa's build directory to C:\Windows\System32. 
You will have to modify the permissions on the existing opengl32.ll there to replace. 
First change ownership of the dll to yourself and then assign full rights. 
If the dll is being used by another process, use -say Process Explorer-to locate and kill it, before copying.
(In reply to Andrew Quartey [:drexler] from comment #11)
> Hit a snag getting firefox to work with this particular graphics card
> although after following the exact same steps i got it working previously on
> a different machine. With this particular card, it defaults to a different
> webgl renderer: 
> Intel -- Intel(R) HD Graphics Family -- 3.1.0 - Build 8.15.10.2538.

That means it's using the Intel OpenGL driver. If you put Mesa's DLL in the same directory as the firefox executable, does it find it? Otherwise, here's what you can try:
 - search for all files named opengl32.dll on your system
 - edit gecko source code to replace "opengl32.dll" by "mesallvmpipe.dll", rename Mesa's binary to that, so that it will only be able to find it and not the system opengl library. That's actually, probably, a step we want to take anyway, as described in comment 0, so that we can dynamically choose between mesa and system opengl.
(In reply to Benoit Jacob [:bjacob] from comment #12)

> If you put Mesa's DLL in the same directory as the firefox executable, does it find it? 
> 
It does.
With llvmpipe, some of the demos are either totally "off-color" or mostly black. I guessed that should be expected. Examples of such are: http://www.chromeexperiments.com/detail/waterstretch/?f=webgl and http://blackberry.github.com/WebGL-Samples/tunneltilt/. Also i failed to mention that attempting to build llvmpipe with LLVM 3.0 doesn't work - still trying to figure that out as it ought to be possible.
I should think that the demos should be accurately rendered. Make sure you're not also using accelerated layers. Also, try some of the very simple demos such as those at SpiderGL.
What Jeff said, plus: do not hesitate to ask Mesa devs for help, the mesa-dev mailing list is already aware of this effort and probably full of people who could help you; mesa devs also respond to bug reports at freedesktop.org.
Attached patch patch (obsolete) — — Splinter Review 
State of patch before our discussion on IRC.
Attachment #613023 - Flags: feedback?(bjacob)
Comment on attachment 613023 [details] [diff] [review]
patch

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

::: content/canvas/src/WebGLContext.cpp
@@ +379,5 @@
>      bool preferOpenGL =
>          Preferences::GetBool("webgl.prefer-native-gl", false);
>  #endif
> +    bool usellvmpipe =
> +        Preferences::GetBool("webgl.use-llvmpipe", false);

As discussed on IRC, I think we should always refer to it as 'Mesa LLVMpipe', not just 'LLVMpipe'. Both in the pref name and the variable.

Indeed:
 - llvmpipe alone doesn't give Mesa enough credit,
 - mesa alone isn't specific enough since on Linux, most people use Mesa with their GPU, just not with the llvmpipe driver.

@@ +489,5 @@
>          LogMessage("Using software rendering via OSMesa (THIS WILL BE SLOW)");
>      }
>  
> +    if (!gl && usellvmpipe) {
> +        gl = gl::GLContextProviderWGL::CreateOffscreen(gfxIntSize(width, height), format);

We can do better/simpler than that, without hardcoding WGL here.

Just GLContextProvider will give you the default context provider on the current platform. On Windows, it gives you GLContextProviderWGL. We already do that below, but only after we try EGL which is what we do by default on Windows (ANGLE rendering uses the EGL path).

Just edit this part:

    if (PR_GetEnv("MOZ_WEBGL_FORCE_OPENGL")) {
        preferEGL = false;
        useANGLE = false;
        useOpenGL = true;
    }

and add

    useMesaLlvmpipe ||

in that if() condition. This way, enabling llvmpipe will cause ANGLE to be disabled and thus we'll skip straight to the desktop OpenGL path.

@@ +491,5 @@
>  
> +    if (!gl && usellvmpipe) {
> +        gl = gl::GLContextProviderWGL::CreateOffscreen(gfxIntSize(width, height), format);
> +        if (!gl || !InitAndValidateGL()) {
> +            LogMessage("Error during Mesa initialization");

Mesa LLVMpipe.

::: gfx/gl/GLContextProviderWGL.cpp
@@ +136,5 @@
>      if (mInitialized)
>          return true;
>  
> +    bool usellvmpipe = Preferences::GetBool("webgl.use-llvmpipe", false);
> +    mozilla::ScopedGfxFeatureReporter reporter("WGL", usellvmpipe);

Good call. This way, if we have a surge of crashiness correlated with llvmpipe usage, we'll know that these people did something nonstandard.

We'll want to revert that, though, if/when we automatically use llvmpipe.

@@ +150,5 @@
> +        else {
> +           mOGLLibrary = PR_LoadLibrary("Opengl32.dll");
> +           if (!mOGLLibrary) {
> +              NS_WARNING("Couldn't load OpenGL DLL.");
> +              return false;

Please factor out this code by doing e.g.

   const char *libGLfilename = useMesaLlvmpipe ? "mesallvmpipe.dll" : "opengl32.dll";

Don't worry about outputting a different NS_WARNING, it's not a sufficient reason to avoid factoring this code.
Attachment #613023 - Flags: feedback?(bjacob) → feedback+
I've been tinkereing with building and using llvmpipe with firefox on Linux. I've not had any trouble building it against LLVM 3.0. I just built LLVM 3.0, put it in my $PATH and built Mesa 8.0.2 against it. Using LLVM 3.0 also fixes some rendering bugs I was seeing with LLVM 2.8.

Slightly off topic, and maybe something for another bug report, but, I've also hacked my version of Mesa to fast path glReadPixels. With Mesa 8.0.2 the developers significantly improved the performance of glReadPixels. Problem is, the performance boost for llvmpipe is for BGRA data only. Firefox seems to read RGBA data, so it doesn't benefit at all. I bodged the code to force a fast path for RGBA data. My patch regresses BRGA performance (and rendering) but boosts RGBA performance (on my machine) by up to 850%.

Before (stats from mesa-demos-8.0.1/src/perf/readpixels):

glReadPixels(10 x 10, RGBA/ubyte): 12002.9 images/sec, 4.6 Mpixels/sec
glReadPixels(100 x 100, RGBA/ubyte): 2229.7 images/sec, 85.1 Mpixels/sec
glReadPixels(500 x 500, RGBA/ubyte): 109.8 images/sec, 104.7 Mpixels/sec
glReadPixels(1000 x 1000, RGBA/ubyte): 27.8 images/sec, 106.2 Mpixels/sec
glReadPixels(10 x 10, BGRA/ubyte): 12760.1 images/sec, 4.9 Mpixels/sec
glReadPixels(100 x 100, BGRA/ubyte): 11959.1 images/sec, 456.2 Mpixels/sec
glReadPixels(500 x 500, BGRA/ubyte): 5736.7 images/sec, 5470.9 Mpixels/sec
glReadPixels(1000 x 1000, BGRA/ubyte): 567.0 images/sec, 2162.9 Mpixels/sec


After:

glReadPixels(10 x 10, RGBA/ubyte): 12226.9 images/sec, 4.7 Mpixels/sec
glReadPixels(100 x 100, RGBA/ubyte): 8047.2 images/sec, 307.0 Mpixels/sec
glReadPixels(500 x 500, RGBA/ubyte): 973.4 images/sec, 928.3 Mpixels/sec
glReadPixels(1000 x 1000, RGBA/ubyte): 250.5 images/sec, 955.5 Mpixels/sec
glReadPixels(10 x 10, BGRA/ubyte): 12291.1 images/sec, 4.7 Mpixels/sec
glReadPixels(100 x 100, BGRA/ubyte): 8287.3 images/sec, 316.1 Mpixels/sec
glReadPixels(500 x 500, BGRA/ubyte): 944.6 images/sec, 900.9 Mpixels/sec
glReadPixels(1000 x 1000, BGRA/ubyte): 239.9 images/sec, 915.2 Mpixels/sec


Patch (a horrible hack, but it works for firefox):

--- src/mesa/main/readpix.c     2012-04-08 18:14:38.263151001 +0100
+++ src/mesa/main/readpix.c     2012-04-08 19:30:21.811151008 +0100
@@ -209,8 +209,6 @@
    GLubyte *dst, *map;
    int dstStride, stride, j, texelBytes;
 
-   if (!_mesa_format_matches_format_and_type(rb->Format, format, type))
-      return GL_FALSE;
 
    /* check for things we can't handle here */
    if (packing->SwapBytes ||
@@ -240,10 +238,19 @@
    }
 
    texelBytes = _mesa_get_format_bytes(rb->Format);
+
+   uint32_t dst_off=0,map_off=0;
+   uint32_t k;
+
    for (j = 0; j < height; j++) {
-      memcpy(dst, map, width * texelBytes);
-      dst += dstStride;
-      map += stride;
+      for(k=0;k<width * texelBytes;k=k+4){
+         dst[dst_off+k+2]=map[map_off+k+0];
+         dst[dst_off+k+1]=map[map_off+k+1];
+         dst[dst_off+k+0]=map[map_off+k+2];
+         dst[dst_off+k+3]=map[map_off+k+3];
+      }
+      dst_off += dstStride;
+      map_off += stride;
    }
 
    ctx->Driver.UnmapRenderbuffer(ctx, rb);
(In reply to Liam Wilson from comment #19)
> I've been tinkereing with building and using llvmpipe with firefox on Linux.
> I've not had any trouble building it against LLVM 3.0. I just built LLVM
> 3.0, put it in my $PATH and built Mesa 8.0.2 against it. Using LLVM 3.0 also
> fixes some rendering bugs I was seeing with LLVM 2.8.
> 
> Slightly off topic, and maybe something for another bug report, but, I've
> also hacked my version of Mesa to fast path glReadPixels. With Mesa 8.0.2
> the developers significantly improved the performance of glReadPixels.
> Problem is, the performance boost for llvmpipe is for BGRA data only.
> Firefox seems to read RGBA data, so it doesn't benefit at all.

Depends, but here, we're talking about software compositing, Firefox uses Cairo for that, which wants BGRA. We actually pay for the conversion cost between RGBA and BGRA. So if we have an interface somewhere that wants RGBA, that means we're paying the conversion twice. Very interesting stuff, but please move it to a separate bug to keep the conversation tractable.
(In reply to Benoit Jacob [:bjacob] from comment #20)
> (In reply to Liam Wilson from comment #19)
> > Slightly off topic, and maybe something for another bug report, but, I've
> > also hacked my version of Mesa to fast path glReadPixels. With Mesa 8.0.2
> > the developers significantly improved the performance of glReadPixels.
> > Problem is, the performance boost for llvmpipe is for BGRA data only.
> > Firefox seems to read RGBA data, so it doesn't benefit at all.
> 
> Depends, but here, we're talking about software compositing, Firefox uses
> Cairo for that, which wants BGRA. We actually pay for the conversion cost
> between RGBA and BGRA. So if we have an interface somewhere that wants RGBA,
> that means we're paying the conversion twice. Very interesting stuff, but
> please move it to a separate bug to keep the conversation tractable.

Thought so, I've opened bug 743585 .
Back to the patch. Another issue, that I wasn't aware of when I filed the bug, is that preferences should only be accessed from the main thread. The problem is that GL contexts can be created from outside the main thread; even if it's not actually going to happen here now (WebGL is entirely on the main thread) it's still worth fixing.

Fortunately there is a simple fix that will also resolve the issue, mentioned in comment 0, that it's not great to be reading that preference twice. See what is done by the main patch in bug 686735: read the preference in gfxPlatform initialization (which happens on main thread), make it available from there by an inline getter so it's super cheap to access. User code will have to #include "gfxPlatform.h".
Blocks: 743755
Whiteboard: [mentor=bjacob] → [mentor=bjacob] webgl-next
(In reply to Benoit Jacob [:bjacob] from comment #22)

> Fortunately there is a simple fix that will also resolve the issue,
> mentioned in comment 0, that it's not great to be reading that preference
> twice. See what is done by the main patch in bug 686735: read the preference
> in gfxPlatform initialization (which happens on main thread), make it
> available from there by an inline getter so it's super cheap to access. User
> code will have to #include "gfxPlatform.h".

This wont be applicable as the old value is what will be read if the pref is changed after initialization. Unfortunately,the pref will have to be read again to determine which loaded library to use.
Here is another solution: the problem here is that GLContext creation can happen off the main thread, but on the other hand, WebGL context creation can only happen on the main thread, so that is a great place to read the llvmpipe pref. Just find a way to pass that info to GLContext creation. I think the right way is to make that a new flag as in GLContext::ContextFlags. In WebGLContext.cpp, when calling CreateOffscreen, you could pass such a new flag to indicate that you want llvmpipe rendering.
Attached patch patch v1 (obsolete) — — Splinter Review 
Made changes to handle the loading of an addtional library, Mesallvmpe.dll. In addition, i am currently running tests to provide performance comparison to Chrome using both fraps and the phoronix test suite.
Attachment #613023 - Attachment is obsolete: true
Attachment #616963 - Flags: review?(bjacob)
Comment on attachment 616963 [details] [diff] [review]
patch v1

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

It's great to see this progress and unfortunate that GLContextProviderWGL is so poorly designed. R- mostly so we can discuss a different approach to working around GLContextProviderWGL crappiness.

::: content/canvas/src/WebGLContext.cpp
@@ +379,5 @@
>      bool preferOpenGL =
>          Preferences::GetBool("webgl.prefer-native-gl", false);
>  #endif
> +    bool useMesaLlvmPipe =
> +        Preferences::GetBool("webgl.use-MesaLLVMPipe", false);

let's keep preferences all lowercase. you can add more dashes to separate words (maybe between mesa and llvmpipe).

::: gfx/gl/GLContext.h
@@ +574,5 @@
>  
>      enum ContextFlags {
> +        ContextFlagsNone     = 0x0,
> +        ContextFlagsGlobal   = 0x1,
> +        ContextFlagsMesaLLVM = 0x2

I would always say 'LLVMPipe' not just LLVM: LLVMPipe is the real name of that driver while LLVM is a general compiler project used in many places.

::: gfx/gl/GLContextProviderWGL.cpp
@@ +54,5 @@
>  namespace mozilla {
>  namespace gl {
>  
> +#define OPENGL_LIB     0
> +#define MESA_LLVMPIPE  1

Can we have enum values instead of #defines?

(But see below about gCurrLib, I'm proposing a different design)

@@ +59,2 @@
>  
> +WGLLibrary sWGLLibrary[2];  //Opengl lib & MesaLLVMPipe

Instead of hardcoding 2, give it a name like GLLibrariesCount.

(But see below about gCurrLib, I'm proposing a different design)

@@ +61,2 @@
>  
> +PRUint32 gCurrLib = OPENGL_LIB;  //current library in use. 

I have to say, I didn't know that we had all these global variables in this file (like gSharedWindow).

This makes it hard to find a solution that will really work, and will allow dynamic switching to/from llvmpipe.

Your solution seems like it only works as long as only one WGL library is in use at a given time (why else would there be a global variable named gCurrLib).

Let me propose that for the short term, we just require that only one WGL library will be used for the whole run of the application. So in the first context creation (maybe in WGLLibrary::EnsureInitialized or in some constructor), record the WGL library type (real opengl or mesa llvmpipe) and in subsequent calls, compare the new library type to the recorded one and fail context creation if they're different.

Indeed, the rationale is that if native OpenGL ever succeeds, there should be no reason to use llvmpipe, and if native OpenGL fails, we have no choice but to use llvmpipe.

Then, the better solution, that will allow dynamic switching between native OpenGL and llvmpipe, will consist in refactoring this GLContextProviderWGL.cpp file to get rid of these global variables.

::: gfx/thebes/gfxPlatform.cpp
@@ +336,5 @@
>      gPlatform->mFontPrefsObserver = new FontPrefsObserver();
>      Preferences::AddStrongObservers(gPlatform->mFontPrefsObserver, kObservedPrefs);
>  
>      gPlatform->mWorkAroundDriverBugs = Preferences::GetBool("gfx.work-around-driver-bugs", true);
> +    

Please avoid whitespace changes far away from code you edited. Makes hg annotate harder to use.

::: gfx/thebes/gfxPlatform.h
@@ +455,5 @@
>       */
>      static PRLogModuleInfo* GetLog(eGfxLog aWhichLog);
>  
>      bool WorkAroundDriverBugs() const { return mWorkAroundDriverBugs; }
> +    

Same.

::: modules/libpref/src/init/all.js
@@ +3432,5 @@
>  pref("webgl.min_capability_mode", false);
>  pref("webgl.disable-extensions", false);
>  pref("webgl.msaa-level", 2);
>  pref("webgl.msaa-force", false);
> +pref("webgl.use-MesaLLVMPipe", false);

lowercase for preferences.
Attachment #616963 - Flags: review?(bjacob) → review-
(In reply to Benoit Jacob [:bjacob] from comment #26)

> 
> Your solution seems like it only works as long as only one WGL library is in
> use at a given time (why else would there be a global variable named
> gCurrLib).

The global variables present enforces this behavior so as to ensure the correctness of the loaded library. Refactoring, will obviously remove this. 

 
> Let me propose that for the short term, we just require that only one WGL
> library will be used for the whole run of the application. 
> Indeed, the rationale is that if native OpenGL ever succeeds, there should
> be no reason to use llvmpipe, and if native OpenGL fails, we have no choice
> but to use llvmpipe.

Does this imply that on a system where native OpenGL will succeed, if the use-mesa-llvmpipe pref is set to true, the preference should be ignored and the Opengl library loaded instead? Also what about the converse scenario? Do we force load the mesa-llvmpipe library even if the pref is still defaulted to false?
Good question. I think that we definitely should let the use-mesa-llvmpipe preference force usage of llvmpipe even if native OpenGL would succeed. So that means that when use-mesa-llvmpipe is true, we should always use Mesa llvmpipe for all OpenGL (non-EGL) contexts and switch WebGL to using OpenGL (non-EGL) as well. Since that means that that preference affects more than just WebGL, how about renaming it to gfx.use-mesa-llvmpipe? Or maybe gfx.prefer-mesa-llvmpipe as I think it should silently revert to native OpenGL is mesa-llvmpipe is not found.
Attached patch patch (obsolete) — — Splinter Review 
Refactored GLContextProviderWGL.cpp: moved dependent global variables for loaded libray to WGLLibrary.h. I avoided making changes to initialization logic so as to be consistent with other GLContextProviders{EGL, OSMesa...} Also, since GL contexts can be created off the main thread, i added events to check the Mesa LLVMpipe preference at places where a GLContext can be created via a call to GLContextProviderWGL::CreateForWindow.
Attachment #616963 - Attachment is obsolete: true
Attachment #622105 - Flags: review?(bjacob)
Comment on attachment 622105 [details] [diff] [review]
patch

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

Sorry, there still is something significant I'm not sure I agree with (scroll down to mLibType).

::: gfx/gl/GLContextProviderWGL.cpp
@@ +59,5 @@
> +PRUint32
> +SelectLibrary(const GLContext::ContextFlags aFlags)
> +{
> +  return aFlags & GLContext::ContextFlagsMesaLLVMPipe ? 
> +    (PRUint32)WGLLibrary::MESA_LLVMPIPE_LIB : (PRUint32)WGLLibrary::OPENGL_LIB;

If this function returned a WGLLibrary::LibraryType, you wouldn't have to cast to PRUint32.

If you do return a plain integer type as opposed to an enum type, why not return just |int|? You don't need to explicitly have it 32bit unsigned.

Nit: I would put the ? and : on new lines, as proposed in mfbt/STYLE:

  = condition
    ? xxx
    : yyy;

@@ +140,2 @@
>  
> +    const char* libGLFilename = useMesaLlvmPipe ? "mesallvmpipe.dll" : "Opengl32.dll";

Multi-line please, with style proposed above.

@@ +141,5 @@
> +    const char* libGLFilename = useMesaLlvmPipe ? "mesallvmpipe.dll" : "Opengl32.dll";
> +    if (!mOGLLibrary) {
> +      mOGLLibrary = PR_LoadLibrary(libGLFilename);
> +      if (!mOGLLibrary) {
> +        char msg[35] = "Couldn't load ";

Why 35 ? :-)

@@ +142,5 @@
> +    if (!mOGLLibrary) {
> +      mOGLLibrary = PR_LoadLibrary(libGLFilename);
> +      if (!mOGLLibrary) {
> +        char msg[35] = "Couldn't load ";
> +        NS_WARNING(strcat(msg, libGLFilename));

Suggest you use strncat here.

@@ +408,5 @@
>      HANDLE mPBuffer;
>      int mPixelFormat;
>  
>      bool mIsDoubleBuffered;
> +    PRUint32 mLibType;     //the library from which the context is created.

That's the part I don't understand:

Didn't we agree earlier that we could make the restriction that only one WGL library type can be used for the entire browser run?

So I expected that something that mLibType wouldn't be needed and you could accordingly make substantial simplications in all the places where it's currently used.

Sorry for having taken so long for this review. Let's nail this point and land this patch asap!
Attachment #622105 - Flags: review?(bjacob) → review-
Comment on attachment 622105 [details] [diff] [review]
patch

IRC discussion -> sorry for self contradicting myself (see comment 26) on whether we want dynamic switching. Will finish the review asap.
Attachment #622105 - Flags: review- → review?(bjacob)
Continued the review; the cost, in terms of code complexity, to allow dynamic switching, is really huge. It will have to be done again for each other context provider. I think I'm back to the opinion I had in comment 26: that allowing dynamic switching is not worth the hassle.

Other things to consider: even allowing this dynamic switching, certain things won't work in full generality. For example, the global context is meant to allow resource sharing across contexts, but it won't work across different GL libraries.
Err, I mean in comment 30. Sorry for the confusion.
Attached patch patch (obsolete) — — Splinter Review 
Restriction to use only one library for duration of browser run.
Attachment #622105 - Attachment is obsolete: true
Attachment #622105 - Flags: review?(bjacob)
Attachment #627255 - Flags: review?(bjacob)
Attached patch patch (obsolete) — — Splinter Review 
Added missing namespace qualifiers.
Attachment #627255 - Attachment is obsolete: true
Attachment #627255 - Flags: review?(bjacob)
Attachment #627293 - Flags: review?(bjacob)
Comment on attachment 627293 [details] [diff] [review]
patch

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

So, after our conversations today (IRC + skype) and reading this patch, I understand a bit more than before --- sorry I've been a bit slow to understanding this: the added complexity to support dynamic switching is actually not that big, since the majority of that is just the changes to move global variables into members of the WGLLibrary singleton, which is a good change anyway. So, I agree again that dynamic switching is a good idea :-) sorry... this time I shouldn't change my opinion again anymore.

So we're very near r+ here, I would just like these changes:
 1. revert to allowing dynamic switching
 2. only read the llvmpipe preference in WebGL context creation, let every other place create OpenGL contexts with default flags (so, no LLVMpipe) regardless of the preference. Indeed, LLVMpipe is only really useful for WebGL. For Layers, using OpenGL layers with LLVMpipe won't be faster than BasicLayers. We will simply have to ensure that the interop between WebGL/LLVMpipe and BasicLayers works efficiently (i.e. we'll have to ensure that we read back the pixels in LLVMpipe's native format).
     -> bonus: this means that you won't have to deal anymore with reading preferences outside of the main thread.

Some nits below:

::: content/canvas/src/WebGLContext.cpp
@@ +370,5 @@
>  #endif
>      bool forceEnabled =
>          Preferences::GetBool("webgl.force-enabled", false);
> +    bool useMesaLlvmPipe =
> +        Preferences::GetBool("gfx.prefer-mesa-llvmpipe", false);

Right, and as we discussed, this should actually be the only place where we read this pref, since only WebGL is interested in LLVMpipe.

@@ +530,5 @@
>          if (gl && !InitAndValidateGL()) {
> +            const char* msg = useMesaLlvmPipe 
> +                              ? "Error during Mesa LLVMPipe initialization"
> +                              : "Error during OpenGL initialization";
> +            GenerateWarning(msg);

GenerateWarning actually takes varargs so you could do %s here. Up to you.

::: gfx/gl/GLContextProviderWGL.cpp
@@ +29,5 @@
> +SelectLibrary(const GLContext::ContextFlags& aFlags)
> +{
> +  return (aFlags & GLContext::ContextFlagsMesaLLVMPipe) 
> +            ? WGLLibrary::MESA_LLVMPIPE_LIB
> +            : WGLLibrary::OPENGL_LIB;

Suggestion: align ? and : with aFlags.

@@ +116,2 @@
>          if (!mOGLLibrary) {
> +            NS_WARNING("Couldn't load MesaLLVMPipe/OpenGL library.");

I would just say OpenGL here. Mesa is a special case of it. Mention of Mesa LLVMpipe will be surprising if we ever start using system OpenGL more widely.

@@ +407,5 @@
>      } else if (offs->mOffscreenTexture) {
> +          GLContext::ContextFlags flag = 
> +                       sWGLLibrary.LibraryType() == WGLLibrary::OPENGL_LIB
> +                       ? GLContext::ContextFlagsNone
> +                       : GLContext::ContextFlagsMesaLLVMPipe;

Nit: Given that the choice is between ContextFlagsMesaLLVMPipe and None, the type I would check for is the LLVMPIPE one.
Attachment #627293 - Flags: review?(bjacob) → review-
Side notes:
 - OpenGL layers with WGL don't work at the moment anyway. This would be interesting to fix, but not likely to be used in practice.
 - a LayerManager only gets created when a new window is opened, so there wouldn't really be dynamic switching to/from LLVMpipe for Layers, even if Layers supported LLVMpipe.
Attached patch patch — — Splinter Review 
Reversion to dynamic switching.
Attachment #627293 - Attachment is obsolete: true
Attachment #629029 - Flags: review?(bjacob)
Comment on attachment 629029 [details] [diff] [review]
patch

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

Wonderful!

Just one nit. Please address it and I'll land it for you? Unless you have L3 access already?

::: gfx/gl/GLContextProviderWGL.cpp
@@ +27,4 @@
>  
> +int 
> +SelectLibrary(const GLContext::ContextFlags& aFlags)
> +{

Couldn't this function return a WGLLibrary::LibraryType instead of a plain int?

Also, I feel that it should be a static method in WGLLibrary instead of a global function. Or if you keep it a global function, at least make its name more specific than mozilla::gl::SelectLibrary. Mention WGL.
Attachment #629029 - Flags: review?(bjacob) → review+
Attached patch updated_patch — — Splinter Review 
Made SelectLibrary a static function of WGLLibrary and typedef'd WGLLibrary::Library. 

> Just one nit. Please address it and I'll land it for you? Unless you have L3
> access already?

Just L1 for now.
OK, will try landing your patch this weekend so it'll hopefully be in Firefox 15.

Try:
https://tbpl.mozilla.org/?tree=Try&rev=f85aa503c34a
Summary: Add a preference to use Mesa llvmpipe for WebGL software rendering → Add a preference to use Mesa llvmpipe for WebGL software rendering (Windows-only for now)
https://hg.mozilla.org/mozilla-central/rev/ffcc51500617
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Hey, I haven't had much luck trying to build Mesa llvmpipe on Windows following Comment 11.

Could anyone share 32bit and 64bit, optimized, non-debug builds of Mesa llvmpipe? Or we could ask on mesa-dev for that.
Blocks: 761155
(In reply to Benoit Jacob [:bjacob] from comment #45)
> Hey, I haven't had much luck trying to build Mesa llvmpipe on Windows
> following Comment 11.
  That's odd since i used it for the debug build of llvmpipe. 
> Could anyone share 32bit and 64bit, optimized, non-debug builds of Mesa
> llvmpipe? Or we could ask on mesa-dev for that.
I'll follow this up on the mesa-users mailing list.
Specifically, my problem was that when I tried to build with the cmake configuration from comment 11, the MSVC 2010 compiler crashed trying to compile LLVM. (got Windows popup dialogs about process crashes for the MSVC compiler).
Btw you're right, mesa-users sounds like the better mailing list for that.
 (In reply to Benoit Jacob [:bjacob] from comment #47)
> Specifically, my problem was that when I tried to build with the cmake
> configuration from comment 11, the MSVC 2010 compiler crashed trying to
> compile LLVM. (got Windows popup dialogs about process crashes for the MSVC
> compiler).

Oh seems you attempted a release build per the instructions in comment 11. I should have said that that was specifically for creating debug builds. Yet to have a proper working release build myself- crashes after the context is created in the few spiderGL demos i tried. The debug versions didn't have a problem with those. Obviously, i am linking the CRT libraries wrongly. Until we get a proper handle on creating the optimized builds perhaps it will be good to put a blocker on this bug..??
Ah, indeed I was trying to make a release build.

Technically this bug is fixed regardless of whether we manage to build Mesa (!!) but "make 32 and 64 bit optimized Mesa llvmpipe / Windows builds" is definitely worth a bug, and should then block the other bug about making it an add-on.
From the feedback I got on the mesa-users mailing list, i was building the release version correctly. I had been using LLVM_USE_CRT_RELEASE=MT  and LLVM_USE_CRT_DEBUG=MTd (thought i needed extra options). So with this, the crash issue i mentioned in comment 49 indicates the problem lies elsewhere. Example demo is: http://spidergl.org/example.php?id=11.
Blocks: 1193695
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: