Closed
Bug 387969
Opened 17 years ago
Closed 17 years ago
Need a way to control text rendering quality
Categories
(Core :: Layout, defect)
Core
Layout
Tracking
()
RESOLVED
FIXED
People
(Reporter: roc, Assigned: roc)
References
Details
(Keywords: dev-doc-complete)
Attachments
(1 file)
32.40 KB,
patch
|
pavlov
:
review+
bzbarsky
:
superreview+
|
Details | Diff | Splinter Review |
We need a way to control quality vs speed. This could include some combination of
1) pref UI
2) heuristics
3) CSS, e.g. SVG text-rendering property:
http://www.w3.org/TR/SVG11/painting.html#TextRenderingProperty
See discussion at
http://groups.google.com/group/mozilla.dev.platform/browse_thread/thread/8a4be4f147f01f37/021d7bf84c8b248a#021d7bf84c8b248a
My current proposal is:
-- implement SVG text-rendering property
-- interpret "optimizeLegibility" and "geometricPrecision" as turning on ligatures, kerning etc
-- interpret "optimizeSpeed" as taking whatever fast paths the platform supports (possibly turning off ligatures, kerning etc)
-- interpret "auto" as "take quality path for text over a certain fixed size, fast path otherwise" (the size being a hidden pref)
-- set textboxes and XUL elements to use text-rendering:optimizeLegibility.
That will do for now. At some point we could add a preference, I would suggest under Fonts And Colors, that sets text-rendering on content-document root elements; it would be implemented very much the same way as we set the background color preference (a magic style rule set up by nsPresShell).
Flags: blocking1.9?
Assignee | ||
Comment 1•17 years ago
|
||
It turns out that text-rendering was already added to the style system when SVG
landed, although it's not used for anything as far as I can tell. That makes
this a lot easier...
Comment 2•17 years ago
|
||
For textboxes, do you also mean textareas? I'd be worried about editing performance there.
Assignee | ||
Comment 3•17 years ago
|
||
Yes, I also mean textareas. I don't forsee any problems with editing performance, what are you concerned about?
Comment 4•17 years ago
|
||
I'm concerned about whether it'll cost more to construct the slower textruns and if that's something we'll do a lot as a user types in a textarea. So basically the usual textarea performance worry.
If it's misplaced, great.
Comment 5•17 years ago
|
||
It's probably already on your radar, but the value of the text-rendering property would be something which would make the same sequence of unicode codepoints render with different glyphs, so we would need to add it to the textrun word cache hash key (as in bug 3863339 comment 6).
Assignee | ||
Comment 6•17 years ago
|
||
> I'm concerned about whether it'll cost more to construct the slower textruns
> and if that's something we'll do a lot as a user types in a textarea.
We will, but we'll typically be dealing with relatively small quantities of text. For Tp/Tp2 we're concerned about millisecond slowdowns when constructing textruns for multi-kilobyte text; that shouldn't be an issue for most editing cases. Also the textrun word cache should work well for editing because everything will be cached except for the word that's actually changing.
> so we would need to add it to the textrun word cache hash key
Yes.
Assignee | ||
Comment 7•17 years ago
|
||
> Also the textrun word cache should work well for editing because
> everything will be cached except for the word that's actually changing.
In fact this point alone pretty much ensures there will be no significant impact to interactive editing.
I don't understand -- why do we want optimizeLegibility for textareas? That seems to be the least interesting place to use that; XUL elements certainly, and I'd have thebes watch the pref and do size-based enabling of that, unless explicitly overridden (so we'd need some way to distinguish default-optimizeSpeed vs. page-specified-optimizeSpeed in thebes land).
What about discretionary ligatures? Or are we not doing anything with that for now? (I really like the Zapfino demo!)
Assignee | ||
Comment 9•17 years ago
|
||
I was thinking of adding a single textrun construction flag to control speed vs quality and then setting that flag in layout, using a utility function in nsLayoutUtils (maybe nsContentUtils for easier pref watching) to compute the flag value based on CSS style wherever we want to honor text-rendering --- nsTextFrameThebes, and maybe SVG itself, but that's about all we care about really. Everything that's still using nsThebesFontMetrics/nsIRenderingContext will get high quality only, at least for now.
> What about discretionary ligatures?
We should use them wherever we can when the flag is set for quality. I'm not sure what platform changes are required, if any.
> I don't understand -- why do we want optimizeLegibility for textareas?
Because there typically isn't too much text in textareas, and because demos like Zapfino depend on it. It's also good for debugging. You might think these are spurious reasons, but making it easier for authors to experiment with this stuff is important, I think.
(In reply to comment #9)
> > What about discretionary ligatures?
>
> We should use them wherever we can when the flag is set for quality. I'm not
> sure what platform changes are required, if any.
Ah, I don't think we should do that -- they change the text too much for us to do it in all cases. For example, the Zapfino demo is cool, but it would be very annoying if you weren't expecting it (especially in a textbox!). I think we need to add an explicit way (probably by extending the options for the svg property) to enable support for discretionary ligatures.
Comment 11•17 years ago
|
||
The problem with discretionary ligatures with the extra pro whatever version of Zapfino is that if you type a word like "Company" the Co forms a ligature and looks really wrong. We should support something like text-opentype-features: dlig, liga, kern imho.
Assignee | ||
Comment 12•17 years ago
|
||
Alright then, let's just disable discretionary ligatures for now. That's slightly orthogonal to this bug --- I don't think overloading the text-rendering property is the best way to deal with that.
Comment 13•17 years ago
|
||
yep -- agreed.
Assignee | ||
Comment 14•17 years ago
|
||
This is the core functionality. It connects the CSS style to a new TEXT_OPTIMIZE_SPEED flag and makes Linux and Windows honor the flag (Windows is untested but very simple). Mac doesn't need to honor the flag because it has no fast path. As Simon noted we also have to make the flag part of the word-cache key.
This adds the pref browser.display.auto_quality_min_font_size which gives the minimum size in device pixels at which text-rendering:auto will switch to high quality mode. We handle dynamic changes in this pref. Setting this to zero forces high quality always; this would be a reasonable control point for a "quality/speed" UI toggle. I think 20 pixels is a reasonable default but we can discuss that.
This patch does not add any style rules, but it does make all users of nsIRenderingContext text methods always use high quality, so the patch will switch most chrome from speed to quality.
After this I would recommend a followup patch that adds "text-rendering:optimizeLegibility" to XUL <window> and <dialog> elements, to catch their textframe based children (e.g. <description>). I'd also add such a rule to forms.css for the <input> and <textarea> elements as discussed above...
Requesting gfx review from Stuart and layout r/sr from Mats.
Attachment #272589 -
Flags: superreview?(mats.palmgren)
Attachment #272589 -
Flags: review?(pavlov)
Comment 15•17 years ago
|
||
What about eStyleUnit_Chars? I guess we don't support letter-spacing specified in ch yet, but it doesn't seem like it should be that hard, actually. Just make the current places that look at mLetterSpacing (including this one) use nsLayoutUtils::GetAbsoluteCoord.
I guess we can do that as a followup and would end up auditing this code anyway...
If not MOZ_SVG, do we want to default to the NS_STYLE_TEXT_RENDERING_AUTO behavior? Or do we really not care there?
Assignee | ||
Comment 16•17 years ago
|
||
> I guess we can do that as a followup
Yeah.
I don't care about !MOZ_SVG. Optimizing for quality is probably fine there; such users, if any, are probably trying to do some kind of light embedded build and probably want their own text engine anyway.
Comment 17•17 years ago
|
||
Should optimize speed also call cairo_font_options_set_antialias(fontOptions, CAIRO_ANTIALIAS_NONE); c.f. http://www.w3.org/TR/SVG11/painting.html#TextRenderingProperty possibly in gfxWindowsFont::CairoScaledFont for windows?
Are there any other cairo_font_options that should be set for different text-rendering e.g. geometricPrecision?
Assignee | ||
Comment 18•17 years ago
|
||
Supporting geometricPrecision (by turning off hinting) would be an interesting future extension. I don't plan to work on that in the forseeable future.
I guess if someone specifies text-rendering:optimizeSpeed we could turn off antialiasing. (We wouldn't want that for auto, though.) I'm not sure how much speed that actually buys. Again, could easily be done as an extension later.
Assignee | ||
Comment 19•17 years ago
|
||
Stuart, Mats, we kinda need this to land for M7...
Assignee | ||
Comment 20•17 years ago
|
||
Which means we need reviews now-ish.
Updated•17 years ago
|
Attachment #272589 -
Flags: review?(pavlov) → review+
Comment 21•17 years ago
|
||
Comment on attachment 272589 [details] [diff] [review]
patch
Please file a followup bug on my earlier comment, ok?
>Index: layout/base/nsPresContext.cpp
>@@ -756,24 +764,26 @@ nsPresContext::Init(nsIDeviceContext* >+ mAutoQualityMinFontSizePixelsPref = nsContentUtils::GetIntPref(sMinFontSizePref);
You need to register to observe this pref too.
With that fixed, sr=bzbarsky
Attachment #272589 -
Flags: superreview?(mats.palmgren) → superreview+
Assignee | ||
Comment 22•17 years ago
|
||
Actually I don't need to register because we already registered browser.display.* above:
nsContentUtils::RegisterPrefCallback("browser.display.",
nsPresContext::PrefChangedCallback,
this);
Added comment to that effect.
Comment 23•17 years ago
|
||
Ah, ok. Yeah, comment is good.
Assignee | ||
Comment 24•17 years ago
|
||
Checked it in a little while ago.
Status: NEW → RESOLVED
Closed: 17 years ago
Resolution: --- → FIXED
Comment 25•17 years ago
|
||
dev-doc-needed?
Updated•17 years ago
|
Keywords: dev-doc-needed
Assignee | ||
Comment 26•17 years ago
|
||
Performance numbers look fine. I checked in the patch with browser.display.auto_quality_min_font_size set to 60 just to make sure we wouldn't hit it in performance tests. We should probably lower that number in a separate patch (and watch tinderboxes carefully while we do so). In fact it might be interesting to lower the number to 0 temporarily just to see what the tinderbox impact is...
There was an issue with linux reftests because I forgot to initialize allBits to zero; I fixed it.
Comment 27•17 years ago
|
||
> Mac doesn't need to honor the flag because it has no fast path.
I'm confused. Safari doesn't do kerning, and Firefox 2 didn't do ligatures...
Assignee | ||
Comment 28•17 years ago
|
||
*Our trunk Mac code* has no fast path for text rendering.
Comment 29•17 years ago
|
||
Oh. Is there a bug on that?
Assignee | ||
Comment 30•17 years ago
|
||
No, it's not necessarily a bug, depending on how you feel about the need for faster Mac text rendering at the expense of quality...
Updated•17 years ago
|
Flags: blocking1.9?
Updated•17 years ago
|
Flags: in-testsuite?
Comment 31•17 years ago
|
||
Is it just me or is text-rendering not standard CSS? Want to confirm that before I start writing this up.
Assignee | ||
Comment 32•17 years ago
|
||
It is standard, but it's specified in SVG 1.1.
Comment 33•17 years ago
|
||
Now documented:
http://developer.mozilla.org/en/docs/CSS:text-rendering
Keywords: dev-doc-needed → dev-doc-complete
Assignee | ||
Comment 34•17 years ago
|
||
You should mention that it can be applied to HTML and XML elements other than SVG. You should also mention that in Gecko 1.9, optimizeLegibility enables kerning and optional ligatures.
BTW what are the rules here? Can I just edit that myself if I want to?
Comment 35•17 years ago
|
||
Anyone can edit anything on MDC. That's the point to it being a wiki. Feel free to hit it.
It's in the CSS reference, which is why it doesn't specifically say anything about what elements it can be applied to; it's presumed it applies to anything you can apply CSS to.
Assignee | ||
Comment 36•17 years ago
|
||
A lot of people expect SVG properties to be inapplicable to non-SVG elements.
You need to log in
before you can comment on or make changes to this bug.
Description
•