Closed Bug 387969 Opened 17 years ago Closed 17 years ago

Need a way to control text rendering quality

Categories

(Core :: Layout, defect)

Product:
Core
Component:
Layout
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED FIXED

People

(Reporter: roc, Assigned: roc)

References

Details

(Keywords: dev-doc-complete)

Attachments

(1 file)

patch
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?
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...

For textboxes, do you also mean textareas? I'd be worried about editing performance there.
Yes, I also mean textareas. I don't forsee any problems with editing performance, what are you concerned about?
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.
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).
> 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.
> 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!)

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.
Blocks: 386759
(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.
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.
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.
yep -- agreed.
Attached patch patch — — Splinter Review 
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)
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?
> 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.
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?

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.
Stuart, Mats, we kinda need this to land for M7...
Which means we need reviews now-ish.
Attachment #272589 - Flags: review?(pavlov) → review+
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+
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.
Ah, ok.  Yeah, comment is good.
Checked it in a little while ago.
Status: NEW → RESOLVED
Closed: 17 years ago
Resolution: --- → FIXED
dev-doc-needed?
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.
> 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...
*Our trunk Mac code* has no fast path for text rendering.
Oh.  Is there a bug on that?
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...
Flags: blocking1.9?
Flags: in-testsuite?
Is it just me or is text-rendering not standard CSS?  Want to confirm that before I start writing this up.
It is standard, but it's specified in SVG 1.1.
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?
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.
A lot of people expect SVG properties to be inapplicable to non-SVG elements.
Depends on: 448156
Depends on: 463413
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: