Closed
Bug 342366
Opened 18 years ago
Closed 18 years ago
make cairo gfx faster (on Windows)
Categories
(Core :: Graphics, defect)
Tracking
()
RESOLVED
FIXED
People
(Reporter: pavlov, Assigned: pavlov)
References
Details
(Keywords: perf)
Attachments
(1 file, 7 obsolete files)
(deleted),
patch
|
Details | Diff | Splinter Review |
i'm going to just put various patches in this bug for cairo speedups
Assignee | ||
Comment 1•18 years ago
|
||
If our transform is identity then we don't need to call SetWorldTransform whenever we select text in (assuming that there were no earlier calls to SetWorldTransform that we should be overwriting, which there aren't, at least in Mozilla). This gives a reasonable Tp win -- more if we could wire up some places to avoid calling SaveDC/RestoreDC when this is the case.
Assignee: nobody → pavlov
Status: NEW → ASSIGNED
The memcmp is a little ugly; I almost think we should be calling some sort of non-existent cairo_matrix_is_identity. Is it faster to translate than it is to call SWT? If so you can do _cairo_matrix_is_integer_translation and translate by the returned x,y (or do nothing if they're 0,0).
Assignee | ||
Comment 3•18 years ago
|
||
this is better, although doesn't do what you were suggesting. I think i'd rather have a (hopefully) inline memcmp than calling out to some other cairo function.
Attachment #226577 -
Attachment is obsolete: true
Assignee | ||
Comment 4•18 years ago
|
||
This is a quick fix to avoid calling save/restoredc if we aren't modifying the worldtransform. This can be taken a step or two further by taking seperate code paths entirely if the color is solid, no scale or rotation is set, etc by just using ExtTextOut directly but we currently have no way to get the color.
Attachment #228484 -
Flags: review?(vladimir)
Comment on attachment 228484 [details] [diff] [review]
avoid save/restore in fast path (checked in)
This looks great -- one question though, can't you still use this even if the matrix is not kIdentity, if it just has a translation? (even just integer translation.) It won't affect measuring, and the x/y offset will get handled anyway because you're still using cairo to draw.
So you might be able to use !HasNonTranslation() here instead of just == kIdentity.
Attachment #228484 -
Flags: review?(vladimir) → review+
Assignee | ||
Comment 6•18 years ago
|
||
Attachment #228733 -
Flags: review?(vladimir)
Attachment #228733 -
Flags: review?(vladimir) → review+
Assignee | ||
Updated•18 years ago
|
Attachment #228484 -
Attachment description: avoid save/restore in fast path → avoid save/restore in fast path (checked in)
Attachment #228484 -
Attachment is obsolete: true
Assignee | ||
Updated•18 years ago
|
Attachment #228733 -
Attachment description: don't copy metrics for no reason → don't copy metrics for no reason (checked in)
Attachment #228733 -
Attachment is obsolete: true
Assignee | ||
Comment 7•18 years ago
|
||
Attachment #228879 -
Flags: review?(vladimir)
Comment on attachment 228879 [details] [diff] [review]
avoid save/restoredc in nativetheme code
I think you should be checking whether the translation is an integer or not. The old code didn't, because it assumed that SWT would do the right thing. The coords you pass in are integers, and the matrix could well have a non-integer translation.
Assignee | ||
Comment 9•18 years ago
|
||
remove save/restore/cairo/etc from the ComputeMetrics function as none of it is actually needed. Also includes a fix to the call to GetGlyphOutlineW to pass in a proper matrix (thanks to Steve Swanson for finding that one in bug 324857)
Attachment #229002 -
Flags: review?(vladimir)
Assignee | ||
Updated•18 years ago
|
Summary: make cairo faster → make cairo gfx faster (on Windows)
Attachment #229002 -
Flags: review?(vladimir) → review+
Assignee | ||
Comment 10•18 years ago
|
||
Attachment #229208 -
Flags: review?(vladimir)
Comment on attachment 229208 [details] [diff] [review]
return cached space width when string is only a space (checked in)
r=me maybe with a comment saying that callers shouldn't measure " " but instead use the metrics?
Attachment #229208 -
Flags: review?(vladimir) → review+
Assignee | ||
Updated•18 years ago
|
Attachment #229208 -
Attachment description: return cached space width when string is only a space → return cached space width when string is only a space (checked in)
Attachment #229208 -
Attachment is obsolete: true
Assignee | ||
Updated•18 years ago
|
Attachment #229002 -
Attachment description: speed up ComputeMetrics → speed up ComputeMetrics (checked in)
Attachment #229002 -
Attachment is obsolete: true
Blocks: 334719
Assignee | ||
Comment 12•18 years ago
|
||
this avoids going through cairo for basic text drawing when we can
Assignee | ||
Updated•18 years ago
|
Attachment #232952 -
Flags: review?(vladimir)
Comment on attachment 232952 [details] [diff] [review]
speed up text rendering (checked in)
>+ /**
>+ * Get the source pattern (solid color, normal pattern, surface, etc)
>+ */
>+ already_AddRefed<gfxPattern> GetPattern();
>+already_AddRefed<gfxPattern>
>+gfxContext::GetPattern()
> {
>- cairo_set_source_surface(mCairo, surface->CairoSurface(), offset.x, offset.y);
>+ cairo_pattern_t *pat = cairo_get_source(mCairo);
>+ NS_ASSERTION(pat, "I was told this couldn't be null");
>+
>+ gfxPattern *wrapper = nsnull;
>+ if (pat)
>+ wrapper = new gfxPattern(pat);
>+ else
>+ wrapper = new gfxPattern(gfxRGBA(0,0,0,0));
>+
>+ NS_ADDREF(wrapper);
>+ return wrapper;
> }
I would add a
PRBool gfxContext::GetColor(gfxRGBA& currentColor)
that would return PR_FALSE if the current source is not a solid, and PR_TRUE and set currentColor appropriately otherwise. This lets you avoid allocating the new gfxPattern instance just to fetch the current color, which I think would be the 99% common usage.
Attachment #232952 -
Flags: review?(vladimir) → review+
Assignee | ||
Updated•18 years ago
|
Attachment #228879 -
Attachment is obsolete: true
Attachment #228879 -
Flags: review?(vladimir)
Assignee | ||
Updated•18 years ago
|
Attachment #232952 -
Attachment description: speed up text rendering → speed up text rendering (checked in)
Attachment #232952 -
Attachment is obsolete: true
Assignee | ||
Comment 14•18 years ago
|
||
gonna mark this fixed for now
Status: ASSIGNED → RESOLVED
Closed: 18 years ago
Resolution: --- → FIXED
Updated•18 years ago
|
You need to log in
before you can comment on or make changes to this bug.
Description
•