1015603 - small-caps should be implemented with gfx, not layout

Status: RESOLVED FIXED

(Core :: Layout: Text and Fonts, defect)

Description

[Jonathan Kew [:jfkthame]]

We should move the implementation of font-variant:small-caps from layout (it's currently handled similarly to text-transform by nsTextFrame, calling code in nsTextRunTransformations) into gfx, making it an attribute of gfxFontStyle that is handled internally by gfxFont when constructing textruns.

This will allow it to work for text that isn't handled by nsTextFrame (see bug 935739 about XUL, and bug 1011187 about canvas).

It will also mean we're in a position to make a proper decision, based on the properties of the actual font being used, regarding whether to use OpenType smcp or "fake" small-caps (uppercased text using a smaller font size).

Tryserver build with an experimental patch that implements this, and fixes the XUL and canvas bugs:
https://tbpl.mozilla.org/?tree=Try&rev=cdb0de559f96

I'll post patches for review after some cleanup.

Comment 1

[Jonathan Kew [:jfkthame]]

Attachment: part 1 - factor out case transformation from nsCaseTransformTextRunFactory::RebuildTextRun into a separate function.

This is a preparatory step, so that gfxFont will be able to call the same case-transformation function.

Comment 2

[Jonathan Kew [:jfkthame]]

Attachment: part 2 - move the special Greek uppercasing code from nsTextRunTransformations to intl/unicharutil.

Comment 3

[Jonathan Kew [:jfkthame]]

Attachment: part 3 - add a smallCaps field to gfxFontStyle, and pass it through from layout when creating gfxFontStyle records.

This adds a smallcaps field to gfxFontStyle, but it won't actually do anything yet.

Comment 4

[Jonathan Kew [:jfkthame]]

Attachment: part 4 - revise internal APIs to pass pointer to text for the current script or font run rather than the entire textrun.

Comment 5

[Jonathan Kew [:jfkthame]]

Attachment: part 5 - implement "fake" small-caps in gfx using a reduced-size clone of the font.

Comment 6

[Jonathan Kew [:jfkthame]]

Attachment: part 6 - remove nsFontVariantTextRunFactory from layout, leaving gfx to handle small-caps.

Comment 7

[Jonathan Kew [:jfkthame]]

Attachment: part 6 - remove nsFontVariantTextRunFactory from layout, leaving gfx to handle small-caps.

Oops, part 5 had a small error introduced while "tidying up"... now fixed.

Comment 8

[Jonathan Kew [:jfkthame]]

Attachment: part 5 - implement "fake" small-caps in gfx using a reduced-size clone of the font.

And this time with the correct patch. (Sigh...sorry for the bugspam.)

Comment 9

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Comment on attachment 8428309 [details] [diff] [review]
part 1 - factor out case transformation from nsCaseTransformTextRunFactory::RebuildTextRun into a separate function.

Review of attachment 8428309 [details] [diff] [review]:
-----------------------------------------------------------------

::: layout/generic/nsTextRunTransformations.cpp
@@ +851,5 @@
> +            ch = 'I';
> +            capitalizeDutchIJ = true;
> +            break;
> +          }
> +  

trailing whitespace

@@ +863,5 @@
> +            }
> +            ch = mcm->mMappedChars[j];
> +            break;
> +          }
> +  

trailing whitespace

Comment 10

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Comment on attachment 8428328 [details] [diff] [review]
part 5 - implement "fake" small-caps in gfx using a reduced-size clone of the font.

Review of attachment 8428328 [details] [diff] [review]:
-----------------------------------------------------------------

::: gfx/thebes/gfxPangoFonts.cpp
@@ +1600,5 @@
> +already_AddRefed<gfxFont>
> +gfxFcFont::GetSmallCapsFont()
> +{
> +    gfxFontStyle style(*GetStyle());
> +    style.size *= 0.8;

Pull 0.8 out as a named constant to be used here and below.

Comment 11

[Robert O'Callahan (:roc) (email my personal email if necessary)]

It seems to me we need another patch to support 'smcp' properly, right?

Comment 12

[Jonathan Kew [:jfkthame]]

(In reply to Robert O'Callahan (:roc) (Mozilla Corporation) from comment #11)
> It seems to me we need another patch to support 'smcp' properly, right?

Yes; that's in bug 458634.

Comment 13

[Jonathan Kew [:jfkthame]]

Pushed these patches to inbound:
https://hg.mozilla.org/integration/mozilla-inbound/rev/7a1aec9d0a5b
https://hg.mozilla.org/integration/mozilla-inbound/rev/d6c6f52d5c5e
https://hg.mozilla.org/integration/mozilla-inbound/rev/76e81e8a02a4
https://hg.mozilla.org/integration/mozilla-inbound/rev/23dd9a017b0a
https://hg.mozilla.org/integration/mozilla-inbound/rev/6d6ba8471f5e
https://hg.mozilla.org/integration/mozilla-inbound/rev/d85a5aa53fd2

FTR, this should have no effect on behavior for HTML text (support for "real" small caps, which will be a behavior change, is bug 458634). This does fix bug 935739 (XUL) and bug 1011187 (canvas), though it might be nice to add unit tests for those bugs before actually closing them.

Comment 14

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/7a1aec9d0a5b
https://hg.mozilla.org/mozilla-central/rev/d6c6f52d5c5e
https://hg.mozilla.org/mozilla-central/rev/76e81e8a02a4
https://hg.mozilla.org/mozilla-central/rev/23dd9a017b0a
https://hg.mozilla.org/mozilla-central/rev/6d6ba8471f5e
https://hg.mozilla.org/mozilla-central/rev/d85a5aa53fd2