scaling svg text causes it to be unreadable

RESOLVED FIXED in mozilla2.0b8

Status

()

Core
SVG
RESOLVED FIXED
8 years ago
8 years ago

People

(Reporter: ben hockey, Assigned: longsonr)

Tracking

Trunk
mozilla2.0b8
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(blocking2.0 final+)

Details

Attachments

(4 attachments, 6 obsolete attachments)

(Reporter)

Description

8 years ago
User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.2.9) 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
(Reporter)

Comment 1

8 years ago
Created attachment 474081 [details]
a self contained test case to demonstrate the problem
(Assignee)

Comment 2

8 years ago
You should probably use fill rather than stroke to draw text.
(Assignee)

Updated

8 years ago
Component: General → SVG
Product: Firefox → Core
QA Contact: general → general
(Assignee)

Comment 3

8 years ago
Even allowing for that the trunk display is odd.
minimal testcase without the dojo goop would be nice...
Keywords: qawanted
(Reporter)

Comment 5

8 years ago
Created attachment 474323 [details]
testcase, no dojo

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.
(Reporter)

Comment 6

8 years ago
Created attachment 474324 [details]
testcase, no dojo (without console.log)

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.
Status: UNCONFIRMED → NEW
blocking2.0: --- → ?
Ever confirmed: true
Keywords: qawanted
blocking2.0: ? → final+

Comment 8

8 years ago
We call CharacterIterator::SetupFor() which rescales stroke-width in nsSVGGlyphFrame::AddCharactersToPath() for each glyph.
As a result we rescale stroke-width multiple times.

http://mxr.mozilla.org/mozilla-central/source/layout/svg/base/src/nsSVGGlyphFrame.cpp#1639
OS: Windows XP → All
Hardware: x86 → All
Version: unspecified → Trunk

Comment 9

8 years ago
Created attachment 478712 [details] [diff] [review]
Reftest

Convert the attachment to reftest
Attachment #478712 - Flags: review?(longsonr)

Comment 10

8 years ago
Created attachment 478713 [details] [diff] [review]
Patch

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?
(Assignee)

Comment 12

8 years ago
Echoing bz, why can't we Save/Restore outside the loops?
(Assignee)

Updated

8 years ago
Attachment #478712 - Flags: review?(longsonr) → review+

Comment 13

8 years ago
Created attachment 478751 [details] [diff] [review]
Patch

Yeah, we should save/restore outside the loop.
Attachment #478713 - Attachment is obsolete: true
Attachment #478751 - Flags: review?(longsonr)
Attachment #478713 - Flags: review?(longsonr)
(Assignee)

Comment 14

8 years ago
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.
(Assignee)

Updated

8 years ago
Attachment #478751 - Flags: review?(longsonr) → review-
(Assignee)

Updated

8 years ago
QA Contact: general → longsonr
(Assignee)

Updated

8 years ago
Blocks: 580871
(Assignee)

Comment 15

8 years ago
Created attachment 486562 [details] [diff] [review]
patch

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)
(Assignee)

Comment 16

8 years ago
Created attachment 486563 [details] [diff] [review]
patch

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
(Assignee)

Comment 17

8 years ago
Created attachment 486863 [details] [diff] [review]
hg changeset patch
Attachment #486563 - Attachment is obsolete: true
(Assignee)

Comment 18

8 years ago
Created attachment 486864 [details] [diff] [review]
hg changeset patch
Attachment #478712 - Attachment is obsolete: true
(Assignee)

Updated

8 years ago
Keywords: checkin-needed
Whiteboard: [c-n: after 2.0b7 freeze][2 patches to check in]
(Assignee)

Updated

8 years ago
Blocks: 521407
Landed patch: http://hg.mozilla.org/mozilla-central/rev/c428371144df
and tests:    http://hg.mozilla.org/mozilla-central/rev/ddfa6625b645
Status: NEW → RESOLVED
Last Resolved: 8 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.