Closed Bug 1015603 Opened 6 years ago Closed 6 years ago

small-caps should be implemented with gfx, not layout

Categories

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

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla32

People

(Reporter: jfkthame, Assigned: jfkthame)

References

(Depends on 1 open bug)

Details

Attachments

(6 files, 2 obsolete files)

15.63 KB, patch
roc
: review+
Details | Diff | Splinter Review
23.57 KB, patch
roc
: review+
Details | Diff | Splinter Review
8.26 KB, patch
roc
: review+
Details | Diff | Splinter Review
12.95 KB, patch
roc
: review+
Details | Diff | Splinter Review
10.49 KB, patch
roc
: review+
Details | Diff | Splinter Review
18.51 KB, patch
roc
: review+
Details | Diff | Splinter Review
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.
Blocks: 458634
This is a preparatory step, so that gfxFont will be able to call the same case-transformation function.
Attachment #8428309 - Flags: review?(roc)
Assignee: nobody → jfkthame
Status: NEW → ASSIGNED
This adds a smallcaps field to gfxFontStyle, but it won't actually do anything yet.
Attachment #8428312 - Flags: review?(roc)
Oops, part 5 had a small error introduced while "tidying up"... now fixed.
Attachment #8428327 - Flags: review?(roc)
Attachment #8428315 - Attachment is obsolete: true
Attachment #8428315 - Flags: review?(roc)
And this time with the correct patch. (Sigh...sorry for the bugspam.)
Attachment #8428328 - Flags: review?(roc)
Attachment #8428314 - Attachment is obsolete: true
Attachment #8428314 - Flags: review?(roc)
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
Attachment #8428309 - Flags: review?(roc) → review+
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.
Attachment #8428328 - Flags: review?(roc) → review+
It seems to me we need another patch to support 'smcp' properly, right?
(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.
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.
Target Milestone: --- → mozilla32
Duplicate of this bug: 747849
Blocks: 961558
Depends on: 1069752
You need to log in before you can comment on or make changes to this bug.