Closed Bug 1309917 Opened 3 years ago Closed 3 years ago

Stop defaulting to system wide default fonts if no IDWriteFont exists

Categories

(Core :: Graphics, defect)

21 Branch
x86
Windows 8
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla52
Tracking Status
firefox52 --- fixed

People

(Reporter: mchang, Assigned: mchang)

References

Details

Attachments

(1 file)

+++ This bug was initially created as a clone of Bug #842894 +++

See https://bugzilla.mozilla.org/show_bug.cgi?id=842894#c21
Attached patch font.patchSplinter Review
(In reply to Jonathan Kew (:jfkthame) from comment #21)
> Comment on attachment 8704768 [details] [diff] [review]
> Support dwrite fonts in skia
> It's expected that a gfxDWriteFontEntry may not hold an IDWriteFont
> reference; in the case of an @font-face resource, it will *instead* have an
> IDWriteFontFile. See
> https://dxr.mozilla.org/mozilla-central/rev/
> 7ae377917236b7e6111146aa9fb4c073c0efc7f4/gfx/thebes/gfxDWriteFontList.h#190-
> 195.
> 
> But if it's a font entry for a downloaded resource (and therefore
> fe->GetFont() returns null), it doesn't make sense to just substitute some
> default font instead. I don't know what this IDWriteFont ends up getting
> used for in the Skia backend -- maybe nothing, if we're lucky?! -- but if
> we're supposed to be using the downloaded font, we can't just start using
> the default font instead for any meaningful purpose.

You're right, luckily it wasn't using the IDWriteFontFamily for anything but debug purposes. The IDwriteFont was being used to calculate some style data [1]. To actually render the glyph, Skia was using only the font face, which we always seem to have.

This patch stops using IDWriteFont and instead creates a ScaledDWriteFont with style data from gfxFontStyle information. It also stops defaulting to a system wide default font if it can't find an IDwriteFont. Thanks for finding the bug!

Try looks good - https://treeherder.mozilla.org/#/jobs?repo=try&revision=85063d2d16cc

[1] http://searchfox.org/mozilla-central/source/gfx/skia/skia/src/ports/SkTypeface_win_dw.h#27
Attachment #8800732 - Flags: review?(jfkthame)
Comment on attachment 8800732 [details] [diff] [review]
font.patch

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

Thanks for dealing with this; looks reasonable to me.
Attachment #8800732 - Flags: review?(jfkthame) → review+
Pushed by mchang@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/5b7c66cd059c
Stop defaulting to system wide default fonts if no IDWriteFont exists. r=jfkthame
https://hg.mozilla.org/mozilla-central/rev/5b7c66cd059c
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla52
You need to log in before you can comment on or make changes to this bug.