Closed Bug 595209 Opened 14 years ago Closed 14 years ago

scaling svg text causes it to be unreadable


(Core :: SVG, defect)

Not set



Tracking Status
blocking2.0 --- final+


(Reporter: neonstalwart, Assigned: longsonr)




(4 files, 6 obsolete files)

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv: Gecko/20100824 Firefox/3.6.9 ( .NET CLR 3.5.30729)
Build Identifier:  Mozilla/5.0 (Windows NT 5.1; rv:2.0b5) Gecko/20100101 Firefox/4.0b5

using dojo's gfx library (dojox.gfx) i create an svg surface and add some text to it.  when the surface is resized, the text is unreadable - blurry.

Reproducible: Always

Steps to Reproduce:
1. load the attached test file (note the text 'abc' is clear)
2. click the 'click to resize' button (note the text is very blurry)
3. click the 'click to resize' button again (note the text is clear)
Actual Results:  
after the first resize, the text is very blurry.

Expected Results:  
the text should be clear
You should probably use fill rather than stroke to draw text.
Component: General → SVG
Product: Firefox → Core
QA Contact: general → general
Even allowing for that the trunk display is odd.
minimal testcase without the dojo goop would be nice...
Keywords: qawanted
Attached file testcase, no dojo (obsolete) —
a test case without dojo.  it seems the cause of the problem is the combination of using stroke and rotate on the text element.  commenting out the lines for either one causes the expected result but the combination causes the error.
sorry... left a console.log in the last one which meant that firebug had to be enabled to see this working.  removed the console.log so you don't need firebug enabled
Attachment #474323 - Attachment is obsolete: true
Thanks, confirming.  That click handler should really be idempotent; the fact that it's not is totally bogus.
blocking2.0: --- → ?
Ever confirmed: true
Keywords: qawanted
We call CharacterIterator::SetupFor() which rescales stroke-width in nsSVGGlyphFrame::AddCharactersToPath() for each glyph.
As a result we rescale stroke-width multiple times.
OS: Windows XP → All
Hardware: x86 → All
Version: unspecified → Trunk
Attached patch Reftest (obsolete) — Splinter Review
Convert the attachment to reftest
Attachment #478712 - Flags: review?(longsonr)
Attached patch Patch (obsolete) — Splinter Review
Add Save()/Restore() for each iteration.
Attachment #478713 - Flags: review?(longsonr)
Hmm.  So with this patch we'll save/restore for every single glyph in SVG text?  Is that reasonably cheap?
Echoing bz, why can't we Save/Restore outside the loops?
Attachment #478712 - Flags: review?(longsonr) → review+
Attached patch Patch (obsolete) — Splinter Review
Yeah, we should save/restore outside the loop.
Attachment #478713 - Attachment is obsolete: true
Attachment #478751 - Flags: review?(longsonr)
Attachment #478713 - Flags: review?(longsonr)
Why doesn't this affect textPaths or text with multiple lengths or rotations. If it does then your Save/Restore needs to go round the DirectTextRun... calls too.

On top of that can you use gfxContextAutoSaveRestore, then you won't need the restore call. See nsSVGInnerSVGFrame.cpp for usage.
Attachment #478751 - Flags: review?(longsonr) → review-
QA Contact: general → longsonr
Blocks: 580871
Attached patch patch (obsolete) — Splinter Review
When we go via the placed glyphs path - always for text-paths or as in the example when there is a rotation, we apply the line width scaling factor for each character rather than just once.

This bug has been around for a very long time, it's much more noticable now that we use the place glyphs code patch for ordinary text rather than just text-paths though.
Attachment #478751 - Attachment is obsolete: true
Attachment #486562 - Flags: review?(roc)
Attached patch patch (obsolete) — Splinter Review
part of another patch crept into the previous version
Attachment #486562 - Attachment is obsolete: true
Attachment #486563 - Flags: review?(roc)
Attachment #486562 - Flags: review?(roc)
Assignee: nobody → longsonr
QA Contact: longsonr → general
Attachment #486563 - Attachment is obsolete: true
Attachment #478712 - Attachment is obsolete: true
Keywords: checkin-needed
Whiteboard: [c-n: after 2.0b7 freeze][2 patches to check in]
Blocks: 521407
Landed patch:
and tests:
Closed: 14 years ago
Flags: in-testsuite+
Keywords: checkin-needed
Resolution: --- → FIXED
Whiteboard: [c-n: after 2.0b7 freeze][2 patches to check in]
Target Milestone: --- → mozilla2.0b8
You need to log in before you can comment on or make changes to this bug.