Closed
Bug 584539
Opened 14 years ago
Closed 14 years ago
ThebesLayerD3D9 needs to use D2D interop
Categories
(Core :: Graphics, defect)
Tracking
()
RESOLVED
FIXED
Tracking | Status | |
---|---|---|
blocking2.0 | --- | beta5+ |
People
(Reporter: bas.schouten, Assigned: bas.schouten)
References
Details
Attachments
(3 files)
7.89 KB,
patch
|
jrmuizel
:
review+
|
Details | Diff | Splinter Review |
1.50 KB,
patch
|
jrmuizel
:
review+
|
Details | Diff | Splinter Review |
13.59 KB,
patch
|
vlad
:
review+
|
Details | Diff | Splinter Review |
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.
Assignee | ||
Comment 1•14 years ago
|
||
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: --- → ?
Updated•14 years ago
|
blocking2.0: ? → beta5+
Updated•14 years ago
|
Blocks: d3d9-layers
Assignee | ||
Comment 2•14 years ago
|
||
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)
Assignee | ||
Comment 3•14 years ago
|
||
Attachment #464303 -
Flags: review?(jmuizelaar)
Assignee | ||
Comment 4•14 years ago
|
||
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 5•14 years ago
|
||
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+
Updated•14 years ago
|
Attachment #464303 -
Flags: review?(jmuizelaar) → review+
Assignee | ||
Comment 6•14 years ago
|
||
(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+
Assignee | ||
Comment 8•14 years ago
|
||
Pushed http://hg.mozilla.org/mozilla-central/rev/2cd5d96fcc70.
Pushed http://hg.mozilla.org/mozilla-central/rev/91bf043c5365.
Pushed http://hg.mozilla.org/mozilla-central/rev/72ec4df3de76.
Status: ASSIGNED → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•