617220 - Enable D3D10 HTML5 Stereo Rendering

Status: RESOLVED FIXED

(Core :: Graphics, defect)

Description

[waveletcoeff]

User-Agent:       Mozilla/5.0 (Windows NT 6.1; rv:2.0b7) Gecko/20100101 Firefox/4.0b7
Build Identifier: 

Current HTML5 Stereo rendering works on the d3d9 layer only which users have to choose via a pref. This bug addresses this limitation.

Reproducible: Always

Comment 1

[Dhiren Bhatia]

Attachment: Enable D3D10 HTML5 Stereo rendering

Largely similar to the D3D9 code, this enables stereo html5 video if 3d_video is enabled in user prefs.

Comment 2

[Matthew Gregan [:kinetik]]

Would you mind basing this on top of the patches in bug 584259?  That removes Is3DEnabled() and adds a switch to set up the stereo mode in RenderLayer.

Comment 3

[Dhiren Bhatia]

Attachment: Enable D3D10 HTML5 Stereo Rendering, pass MONO mode to driver

Comment 4

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

Comment on attachment 498444 [details] [diff] [review]
Enable D3D10 HTML5 Stereo Rendering, pass MONO mode to driver

>diff --git a/gfx/layers/d3d10/ImageLayerD3D10.cpp b/gfx/layers/d3d10/ImageLayerD3D10.cpp
>--- a/gfx/layers/d3d10/ImageLayerD3D10.cpp
>+++ b/gfx/layers/d3d10/ImageLayerD3D10.cpp
>@@ -35,16 +35,17 @@
>  *
>  * ***** END LICENSE BLOCK ***** */
> 
> #include "ImageLayerD3D10.h"
> #include "gfxImageSurface.h"
> #include "gfxD2DSurface.h"
> #include "gfxWindowsSurface.h"
> #include "yuv_convert.h"
>+#include "..\d3d9\Nv3DVUtils.h"

Use "/" instead of "\" -- even though this code will only run on windows, it might get built by a compiler that doesn't like "\" as path separators.  Same in LayerManagerD3D10.cpp.

That can just get fixed during checkin though; rest looks fine to me, but asking bas to take a look as well.

Comment 5

[Bas Schouten (:bas.schouten)]

Just FYI, I'll have this reviewed tomorrow.

Comment 6

[Dhiren Bhatia]

Hi Bas, Any update? We would love to have the patch in soon this week. Thanks.

Comment 7

[Bas Schouten (:bas.schouten)]

Comment on attachment 498444 [details] [diff] [review]
Enable D3D10 HTML5 Stereo Rendering, pass MONO mode to driver

So, in general I think the patch looks good. I've thought about this for a while though, and I think there's one thing we should change.

We now create a new Nv3DVUtils instance for every window (every window has a layer manager), this is not the right thing to do I think.

I think we should store the Nv3DVUtils as a singleton on a per device basis. A nice way to do this would be the SetPrivateData function used for other single instances like the effects, however since Nv3DVUtils isn't an interface this means we'd need to clear it somehow on device destruction, and that seems like a slightly more complex problem.

We could implement IUnknown in Nv3DVUtils and then use SetPrivateDataInterface and we'll get it released when the device is destroyed. Essentially using a structure much like  we do for mEffect for example. I realize implementing IUnknown is a bit of a pain but it seems like the cleanest way not to get a lot of copies, although I'm open to other suggestions.

Comment 8

[Bas Schouten (:bas.schouten)]

Comment on attachment 498444 [details] [diff] [review]
Enable D3D10 HTML5 Stereo Rendering, pass MONO mode to driver

As per IRC discussion, we can do the aforementioned change in a follow-up. r+, but as vlad said, s/\\//

Comment 9

[Dhiren Bhatia]

Attachment: Enable D3D10 HTML5 Stereo Rendering, pass MONO mode to driver

Change \ to / as suggested by Vlad.

Comment 10

[Bas Schouten (:bas.schouten)]

I will get this checked in today.

Comment 11

[Matthew Gregan [:kinetik]]

Attachment: rebased for trunk

Rebased for trunk.

Comment 12

[Bas Schouten (:bas.schouten)]

http://hg.mozilla.org/mozilla-central/rev/3a20e915442d