Open
Bug 548072
Opened 15 years ago
Updated 2 years ago
More efficient ImageSurface usage in gfxWindowsNativeDrawing
Categories
(Core :: Graphics, defect)
Tracking
()
UNCONFIRMED
People
(Reporter: bas.schouten, Unassigned)
References
Details
Attachments
(2 files)
(deleted),
patch
|
jrmuizel
:
review-
jimm
:
review+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
Details | Diff | Splinter Review |
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.
Attachment #428503 -
Flags: review?(jmuizelaar)
Reporter | ||
Updated•15 years ago
|
Attachment #428503 -
Flags: review?(jmathies)
Comment 1•15 years ago
|
||
Comment on attachment 428503 [details] [diff] [review]
Fix native drawing
looks good to me.
Attachment #428503 -
Flags: review?(jmathies) → review+
Comment 2•15 years ago
|
||
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.
Attachment #428503 -
Flags: review?(jmuizelaar) → review-
Comment 3•15 years ago
|
||
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?
Reporter | ||
Comment 4•15 years ago
|
||
(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
Reporter | ||
Comment 5•15 years ago
|
||
This patch can be used to draw native theme elements with the new efficient codepaths. Not sure on which platforms it works yet.
Reporter | ||
Comment 7•14 years ago
|
||
Jeff had a better idea for a patch :) But we didn't get to it yet.
Reporter | ||
Updated•4 years ago
|
Assignee: bas → nobody
Status: ASSIGNED → UNCONFIRMED
Ever confirmed: false
Updated•2 years ago
|
Severity: normal → S3
You need to log in
before you can comment on or make changes to this bug.
Description
•