Closed Bug 1381471 Opened 3 years ago Closed 3 years ago

Refactor implementations of gfxPlatform::IsFontFormatSupported

Categories

(Core :: Graphics: Text, enhancement)

enhancement
Not set

Tracking

()

RESOLVED FIXED
mozilla56
Tracking Status
firefox56 --- fixed

People

(Reporter: jfkthame, Assigned: jfkthame)

Details

Attachments

(1 file)

Currently, we implement this separately in each gfxPlatform subclass. This is a bit silly, as all the implementations behave exactly the same, with one exception (macOS) which recognizes an additional format hint. Let's move the common behavior into the base class, so we only have one place to maintain it.
When looking over your cleanup in bug 1376964 pt 7, it occurred to me that it would be more sensible to do something like this, rather than all those separate-but-equivalent implementations.
Attachment #8887032 - Flags: review?(cam)
Assignee: nobody → jfkthame
Status: NEW → ASSIGNED
Comment on attachment 8887032 [details] [diff] [review]
Refactor gfxPlatform::IsFontFormatSupported implementations to put all the cross-platform behavior in the base class and remove most overrides

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

::: gfx/thebes/gfxPlatformGtk.cpp
@@ -319,5 @@
> -
> -    // accept supported formats
> -    // Pango doesn't apply features from AAT TrueType extensions.
> -    // Assume that if this is the only SFNT format specified,
> -    // then AAT extensions are required for complex script support.

I assume it's not worth preserving this comment somewhere?
Attachment #8887032 - Flags: review?(cam) → review+
(In reply to Cameron McCormack (:heycam) from comment #2)
> > -    // accept supported formats
> > -    // Pango doesn't apply features from AAT TrueType extensions.
> > -    // Assume that if this is the only SFNT format specified,
> > -    // then AAT extensions are required for complex script support.
> 
> I assume it's not worth preserving this comment somewhere?

It's out-dated: we don't use Pango any more. But even in an updated form, I don't think it really adds any value to the code itself.
https://hg.mozilla.org/integration/mozilla-inbound/rev/90d17c7dc5d407b32c2e9645f821ea94b55d4e96
Bug 1381471 - Refactor gfxPlatform::IsFontFormatSupported implementations to put all the cross-platform behavior in the base class and remove most overrides. r=heycam
https://hg.mozilla.org/mozilla-central/rev/90d17c7dc5d4
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla56
You need to log in before you can comment on or make changes to this bug.