Improve efficiency of using multiple paths with Cairo Moz2D

RESOLVED FIXED in mozilla27

Status

()

Core
Graphics
RESOLVED FIXED
5 years ago
4 years ago

People

(Reporter: bas, Assigned: bas)

Tracking

unspecified
mozilla27
x86_64
Windows 8
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [Australis:M?][Australis:P1])

Attachments

(1 attachment, 1 obsolete attachment)

(Assignee)

Description

5 years ago
Created attachment 807553 [details] [diff] [review]
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.
Attachment #807553 - Flags: feedback?(jmuizelaar)
(Assignee)

Comment 1

5 years ago
Created attachment 810528 [details] [diff] [review]
Use cairo_path_t and only copy to a context when needed v2

This patch passes all the tests on try.
Attachment #807553 - Attachment is obsolete: true
Attachment #807553 - Flags: feedback?(jmuizelaar)
Attachment #810528 - Flags: review?(jmuizelaar)
(Assignee)

Updated

5 years ago
Blocks: 919471
(Assignee)

Updated

5 years ago
Blocks: 919656
What's with the CopyGlyphsToBuilder signature change?
Marking this for our Australis tracker, since it seems this will likely address bug 919656.
Whiteboard: [Australis:M?][Australis:P1]
(Assignee)

Comment 4

5 years ago
(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.
Blocks: 920744
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?
Attachment #810528 - Flags: review?(jmuizelaar) → review+
https://hg.mozilla.org/mozilla-central/rev/3287d7bbdc14
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla27
Depends on: 924749
(In reply to Carsten Book [:Tomcat] from comment #8)
> https://hg.mozilla.org/mozilla-central/rev/e1226725f674

This was actually bug 918163.

Updated

5 years ago
Duplicate of this bug: 919471

Updated

5 years ago
Depends on: 943351
Depends on: 969203
No longer depends on: 969203

Updated

4 years ago
Depends on: 976153
You need to log in before you can comment on or make changes to this bug.