Closed
Bug 959128
Opened 10 years ago
Closed 10 years ago
incorrect path transform being used within SVG-in-OpenType glyph
Categories
(Core :: SVG, defect)
Core
SVG
Tracking
()
VERIFIED
FIXED
mozilla31
People
(Reporter: jfkthame, Assigned: jwatt)
References
Details
(Keywords: regression)
Attachments
(1 file, 1 obsolete file)
1.74 KB,
patch
|
heycam
:
review+
Sylvestre
:
approval-mozilla-aurora+
Sylvestre
:
approval-mozilla-beta+
|
Details | Diff | Splinter Review |
(Reported to us by Chris Lilley - thanks!) It looks like bug 929350 has broken rendering within SVG-in-OT glyphs, depending how transforms are used. See example at http://people.mozilla.org/~jkew/opentype-svg/soccer.html. The black patches on the soccer ball render fine in Firefox 26, but are broken (totally mis-scaled) in FF27 beta and later versions, including current trunk. To examine the SVG data that's embedded in the font, download the font file and then load it into Roc's SVG OpenType Workshop tool. Bisecting with mozilla-central nightlies leads to: 2012-10-22 OK http://hg.mozilla.org/mozilla-central/rev/177bf37a49f5 2012-10-23 BAD http://hg.mozilla.org/mozilla-central/rev/21d97baadc05 http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=177bf37a49f5&tochange=21d97baadc05 I strongly suspect: 041018f17534 Jonathan Watt — Bug 929350 - Allow the context for gfxContextMatrixAutoSaveRestore to be set lazily. r=mattwoodrow [1] http://people.mozilla.org/~jkew/opentype-svg/soccer.otf [2] Available from https://github.com/rocallahan/svg-opentype-workshop
Reporter | ||
Comment 1•10 years ago
|
||
jwatt: any chance you could look into this, given that it was apparently triggered by bug 929350? Is backing that out an option, if there's not a straightforward fix here?
Flags: needinfo?(jwatt)
Reporter | ||
Comment 2•10 years ago
|
||
Ah, I think it's actually bug 929364 that triggers the problem here. In trying to back out bug 929350 locally, I also ended up effectively undoing one part of bug 929364, and that's the bit that matters.
Keywords: regression
Assignee | ||
Comment 3•10 years ago
|
||
Bug 929364 seems like a much more likely culprit. I'll look tomorrow. Which part did you back out that fixed things for you?
Flags: needinfo?(jwatt)
Reporter | ||
Comment 4•10 years ago
|
||
Here's the experimental backout that fixed things for me. Note that gfxContextMatrixAutoSaveRestore is no longer lazy, so this effectively removes the special-case handling of the renderMode == SVGAutoRenderState::CLIP case in nsSVGPathGeometryFrame::Render; I suspect that's the part that matters here.
Reporter | ||
Updated•10 years ago
|
Assignee: nobody → jfkthame
Reporter | ||
Comment 5•10 years ago
|
||
Unassigning myself; that was an oversight when attaching the above patch. :jwatt, had any chance to look into this?
Assignee: jfkthame → nobody
Assignee | ||
Updated•10 years ago
|
QA Contact: jwatt
Assignee | ||
Updated•10 years ago
|
Assignee: nobody → jwatt
QA Contact: jwatt
Assignee | ||
Updated•10 years ago
|
Assignee | ||
Comment 6•10 years ago
|
||
I'm giving up on fixing this for the current code. Let's just revert back to the old behavior. I'll fix this properly in bug 987193 once we're in a position to switch clipPath to Moz2D.
Attachment #8368024 -
Attachment is obsolete: true
Attachment #8395743 -
Flags: review?(cam)
Comment 7•10 years ago
|
||
Comment on attachment 8395743 [details] [diff] [review] patch to revert back to setting the transform for CLIP Review of attachment 8395743 [details] [diff] [review]: ----------------------------------------------------------------- ::: layout/svg/nsSVGPathGeometryFrame.cpp @@ +610,5 @@ > // gfxContext and using Moz2D directly. Not bothering to Save()/Restore() > // is actually okay, since we know that doesn't matter in the > // SVGAutoRenderState::CLIP case (at least for the current implementation). > gfxContextMatrixAutoSaveRestore autoSaveRestore; > + // For now revent back do doing the save even for CLIP to fix bug 959128. s/do/to/
Attachment #8395743 -
Flags: review?(cam) → review+
Assignee | ||
Comment 8•10 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/ce59cf46f524
Assignee | ||
Updated•10 years ago
|
status-firefox28:
--- → wontfix
status-firefox29:
--- → affected
status-firefox30:
--- → affected
tracking-firefox29:
--- → ?
tracking-firefox30:
--- → ?
Updated•10 years ago
|
https://hg.mozilla.org/mozilla-central/rev/ce59cf46f524
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla31
Assignee | ||
Comment 10•10 years ago
|
||
Comment on attachment 8395743 [details] [diff] [review] patch to revert back to setting the transform for CLIP [Approval Request Comment] Bug caused by (feature/regressing bug #): bug 929364 User impact if declined: we've been fighting against SVG Fonts, promoting SVG-in-OpenType as a better alternative. So we really want this to work again ASAP. Besides that there's general SVG brokenness (bug 976830 that has also been reported) Testing completed (on m-c, etc.): landed on m-c Risk to taking this patch (and alternatives if risky): very low risk String or IDL/UUID changes made by this patch: none
Attachment #8395743 -
Flags: approval-mozilla-beta?
Attachment #8395743 -
Flags: approval-mozilla-aurora?
Updated•10 years ago
|
Attachment #8395743 -
Flags: approval-mozilla-beta?
Attachment #8395743 -
Flags: approval-mozilla-beta+
Attachment #8395743 -
Flags: approval-mozilla-aurora?
Attachment #8395743 -
Flags: approval-mozilla-aurora+
Assignee | ||
Comment 11•10 years ago
|
||
https://hg.mozilla.org/releases/mozilla-beta/rev/886f105345ce https://hg.mozilla.org/releases/mozilla-aurora/rev/2f010596adf2
Comment 12•10 years ago
|
||
Verified as fixed on Firefox 29.0b3 and the latest Firefox 30.0a2 and 31.0a1. Tested on Windows 7 64bit, Mac OS X 10.9 and Ubuntu 13.04 x86.
Status: RESOLVED → VERIFIED
Keywords: verifyme
Updated•10 years ago
|
You need to log in
before you can comment on or make changes to this bug.
Description
•