605808 - "layers.prefer-opengl" won't work with D3D10 available

Status: RESOLVED FIXED

(Core :: Graphics, defect)

Description

[Dave Garrett]

http://hg.mozilla.org/mozilla-central/file/877a1f5be4f4/widget/src/windows/nsWindow.cpp#l3171

3171     if (mUseAcceleratedRendering) {
3172 #ifdef MOZ_ENABLE_D3D10_LAYER
3173       if (!preferD3D9) {
3174         nsRefPtr<mozilla::layers::LayerManagerD3D10> layerManager =
3175           new mozilla::layers::LayerManagerD3D10(this);
3176         if (layerManager->Initialize()) {
3177           mLayerManager = layerManager;
3178         }
3179       }
3180 #endif
3181 #ifdef MOZ_ENABLE_D3D9_LAYER
3182       if (!preferOpenGL && !mLayerManager) {
3183         nsRefPtr<mozilla::layers::LayerManagerD3D9> layerManager =
3184           new mozilla::layers::LayerManagerD3D9(this);
3185         if (layerManager->Initialize()) {
3186           mLayerManager = layerManager;
3187         }
3188       }
3189 #endif
3190       if (!mLayerManager && preferOpenGL) {
3191         nsRefPtr<mozilla::layers::LayerManagerOGL> layerManager =
3192           new mozilla::layers::LayerManagerOGL(this);
3193         if (layerManager->Initialize()) {
3194           mLayerManager = layerManager;
3195         }
3196       }
3197     }
3198
3199     // Fall back to software if we couldn't use any hardware backends.
3200     if (!mLayerManager)
3201       mLayerManager = CreateBasicLayerManager();

If MOZ_ENABLE_D3D10_LAYER and !preferD3D9 then you get the D3D10 layer manager. "layers.prefer-opengl" is ignored unless you also set "layers.prefer-d3d9" which doesn't make much sense to need to turn on OpenGL.

I think the " && preferOpenGL" needs to be removed from the OpenGL part (line 3190) and a "!preferOpenGL && " needs to be added to the D3D10 part (line 3173).

Comment 1

[Dave Garrett]

(In reply to comment #0)
> the " && preferOpenGL" needs to be removed from the OpenGL part (line 3190)

Scratch that first part there. It looks like Windows is only opt-in for OpenGL, so the pref check is required there.

Just needs the "!preferOpenGL && " check for D3D10 then to allow opting-in for OpenGL to work in that instance.

Comment 3

[Bas Schouten (:bas.schouten)]

Attachment: Do not use D3D10 when OGL is preferred

Comment 4

[Bas Schouten (:bas.schouten)]

This shouldn't block. OpenGL on windows is not something we officially support (as you say, it's opt-in). I want it to be available, and that's why I requested approval :-). But it doesn't block us releasing anything.

Comment 5

[Dave Garrett]

Fair enough. I just carried over the request from the dupe (which I probably didn't need to). I could probably request wanted+ but we don't seem to be using that flag much. Doesn't matter if you're already on top of things, though. ;)

Comment 6

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

Better would be to add a "layers.type" pref with string values "auto", "d3d9", "d3d10", "opengl"; otherwise this logic gets convoluted..

Comment 7

[Dave Garrett]

(In reply to comment #6)
> Better would be to add a "layers.type" pref with string values "auto", "d3d9",
> "d3d10", "opengl"; otherwise this logic gets convoluted..

The same sort of idea for multi-state prefs instead of bools was brought up in bug 590599 WRT the base enabling prefs. The problem with using a multi-state is that you don't know all the possible values, whereas if you use bools you just see the full list to flip on/off on filtering for "layers" in about:config.

The logic of the two bool prefs as we have them does make sense (assuming this patch is committed). Prefer d3d9 toggles which version of d3d to use and prefer opengl obviously means you want opengl instead. The only non-obvious case is if they're both flipped on, but it makes sense that opengl would be preferred in that case. You could add logic to flip the other off if one is on so you can set only one preference, but that's probably overkill.

If an in-GUI preference were ever to be added for all this, then yes, I agree a multi-state option would probably be better.

Comment 8

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

Sure, but you're already in about:config -- it's not really intended to be user-friendly UI.  I don't even care too much about the multiple prefs, it's just that having them interact in some magic ways here doesn't really help.  This is bikesheddy really, though :-)

Comment 9

[Jim Jeffery not reading bug-mail 1/2/11]

this appears to have had approval for awhile now - did this get lost in the shuffle ?>

Comment 10

[[not reading bugmail]]

I thought OGL was only being used with Fullscreen video on Windows or used to accelerate on OSX.

Comment 11

[Dave Garrett]

This has approval2.0+. Why hasn't it landed yet?

(In reply to comment #10)
> I thought OGL was only being used with Fullscreen video on Windows

An OpenGL renderer exists on Windows which can be enabled in full via this pref if the D3D renderer either doesn't work or is otherwise not desired.

Comment 12

[Bas Schouten (:bas.schouten)]

Hrm, anyone can land this patch of course. Thinking about it though, maybe we should leave it be and require direct2d.disabled combined with preferogl, which already works. Direct2D with OGL (as this patch allows) would lead to a very slow and unusable rendering path. OGL can not really be combined with D2D well.

Comment 13

[Dave Garrett]

(In reply to comment #12)
> should leave it be and require direct2d.disabled combined with preferogl

That wouldn't "leave it be"; that'd be a new patch. The current situation requires layers.prefer-d3d9=true to get to OpenGL mode. Direct2D isn't involved in these prefs themselves. The current prefs don't do what they say, WRT this.

> Direct2D with OGL (as this patch allows) would lead to a very
> slow and unusable rendering path. OGL can not really be combined with D2D well.

If that's the case, then possibly also disabling Direct2D with OpenGL enabled would make sense. That, however, is outside the scope of this bug which just revolves around the one existing pref not working as advertised: layers.prefer-opengl=true should turn OpenGL on when set, but it doesn't by itself.

Comment 14

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

Can we just make a layers.prefer-backend with some numbers?  I know it's not as easy to read, but the interactions between all of these are even more obtuse.  (What happens if you have both prefer-d3d9 and prefer-opengl?  I know now you get opengl, but it's not exactly obvious!  What happens if you have prefer-d3d10 and prefer-d3d9? etc.)

Comment 15

[Dave Garrett]

(In reply to comment #14)
> Can we just make a layers.prefer-backend with some numbers?  I know it's not as
> easy to read, but the interactions between all of these are even more obtuse.

A simpler solution then resorting to a convoluted multi-value pref with no user-visible legend as to what values are available would be to just auto-correct unsupported combinations. For example, if prefer-opengl is flipped on then automatically flip off prefer-d3d9 and vice versa.

> What happens if you have prefer-d3d10 and prefer-d3d9?

No prefer-d3d10 pref exists. D3D10 is preferred by default if available.

Comment 16

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

(In reply to comment #15)
> (In reply to comment #14)
> > Can we just make a layers.prefer-backend with some numbers?  I know it's not as
> > easy to read, but the interactions between all of these are even more obtuse.
> 
> A simpler solution then resorting to a convoluted multi-value pref with no
> user-visible legend as to what values are available would be to just
> auto-correct unsupported combinations. For example, if prefer-opengl is flipped
> on then automatically flip off prefer-d3d9 and vice versa.

This is actually a lot more complex code-wise; that complexity isn't worth it for something that's a hidden pref.

> > What happens if you have prefer-d3d10 and prefer-d3d9?
> 
> No prefer-d3d10 pref exists. D3D10 is preferred by default if available.

True, so it's even more magic.

Comment 17

[Dave Garrett]

(In reply to comment #16)
> (In reply to comment #15)
> > A simpler solution then resorting to a convoluted multi-value pref with no
> > user-visible legend as to what values are available would be to just
> > auto-correct unsupported combinations. For example, if prefer-opengl is flipped
> > on then automatically flip off prefer-d3d9 and vice versa.
> 
> This is actually a lot more complex code-wise; that complexity isn't worth it
> for something that's a hidden pref.

Fair enough. If you want simplest route, this patch keeps things simple and makes the OpenGL pref work as expected.

(In reply to comment #14)
> Can we just make a layers.prefer-backend with some numbers?  I know it's not as
> easy to read, but the interactions between all of these are even more obtuse.

That would add its own complexity to do. Also, just because the pref is hidden doesn't mean it won't get used by those who need it, so simple naming is a must. Numbers wouldn't be simple.

Comment 18

[Bas Schouten (:bas.schouten)]

(In reply to comment #13)
> (In reply to comment #12)
> > should leave it be and require direct2d.disabled combined with preferogl
> 
> That wouldn't "leave it be"; that'd be a new patch. The current situation
> requires layers.prefer-d3d9=true to get to OpenGL mode. Direct2D isn't involved
> in these prefs themselves. The current prefs don't do what they say, WRT this.

This is not a new patch, currently if you disable direct2d and preferogl you will also get OpenGL.


Also, D3D10 is not exactly preferred. D3D10 is basically used if Direct2D is used. This because Direct2D uses D3D10 internally and doesn't match up well with other layers backends. I personally don't think we need backend switches all together (just accelerated vs. non-accelerated as we already have). But they're useful for testing purposes so that's the reason I think we have them.

I don't think think we have them for usage by our average user.

Comment 19

[Dave Garrett]

(In reply to comment #18)
> I don't think think we have them for usage by our average user.

I could easily envision a user with an old or bran new graphics card that doesn't work well or at all with the D3D acceleration but does work with OpenGL, in which case troubleshooting documents could point to these prefs to try.

Comment 20

[Emanuel Hoogeveen [:ehoogeveen]]

So if this were to be condensed into a single numerical setting, how would it work? The following seems the most logical to me:

layers.backend = 0 (automatic)
layers.backend = 1 (no hardware acceleration)
layers.backend = 2 (OpenGl)
layers.backend = 3 (D3D9 - only valid in Windows)
layers.backend = 4 (D3D10/D2D - only valid in Windows 7 or Windows Vista w/ updates)

I sorted them in order of decreasing portability. Of course many variations are possible, for instance the default could be -1 for automatic, or -1 could refer to disabling hardware acceleration instead.

I'm not saying this is how it should work, just making it more concrete.

Comment 21

[Bas Schouten (:bas.schouten)]

(In reply to comment #19)
> (In reply to comment #18)
> > I don't think think we have them for usage by our average user.
> 
> I could easily envision a user with an old or bran new graphics card that
> doesn't work well or at all with the D3D acceleration but does work with
> OpenGL, in which case troubleshooting documents could point to these prefs to
> try.

Really? I don't think there's cards that work well with OpenGL but not with D3D9 or 10 on windows. And going from D3D10 to D3D9 is essentially disabling Direct2D which is a separate option. (i.e. I'd prefer to remove the D3D9+D2D code so prefer-d3d9 can be taken out and disabling D2D is the only way to switch to D3D9)

Comment 22

[Emanuel Hoogeveen [:ehoogeveen]]

(In reply to comment #21)
> Really? I don't think there's cards that work well with OpenGL but not with
> D3D9 or 10 on windows.

My laptop's card (a GeForce Go 7700) has problems with large textures using Direct3D 9, but not in OpenGl. I've had to disable subtitles in Media Player Classic to work around this (enabling them in ffdshow instead), and it was quite painful in the emulator bsnes. I don't know if it's a hardware problem or a long standing driver issue though, and I haven't retested recently.

Comment 23

[Bas Schouten (:bas.schouten)]

(In reply to comment #22)
> (In reply to comment #21)
> > Really? I don't think there's cards that work well with OpenGL but not with
> > D3D9 or 10 on windows.
> 
> My laptop's card (a GeForce Go 7700) has problems with large textures using
> Direct3D 9, but not in OpenGl. I've had to disable subtitles in Media Player
> Classic to work around this (enabling them in ffdshow instead), and it was
> quite painful in the emulator bsnes. I don't know if it's a hardware problem or
> a long standing driver issue though, and I haven't retested recently.

A card that old won't get D3D10 by default anyway, so prefer-ogl just works there :).

Comment 24

[Emanuel Hoogeveen [:ehoogeveen]]

Yeah, I don't think it supports D3D10 at all. Actually, for some reason I had the impression you were arguing against testing with OpenGl in Windows, but reading back I can't find anything like that, only a comment regarding OpenGl + D2D, which I agree doesn't make any sense.

Comment 25

[Dave Garrett]

It is nonetheless an actual example of why these prefs are needed for users for non-testing purposes in response to comment 18 (i.e. it's basically what I theorized in comment 19). Thanks for posting the example, in fact if your card is bad enough under D3D9 you might want to file a bug to get it blacklisted to have everyone with that (and maybe similar) card run by default using OpenGL.

Comment 26

[Emanuel Hoogeveen [:ehoogeveen]]

I haven't really noticed any performance issues using D3D9 with it in Firefox, though. At the very least, I'd have to 1) confirm the issue still exists with recent drivers, 2) find a test case where it slows Firefox to a crawl where OpenGl doesn't.

Comment 27

[Bas Schouten (:bas.schouten)]

(In reply to comment #25)
> It is nonetheless an actual example of why these prefs are needed for users for
> non-testing purposes in response to comment 18 (i.e. it's basically what I
> theorized in comment 19). Thanks for posting the example, in fact if your card
> is bad enough under D3D9 you might want to file a bug to get it blacklisted to
> have everyone with that (and maybe similar) card run by default using OpenGL.

At the moment we do not intent to support OpenGL as default on Windows for Firefox 4. This means that we can certainly blacklist the hard and make it non-accelerated if it causes issues. However it won't get OpenGL by default.

(In reply to comment #22)
> (In reply to comment #21)
> > Really? I don't think there's cards that work well with OpenGL but not with
> > D3D9 or 10 on windows.
> 
> My laptop's card (a GeForce Go 7700) has problems with large textures using
> Direct3D 9, but not in OpenGl. I've had to disable subtitles in Media Player
> Classic to work around this (enabling them in ffdshow instead), and it was
> quite painful in the emulator bsnes. I don't know if it's a hardware problem or
> a long standing driver issue though, and I haven't retested recently.

What is the max texture size in D3D9? We currently require 4096x4096 and if that's not supported we just use software already, i.e. the problem you mention about texture sizes shouldn't adversely affect firefox. You'll automatically be moved to software.

Some cards do have different max texture sizes in OGL/D3D9 (or different Non-Power-of-2 support). But for a texture size to be reported supported in D3D9 and not working on a card that old certainly sounds like a hardware issue, and I'd be very surprised if OGL did not suffer from the same issues.

Comment 28

[Emanuel Hoogeveen [:ehoogeveen]]

I'm pretty sure the card does support 4096x4096 textures, but with textures as small as 1024x1024 I was noticing significant slowdown (the card doesn't support non-power-of-two textures although it will create multiple-of-400 textures if the nearest power of two is farther away). I only did any significant testing with bsnes though, and at the time it was using a 1024x1024 pixel offscreen plain surface with StretchRect, I believe. I would have to do some testing to see which situations slow it down.

Comment 29

[Dave Garrett]

Why was this marked FIXED? I don't think the patch ever landed and it still looks like the D3D10 check is missing the OpenGL pref check.
http://hg.mozilla.org/mozilla-central/file/290712e55ade/widget/src/windows/nsWindow.cpp#l3444

Comment 30

[Joe Drew (not getting mail)]

Huh! I don't know why I thought the patch landed. Sorry!

Comment 31

[Joe Drew (not getting mail)]

Comment on attachment 484677 [details] [diff] [review]
Do not use D3D10 when OGL is preferred

Too late for 2.0, sorry.

Comment 32

[Dave Garrett]

Also won't apply to current Trunk anymore either. Needs a slightly new patch.

Comment 33

[Marco Castelluccio [:marco]]

Attachment: Patch

I've done the patch for the current codebase.

Comment 34

[Ed Morley [:emorley]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/08819514d159

Comment 35

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/08819514d159

Comment 36

[ZeDestructor]

Re-open this bug please. enabling the pref results in garbage rendering with my ATi mobility HD 4650. If I understand how this patch should work, it should ignore the pref and as such use D10 and not result in garbage rendering.

Comment 37

[Joe Drew (not getting mail)]

Please file a new bug, as that's a separate issue.

Comment 38

[ZeDestructor]

New bug for related issue: https://bugzilla.mozilla.org/show_bug.cgi?id=701277