Closed Bug 877957 Opened 11 years ago Closed 11 years ago

do not use a font size scale factor when painting text into a mask, clipPath, etc.

Categories

(Core :: SVG, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla25

People

(Reporter: heycam, Assigned: heycam)

References

Details

Attachments

(1 file)

In theory, we should be reflowing our nsSVGTextFrame2's anonymous block frame every time we paint it into a <mask>, <clipPath>, etc., since the text could be used in contexts with different transforms, and we'd therefore want to select a different font sizes for the text runs we create (in UpdateFontSizeScaleFactor). But I think it's a good thing we don't this, because it would suck for performance. Currently, whatever <mask> frame happens to get painted first will be the one that decides what the font size scale factor is for reflowing the anonymous block child. This is not terrible, but it seems a bit wrong. Robert, what do you think about either setting mFontSizeScaleFactor = 1, or doing the equivalent of text-rendering:geometricPrecision (where we try to target a text run font size of 200), to avoid these differences? The alternative is to come up with a system where we can keep multiple anonymous block frame children around -- one for each element that uses the <mask>, etc. This seems like a lot of work, so I'd like to avoid it if possible.
mFontSizeScaleFactor = 1 seems fine. geometricPrecision will make the text look somewhat different but not unreasonably so I guess. I don't think the old text code recalculates does it?
(In reply to Robert Longson from comment #1) > mFontSizeScaleFactor = 1 seems fine. geometricPrecision will make the text > look somewhat different but not unreasonably so I guess. Hmm, I think it'd be better to look for geometricPrecision first before returning 1 early. And I guess all we want to do is avoid depending on the context scale. So maybe it would be better just to avoid calling GetCanvasTM when we are non-display? > I don't think the old text code recalculates does it? No it doesn't.
Attached patch patch (deleted) — Splinter Review
Like this.
Assignee: nobody → cam
Status: NEW → ASSIGNED
Attachment #771822 - Flags: review?(longsonr)
Comment on attachment 771822 [details] [diff] [review] patch > // The context scale is the ratio of the length of the transformed > // diagonal vector (1,1) to the length of the untransformed diagonal > // (which is sqrt(2)). >- gfxPoint p = m.Transform(gfxPoint(1, 1)) - m.Transform(gfxPoint(0, 0)); >- double contextScale = SVGContentUtils::ComputeNormalizedHypotenuse(p.x, p.y); >+ // >+ // But when we are non-display, we could be painted in different coordinate >+ // spaces, and we don't want to have to reflow for each of these. We >+ // just assume that the context scale is 1.0 for them all, so we don't >+ // get stuck with a font size scale factor based on whichever referencing >+ // frame happens to reflow first. >+ double contextScale = 1.0; >+ if (!(mState & NS_STATE_SVG_NONDISPLAY_CHILD)) { >+ gfxMatrix m(GetCanvasTM(mGetCanvasTMForFlag)); >+ if (!m.IsSingular()) { >+ gfxPoint p = m.Transform(gfxPoint(1, 1)) - m.Transform(gfxPoint(0, 0)); >+ contextScale = SVGContentUtils::ComputeNormalizedHypotenuse(p.x, p.y); >+ } >+ } Aren't you introducing a function for this in the textLength bug. Either land that first and use the function or move the function to this bug and use it here. r=longsonr with that
Attachment #771822 - Flags: review?(longsonr) → review+
Just noticed you landed bug 569722 so you should use the GetContextScale you added in that bug.
Blocks: 890756
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla25
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: