Last Comment Bug 387969 - Need a way to control text rendering quality
: Need a way to control text rendering quality
Status: RESOLVED FIXED
: dev-doc-complete
Product: Core
Classification: Components
Component: Layout (show other bugs)
: Trunk
: All All
: -- normal (vote)
: ---
Assigned To: Robert O'Callahan (:roc) (Exited; email my personal email if necessary)
:
Mentors:
Depends on: 448156 463413
Blocks: 386759
  Show dependency treegraph
 
Reported: 2007-07-12 18:10 PDT by Robert O'Callahan (:roc) (Exited; email my personal email if necessary)
Modified: 2008-11-06 17:17 PST (History)
21 users (show)
jwalden+bmo: in‑testsuite?
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
patch (32.40 KB, patch)
2007-07-16 21:02 PDT, Robert O'Callahan (:roc) (Exited; email my personal email if necessary)
pavlov: review+
bzbarsky: superreview+
Details | Diff | Review

Description Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2007-07-12 18:10:02 PDT
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).
Comment 1 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2007-07-12 20:09:32 PDT
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 Boris Zbarsky [:bz] (Out June 25-July 6) 2007-07-12 20:21:04 PDT
For textboxes, do you also mean textareas? I'd be worried about editing performance there.
Comment 3 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2007-07-12 21:00:23 PDT
Yes, I also mean textareas. I don't forsee any problems with editing performance, what are you concerned about?
Comment 4 Boris Zbarsky [:bz] (Out June 25-July 6) 2007-07-12 21:44:50 PDT
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 Simon Montagu :smontagu 2007-07-12 22:13:23 PDT
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).
Comment 6 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2007-07-12 22:26:03 PDT
> 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.
Comment 7 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2007-07-12 22:32:13 PDT
> 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.
Comment 8 Vladimir Vukicevic [:vlad] [:vladv] 2007-07-13 04:42:18 PDT
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!)
Comment 9 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2007-07-13 05:29:24 PDT
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.
Comment 10 Vladimir Vukicevic [:vlad] [:vladv] 2007-07-13 12:36:32 PDT
(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 Stuart Parmenter 2007-07-13 14:27:13 PDT
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.
Comment 12 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2007-07-13 15:07:21 PDT
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 Stuart Parmenter 2007-07-13 15:27:55 PDT
yep -- agreed.
Comment 14 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2007-07-16 21:02:42 PDT
Created attachment 272589 [details] [diff] [review]
patch

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.
Comment 15 Boris Zbarsky [:bz] (Out June 25-July 6) 2007-07-16 21:34:14 PDT
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?
Comment 16 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2007-07-16 22:28:44 PDT
> 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 Robert Longson 2007-07-16 22:47:28 PDT
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?
Comment 18 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2007-07-16 22:52:21 PDT
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.
Comment 19 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2007-07-25 11:14:04 PDT
Stuart, Mats, we kinda need this to land for M7...
Comment 20 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2007-07-25 11:35:20 PDT
Which means we need reviews now-ish.
Comment 21 Boris Zbarsky [:bz] (Out June 25-July 6) 2007-07-25 22:53:38 PDT
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
Comment 22 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2007-07-26 00:15:12 PDT
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 Boris Zbarsky [:bz] (Out June 25-July 6) 2007-07-26 00:18:52 PDT
Ah, ok.  Yeah, comment is good.
Comment 24 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2007-07-26 03:11:00 PDT
Checked it in a little while ago.
Comment 25 Dão Gottwald [:dao] 2007-07-26 03:20:33 PDT
dev-doc-needed?
Comment 26 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2007-07-26 05:57:35 PDT
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 Jesse Ruderman 2007-07-30 15:34:19 PDT
> 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...
Comment 28 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2007-07-30 16:27:05 PDT
*Our trunk Mac code* has no fast path for text rendering.
Comment 29 Jesse Ruderman 2007-07-30 17:42:10 PDT
Oh.  Is there a bug on that?
Comment 30 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2007-07-30 18:07:49 PDT
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...
Comment 31 Eric Shepherd [:sheppy] 2008-03-03 14:38:54 PST
Is it just me or is text-rendering not standard CSS?  Want to confirm that before I start writing this up.
Comment 32 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2008-03-03 14:51:01 PST
It is standard, but it's specified in SVG 1.1.
Comment 33 Eric Shepherd [:sheppy] 2008-03-04 12:17:53 PST
Now documented:

http://developer.mozilla.org/en/docs/CSS:text-rendering
Comment 34 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2008-03-04 12:34:00 PST
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 Eric Shepherd [:sheppy] 2008-03-04 12:36:21 PST
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.
Comment 36 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2008-03-04 12:44:38 PST
A lot of people expect SVG properties to be inapplicable to non-SVG elements.

Note You need to log in before you can comment on or make changes to this bug.