Closed Bug 593268 Opened 9 years ago Closed 9 years ago

CanvasLayerD3D9 should use D2D Interop

Categories

(Core :: Graphics, defect)

x86
Windows 7
defect
Not set

Tracking

()

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

People

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

References

Details

Attachments

(5 files, 4 obsolete files)

We should allow CanvasLayerD3D9 to interop while Canvas is drawing to a D2D surface. We decided to do this by exposing a CreateOptimalSurface from LayerManager, so that layer managers can choose to expose surfaces specifically suited to be used by the layer manager.
Attachment #471749 - Flags: superreview?(roc)
Attachment #471749 - Flags: review?(vladimir)
Attachment #471749 - Attachment description: Expose CreateOptimalSurface API on LayerManager → Part 1: Expose CreateOptimalSurface API on LayerManager
Attachment #471750 - Flags: review?(vladimir)
I think it would be a little better to have a method LayerManager::CreateCanvasSurface. It's more clear exactly what that means.
(In reply to comment #4)
> I think it would be a little better to have a method
> LayerManager::CreateCanvasSurface. It's more clear exactly what that means.

I think, as discussed on IRC, that there's no real use case for different types. I cannot envision a LayerManager wanting different kinds of optimal surfaces for subtly different users. I believe a general, re-usable Optimal surface is a better idea.
Canvas surfaces are read/write. For images you could easily imagine having a write-once-only surface that's optimized by caching the rendered image somewhere where there's no protocol to update it.
Comment on attachment 471749 [details] [diff] [review]
Part 1: Expose CreateOptimalSurface API on LayerManager

I think it's a minor point though, so I'll let this go.
Attachment #471749 - Flags: superreview?(roc) → superreview+
Attached patch Add surface types (obsolete) — Splinter Review
Attachment #471864 - Flags: review?(bas.schouten)
Comment on attachment 471750 [details] [diff] [review]
Part 2: Make Canvas use CreateOptimalSurface

Hmm, this has a chance of crashing if content is null or if there's no owner document (LayerManagerForDocument assumes a non-null arg).  We can hit that in various cases, including rob arnold's aero peek crazyland.  So need to null check everything along the way and fall back to CreateOffscreenSurface if anything along the way is null.
Attachment #471750 - Flags: review?(vladimir) → review-
Attachment #471864 - Flags: review?(bas.schouten) → review+
Attachment #471751 - Attachment is obsolete: true
Attachment #471906 - Flags: review+
Attachment #471751 - Flags: review?(jmuizelaar)
Attachment #471906 - Flags: review+ → review?(jmuizelaar)
Updated to be ready to land, r=me, patch is by jrmuizel.
Attachment #471864 - Attachment is obsolete: true
Attachment #471907 - Flags: review+
Updated per review comments and ready to land.
Attachment #471750 - Attachment is obsolete: true
Attachment #471909 - Flags: review?(vladimir)
Corrected copy-paste mistake.
Attachment #471911 - Flags: review?(vladimir)
Attachment #471911 - Attachment description: Part 3: Implement and use CreateOptimalSurface for CanvasLayerD3D9 v3 → Part 2: Make Canvas use CreateOptimalSurface v3
Attachment #471909 - Attachment is obsolete: true
Attachment #471909 - Flags: review?(vladimir)
Duplicate of this bug: 593595
Comment on attachment 471906 [details] [diff] [review]
Part 3: Implement and use CreateOptimalSurface for CanvasLayerD3D9 v2


>+cairo_user_data_key_t gKeyD3D9Texture;
>+
>+void ReleaseTexture(void *texture)
>+{
>+  ((IDirect3DTexture9*)texture)->Release();
>+}

c++ style cast would be better.


>+  nsRefPtr<gfxD2DSurface> surface =
>+    new gfxD2DSurface(sharedHandle, aFormat == gfxASurface::ImageFormatRGB24 ?
>+      gfxASurface::CONTENT_COLOR : gfxASurface::CONTENT_COLOR_ALPHA);

Maybe CreateOptimalSurface should take a content instead of a surface. (I haven't thought about this...)?
Attachment #471906 - Flags: review?(jmuizelaar) → review+
These patchset brakes --enable-system-cairo: https://bugzilla.mozilla.org/show_bug.cgi?id=594322
You need to log in before you can comment on or make changes to this bug.