918613 - Improve efficiency of using multiple paths with Cairo Moz2D

Status: RESOLVED FIXED

(Core :: Graphics, defect)

Description

[Bas Schouten (:bas.schouten)]

Attachment: Use cairo_path_t and only copy to a context when needed

Right now the cairo code does some complex things to try and work with a path on the current context. We believe that in retrospect this might not have been the right approach, we should try using a cairo_path_t directly.

The patch in here is still a little bit of a work in progress, some things could probably be done a little more efficient fairly easily.

Comment 1

[Bas Schouten (:bas.schouten)]

Attachment: Use cairo_path_t and only copy to a context when needed v2

This patch passes all the tests on try.

Comment 2

[Jeff Muizelaar [:jrmuizel]]

What's with the CopyGlyphsToBuilder signature change?

Comment 3

[Mike Conley (:mconley) (:⚙️)]

Marking this for our Australis tracker, since it seems this will likely address bug 919656.

Comment 4

[Bas Schouten (:bas.schouten)]

(In reply to Jeff Muizelaar [:jrmuizel] from comment #2)
> What's with the CopyGlyphsToBuilder signature change?

Depending on the transform we get different glyphs outlines from Cairo when using GDI. This used to sort of 'automatically' work because the context we were setting the path on was usually the context of our actual DrawTarget. And as such already had the target transform set. This adjustment lets us explicitly specify the transform we want to draw the glyph with, which will guarantee the right behavior with Cairo.

Comment 5

[Jeff Muizelaar [:jrmuizel]]

Comment on attachment 810528 [details] [diff] [review]
Use cairo_path_t and only copy to a context when needed v2

Review of attachment 810528 [details] [diff] [review]:
-----------------------------------------------------------------

::: gfx/2d/ScaledFontBase.cpp
@@ +81,2 @@
>  
> +	bool isNewContext = !ctx;

weird indentation here?

Comment 6

[Bas Schouten (:bas.schouten)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/3287d7bbdc14

Comment 7

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/3287d7bbdc14

Comment 8

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/e1226725f674

Comment 9

[Ryan VanderMeulen [:RyanVM]]

(In reply to Carsten Book [:Tomcat] from comment #8)
> https://hg.mozilla.org/mozilla-central/rev/e1226725f674

This was actually bug 918163.