Enable D3D10 HTML5 Stereo Rendering

RESOLVED FIXED

Status

()

Core
Graphics
RESOLVED FIXED
7 years ago
6 years ago

People

(Reporter: waveletcoeff, Unassigned)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 3 obsolete attachments)

(Reporter)

Description

7 years ago
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
(Reporter)

Updated

7 years ago
See Also: → bug 584255

Comment 1

7 years ago
Created attachment 495876 [details] [diff] [review]
Enable D3D10 HTML5 Stereo rendering

Largely similar to the D3D9 code, this enables stereo html5 video if 3d_video is enabled in user prefs.
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

7 years ago
Created attachment 498444 [details] [diff] [review]
Enable D3D10 HTML5 Stereo Rendering, pass MONO mode to driver
Attachment #495876 - Attachment is obsolete: true
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.
Attachment #498444 - Flags: review+
Attachment #498444 - Flags: review?(bas.schouten)
Just FYI, I'll have this reviewed tomorrow.

Comment 6

7 years ago
Hi Bas, Any update? We would love to have the patch in soon this week. Thanks.
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 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/\\//
Attachment #498444 - Flags: review?(bas.schouten) → review+
Attachment #498444 - Flags: approval2.0?
Attachment #498444 - Flags: approval2.0? → approval2.0+

Comment 9

7 years ago
Created attachment 502910 [details] [diff] [review]
Enable D3D10 HTML5 Stereo Rendering, pass MONO mode to driver

Change \ to / as suggested by Vlad.
Attachment #498444 - Attachment is obsolete: true
Attachment #502910 - Flags: approval2.0?
Attachment #502910 - Flags: approval2.0? → approval2.0+
Attachment #498444 - Attachment is obsolete: false
I will get this checked in today.
Created attachment 503366 [details] [diff] [review]
rebased for trunk

Rebased for trunk.
Attachment #498444 - Attachment is obsolete: true
Attachment #502910 - Attachment is obsolete: true
http://hg.mozilla.org/mozilla-central/rev/3a20e915442d
Status: UNCONFIRMED → RESOLVED
Last Resolved: 7 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.