Closed Bug 342366 Opened 18 years ago Closed 18 years ago

make cairo gfx faster (on Windows)

Categories

(Core :: Graphics, defect)

x86
Windows XP
defect
Not set
normal

Tracking

()

RESOLVED FIXED

People

(Reporter: pavlov, Assigned: pavlov)

References

Details

(Keywords: perf)

Attachments

(1 file, 7 obsolete files)

i'm going to just put various patches in this bug for cairo speedups
Attached patch SetWorldTransform avoidance (obsolete) (deleted) — Splinter Review
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).
Attached patch SetWorldTransform avoidance 2 (deleted) — Splinter Review
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
Attached patch avoid save/restore in fast path (checked in) (obsolete) (deleted) — Splinter Review
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+
Attached patch don't copy metrics for no reason (checked in) (obsolete) (deleted) — Splinter Review
Attachment #228733 - Flags: review?(vladimir)
Attachment #228484 - Attachment description: avoid save/restore in fast path → avoid save/restore in fast path (checked in)
Attachment #228484 - Attachment is obsolete: true
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
Attached patch avoid save/restoredc in nativetheme code (obsolete) (deleted) — Splinter Review
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.
Attached patch speed up ComputeMetrics (checked in) (obsolete) (deleted) — Splinter Review
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)
Summary: make cairo faster → make cairo gfx faster (on Windows)
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+
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
Attachment #229002 - Attachment description: speed up ComputeMetrics → speed up ComputeMetrics (checked in)
Attachment #229002 - Attachment is obsolete: true
Attached patch speed up text rendering (checked in) (obsolete) (deleted) — Splinter Review
this avoids going through cairo for basic text drawing when we can
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+
Depends on: 348223
Attachment #228879 - Attachment is obsolete: true
Attachment #228879 - Flags: review?(vladimir)
Attachment #232952 - Attachment description: speed up text rendering → speed up text rendering (checked in)
Attachment #232952 - Attachment is obsolete: true
Depends on: 358757
Depends on: 359147
gonna mark this fixed for now
Status: ASSIGNED → RESOLVED
Closed: 18 years ago
Resolution: --- → FIXED
No longer blocks: 362118
Depends on: 362118
No longer depends on: 362118
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: