Closed Bug 584539 Opened 14 years ago Closed 14 years ago

ThebesLayerD3D9 needs to use D2D interop

Categories

(Core :: Graphics, defect)

x86
Windows Vista
defect
Not set
normal

Tracking

()

RESOLVED FIXED
Tracking Status
blocking2.0 --- beta5+

People

(Reporter: bas.schouten, Assigned: bas.schouten)

References

Details

Attachments

(3 files)

When the render mode is Direct2D ThebesLayerD3D9 should use D3D9/D3D10 interop in order to allow cairo to use the Direct2D backend to draw directly to the ThebesLayer's texture.
Blocks: 584255
Depends on: 584754
This is needed to be able to enable D3D9 layers for D2D enabled machines. This in turn is needed for fast webgl, hardware accelerated YUV->RGB conversion and
some other things.
blocking2.0: --- → ?
blocking2.0: ? → beta5+
This patch allows creating a Direct2D surface from a SharedHandle which can come from another device. It also allows the D2D device to be flushed in such a way that the surface is ready to be used by another device.
Attachment #464302 - Flags: review?(jmuizelaar)
This does the job. These patches depend on the cairo_d2d_device_t and D3D9Ex patches. We may want to spend some time making CanvasLayer use this as well, so we get super-fast canvas too. But that will require some restructuring. It's probably best combined with the work to make ANGLE interop with us.
Attachment #464310 - Flags: review?(vladimir)
Comment on attachment 464302 [details] [diff] [review]
Part 1: Allow creating a D2D surface from a shared handle

>+void
>+cairo_d2d_flush_device(cairo_device_t *device)
>+{
>+    cairo_d2d_device_t *d2d_device = reinterpret_cast<cairo_d2d_device_t*>(device);
>+    // Here it becomes interesting, this flush method is generally called when
>+    // interop is going on between our device and another device. The
>+    // synchronisation between these devices is not always that great. The
>+    // device flush method may flush the device's command queue, but it gives
>+    // no guarantee that the device will actually be done with those commands,
>+    // and so the surface may still not be complete when the external device
>+    // chooses to use it. The EVENT query will actually tell us when the GPU
>+    // is completely done with our commands.
>+    D3D10_QUERY_DESC queryDesc;
>+    queryDesc.MiscFlags = 0;
>+    queryDesc.Query = D3D10_QUERY_EVENT;
>+    RefPtr<ID3D10Query> query;
>+
>+    d2d_device->mD3D10Device->CreateQuery(&queryDesc, &query);
>+

Probably worth adding a comment that says "Begin() is disabled with D3D10_QUERY_EVENT"

>+    query->End();

Should we flush() the device here?

>+
>+    BOOL done = FALSE;
>+    while (!done) {
>+	// This will return S_OK and done = FALSE when the GPU is not done, and
>+	// S_OK and done = TRUE when the GPU is done. Any other return value
>+	// means we need to break out or risk an infinite loop.
>+	if (FAILED(query->GetData(&done, sizeof(BOOL), 0))) {
>+	    break;
>+	}

Will this spin waiting for GetData to give done = TRUE;


>+cairo_surface_t *
>+cairo_d2d_surface_create_for_handle(cairo_device_t *device, HANDLE handle, cairo_content_t content)
>+{

[snip]
>+    hr = d2d_device->mD3D10Device->OpenSharedResource(handle,
>+						      __uuidof(ID3D10Resource),
>+						      (void**)&newSurf->surface);
>+
>+    RefPtr<ID3D10Texture2D> texture;
>+    RefPtr<IDXGISurface> dxgiSurface;
>+    D2D1_BITMAP_PROPERTIES bitProps;
>+    D2D1_RENDER_TARGET_PROPERTIES props;
>+    DXGI_FORMAT format;
>+    DXGI_SURFACE_DESC desc;
>+
>+    if (FAILED(hr)) {
>+	goto FAIL_CREATEHANDLE;
>+    }

Move this closer to the call that you're testing the return value of.


>diff --git a/gfx/cairo/cairo/src/cairo-win32.h b/gfx/cairo/cairo/src/cairo-win32.h
>--- a/gfx/cairo/cairo/src/cairo-win32.h
>+++ b/gfx/cairo/cairo/src/cairo-win32.h
> /**
>+ * Flushes a D3D device. In most cases the surface backend will do this
>+ * internally, but when using a surfaces created from a shared handle this
>+ * should be executed manually when a different device is going to be accessing
>+ * the same surface data
>+ */
>+void
>+cairo_d2d_flush_device(cairo_device_t *device);

finish may be a better name to use here. finish matches the semantics of glFlush()/glFinish()
Attachment #464302 - Flags: review?(jmuizelaar) → review+
Attachment #464303 - Flags: review?(jmuizelaar) → review+
(In reply to comment #5)
> Comment on attachment 464302 [details] [diff] [review]

> Should we flush() the device here?

GetData does this unless you specify some 'NOFLUSH' flag.

> 
> >+
> >+    BOOL done = FALSE;
> >+    while (!done) {
> >+	// This will return S_OK and done = FALSE when the GPU is not done, and
> >+	// S_OK and done = TRUE when the GPU is done. Any other return value
> >+	// means we need to break out or risk an infinite loop.
> >+	if (FAILED(query->GetData(&done, sizeof(BOOL), 0))) {
> >+	    break;
> >+	}
> 
> Will this spin waiting for GetData to give done = TRUE;

It will, this is when the GPU is completely done executing commands.

> 
> >+    RefPtr<ID3D10Texture2D> texture;
> >+    RefPtr<IDXGISurface> dxgiSurface;
> >+    D2D1_BITMAP_PROPERTIES bitProps;
> >+    D2D1_RENDER_TARGET_PROPERTIES props;
> >+    DXGI_FORMAT format;
> >+    DXGI_SURFACE_DESC desc;
> >+
> >+    if (FAILED(hr)) {
> >+	goto FAIL_CREATEHANDLE;
> >+    }
> 
> Move this closer to the call that you're testing the return value of.

This mainly means moving the declarations to above the call. I don't think we can put declarations after a forward goto, can we?
Comment on attachment 464310 [details] [diff] [review]
Part 3: Use D2D interop in D3D9 layers when D2D is enabled

Looks fine, but:

>+  if (gfxWindowsPlatform::GetPlatform()->GetRenderMode() ==
>+      gfxWindowsPlatform::RENDER_DIRECT2D) {
>+        if (mD3DManager->deviceManager()->IsD3D9Ex()) {
>+          // Strange? D2D but no D3D9Ex?

This comment is misplaced/misstated -- you maybe want something like "// if we have d2d we should have d3d9ex"..
Attachment #464310 - Flags: review?(vladimir) → review+
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: