Open
Bug 1094247
Opened 10 years ago
Updated 2 years ago
Overhaul the way that we provide and propagate gfxTextContextPaint objects
Categories
(Core :: SVG, defect)
Core
SVG
Tracking
()
NEW
People
(Reporter: jwatt, Assigned: jwatt)
References
(Blocks 2 open bugs)
Details
The way that we provide and propagate gfxTextContextPaint objects is currently pretty busted and buggy.
* Since we only use gfxTextContextPaint objects during paint time,
we can fail to correctly calculate overflow areas for
|stroke-width:context-value;|, for example.
* Contexts can be nested. For example we can have a <use> element
(which is a context element) that uses content that contains
text that uses a font with SVG-in-OpenType glyphs. We make a
halfhearted effort to take account of this in SVGTextFrame, but
it's incomplete and we don't do it anywhere else.
Context properties may need to be obtained during reflow, overflow updates or painting.
One complication to obtaining these properties during ReflowSVG and UpdateOverflow is that these methods do not take any arguments that a context object could be added to. I don't think for this uncommon use case we should be adding one either.
One place that a stack of context objects might be built up is on the document, where we have a function called something like SetTemporaryStackBasedSVGPropertyContext that pushes a context every time we recurse passed a context element during reflow/update-overflow/painting...except, as dholbert pointed out, UpdateOverflow is not a recursive function, so that doesn't work.
Due to performance concerns I have been trying to avoid solving these issues by making code that needs context information look up the tree for the nearest nsSVGUseFrame/nsSVGPathGeometryFrame/document that has a context. But I think that's maybe what we should do first. If the performance concerns turn out to be real then we can try to come up with something else. (In general the number of ancestors between an element referencing context properties and the context element is likely to be small. At least for SVG-in-OT and <marker> content. Maybe less so for <use> and SVG-as-an-image.)
Assignee | ||
Comment 1•10 years ago
|
||
Another thing that is wrong is calling these context objects "text" context "paint". First, the contexts can come from <use>, geometry using <marker> and in the future from SVG-as-an-image contexts, not just text. Second, the 'context-value' keyword can be used with the 'stroke-width' property, and that's more geometry than paint (and relevant to reflow/overflow areas, not just painting). Going forward other non-paint properties may also allow context references.
To resolve the naming issues I'd propose calling the class SVGContextProps, or something.
Assignee | ||
Comment 3•9 years ago
|
||
One of the things that's required here is to get rid of SimpleTextContextPaint. That gfxTextContextPaint subclass depends on us having a gfxContext with a pattern set on it. (The other subclass, SVGTextContextPaint, is set up explicitly for a context (currently only SVG text elements), so presumably the bit of code that creates a SimpleTextContextPaint does so to pass the currentColor through to SVG-in-OpenType glyphs for HTML text). We should probably flatten these classes into a single class, and propagate the full context info properly.
Assignee | ||
Comment 4•8 years ago
|
||
Note the patch for bug 1290781 made us set the context paint on the document instead of on a DrawTarget.
Depends on: 1290781
Updated•2 years ago
|
Severity: normal → S3
You need to log in
before you can comment on or make changes to this bug.
Description
•