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)

defect
Not set
normal

Tracking

()

VERIFIED FIXED
mozilla31
Tracking Status
firefox28 --- wontfix
firefox29 + verified
firefox30 + verified
firefox31 --- verified

People

(Reporter: jfkthame, Assigned: jwatt)

References

Details

(Keywords: regression)

Attachments

(1 file, 1 obsolete file)

(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
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)
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
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)
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.
Assignee: nobody → jfkthame
Unassigning myself; that was an oversight when attaching the above patch. :jwatt, had any chance to look into this?
Assignee: jfkthame → nobody
QA Contact: jwatt
Assignee: nobody → jwatt
QA Contact: jwatt
Blocks: 929364
No longer blocks: 929350
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 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+
Blocks: 976830
https://hg.mozilla.org/mozilla-central/rev/ce59cf46f524
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla31
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?
Attachment #8395743 - Flags: approval-mozilla-beta?
Attachment #8395743 - Flags: approval-mozilla-beta+
Attachment #8395743 - Flags: approval-mozilla-aurora?
Attachment #8395743 - Flags: approval-mozilla-aurora+
Keywords: verifyme
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.
Depends on: 993087
Blocks: 993087
No longer depends on: 993087
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: