548072 - More efficient ImageSurface usage in gfxWindowsNativeDrawing

Status: UNCONFIRMED

(Core :: Graphics, defect)

Description

[Bas Schouten (:bas.schouten)]

Attachment: Fix native drawing

Currently even if the CAN_DRAW_TO_COLOR_ALPHA flag is passed to gfxWindowsNativeDrawing we do double pass drawing and then perform alpha recovery. In the case of the CAN_DRAW_TO_COLOR_ALPHA bit being set we should actually just hand an ARGB32 DIB and then paint that to the context directly. Saving us a drawing pass and alpha recovery.

The IsDoublePass function is currently returning incorrect results for none-Win32 surfaces.

I've attached a patch which is aimed and fixing all these issues.

On a related note we currently pass CANNOT_DRAW_TO_COLOR_ALPHA in nsNativeThemeWin, this, atleast on Windows 7, appears incorrect. We need to figure out in what cases this is CAN and in what cases CANNOT, so that we can avoid alpha recovery on operating systems which CAN. This is important for both Layers and D2D, to avoid alpha recovery overhead when not needed.

Comment 1

[Jim Mathies [:jimm]]

Comment on attachment 428503 [details] [diff] [review]
Fix native drawing

looks good to me.

Comment 2

[Jeff Muizelaar [:jrmuizel]]

Comment on attachment 428503 [details] [diff] [review]
Fix native drawing

This patch adds quite a bit of complexity to gfxWindowsNativeDrawing.cpp. We have a bunch more states and the image surfaces ones seem to mirror the native drawing ones.

I wonder if we could re-architect this so that we just do native drawing with the native drawing states on to a gfxWindowsSurface()


> void
> gfxWindowsNativeDrawing::PaintToContext()
> {
>@@ -297,16 +330,38 @@ gfxWindowsNativeDrawing::PaintToContext(
>         if (mNativeDrawFlags & DO_NEAREST_NEIGHBOR_FILTERING)
>             pat->SetFilter(gfxPattern::FILTER_FAST);
> 
>         mContext->SetPattern(pat);
>         mContext->Fill();
>         mContext->Restore();
> 
>         mRenderState = RENDER_STATE_DONE;
>+    } else if (mRenderState == RENDER_STATE_IMAGESURFACE_DRAWING_DONE) {
>+        nsRefPtr<gfxImageSurface> surface = mWinSurface->GetImageSurface();
>+
>+        mContext->Save();
>+        mContext->Translate(mNativeRect.pos);
>+        mContext->NewPath();
>+        mContext->Rectangle(gfxRect(gfxPoint(0.0, 0.0), mNativeRect.size));
>+
>+        nsRefPtr<gfxPattern> pat = new gfxPattern(surface);
>+
>+        gfxMatrix m;
>+        m.Scale(mScale.width, mScale.height);
>+        pat->SetMatrix(m);
>+
>+        if (mNativeDrawFlags & DO_NEAREST_NEIGHBOR_FILTERING)
>+            pat->SetFilter(gfxPattern::FILTER_FAST);
>+
>+        mContext->SetPattern(pat);
>+        mContext->Fill();
>+        mContext->Restore();
>+
>+        mRenderState = RENDER_STATE_DONE;

This duplicates all of the drawing code when the only difference is the pattern used.

Comment 3

[Jeff Muizelaar [:jrmuizel]]

I was thinking about this more last night and wondered if there was any reason we would want to use an image surface for this instead of a gdi interop d2d render target?

Comment 4

[Bas Schouten (:bas.schouten)]

(In reply to comment #3)
> I was thinking about this more last night and wondered if there was any reason
> we would want to use an image surface for this instead of a gdi interop d2d
> render target?

We could, for D2D, but it would require another logic for Layers without D2D. We'd need to create a special D2D surface for it too, since you need to specifically create a surface for it using a special flag to be able to do GDI interop. That flag doesn't work with dxgi render targets (the ones we use for our D3D textures). See: http://msdn.microsoft.com/en-us/library/dd370971%28VS.85%29.aspx

Comment 5

[Bas Schouten (:bas.schouten)]

Attachment: Set CAN_DRAW_ALPHA

This patch can be used to draw native theme elements with the new efficient codepaths. Not sure on which platforms it works yet.

Comment 6

[Adrian]

why this is not added on FF? or it is already added?

Comment 7

[Bas Schouten (:bas.schouten)]

Jeff had a better idea for a patch :) But we didn't get to it yet.

Comment 8

[Tomas]

Any progres on this?