Open Bug 548072 Opened 15 years ago Updated 2 years ago

More efficient ImageSurface usage in gfxWindowsNativeDrawing

Categories

(Core :: Graphics, defect)

x86
Windows XP
defect

Tracking

()

UNCONFIRMED

People

(Reporter: bas.schouten, Unassigned)

References

Details

Attachments

(2 files)

Attached patch Fix native drawing (deleted) — 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)
Attachment #428503 - Flags: review?(jmathies)
Comment on attachment 428503 [details] [diff] [review] Fix native drawing looks good to me.
Attachment #428503 - Flags: review?(jmathies) → review+
Blocks: 548977
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-
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?
(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
Blocks: 549056
Attached patch Set CAN_DRAW_ALPHA (deleted) — Splinter Review
This patch can be used to draw native theme elements with the new efficient codepaths. Not sure on which platforms it works yet.
why this is not added on FF? or it is already added?
Jeff had a better idea for a patch :) But we didn't get to it yet.
Any progres on this?
Assignee: bas → nobody
Status: ASSIGNED → UNCONFIRMED
Ever confirmed: false
Severity: normal → S3
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: