1470515 - ScaledFont plumbing for Android WebRender

Status: RESOLVED FIXED

(Core :: Graphics: WebRender, enhancement)

Description

[Lee Salzman [:lsalzman]]

WebRender currently can't work on Android because we don't have the necessary ScaledFont hooks in place.

Comment 1

[Lee Salzman [:lsalzman]]

Attachment: refactor ScaledFontFreeType for Android WR

Android was using ScaledFontCairo, which was only ever used for Android at this point, being mostly vestigial. Likewise, it was under the illusion that it was FontType::SKIA, for reasons that make no sense anymore.

As a start, I made this a ScaledFontFreeType, which is actually FontType::FREETYPE, so that it makes sense to someone trying to read the code.

I cleaned up some code sharing between UnscaledFont(FreeType/Fontconfig) and NativeFontResource(FreeType/Fontconfig).

Finally, ScaledFontFreeType now supports the necessary WR hooks that will let it work with recording/playback as required.

Comment 2

[Ryan Hunt [:rhunt] (on leave until early May)]

Comment on attachment 8987149 [details] [diff] [review]
refactor ScaledFontFreeType for Android WR

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

This looks good to me, just a couple of nits.

::: gfx/2d/UnscaledFontFreeType.cpp
@@ +88,5 @@
> +    return false;
> +  }
> +
> +  const char* path = mFile.c_str();
> +  size_t pathLength = strlen(path);

Can we use mFile.size() here? I'm aware this is copied code, but it'd be nice to fix this.

::: gfx/2d/UnscaledFontFreeType.h
@@ +52,5 @@
>  
>    FT_Face GetFace() const { return mFace; }
>    const char* GetFile() const { return mFile.c_str(); }
>    uint32_t GetIndex() const { return mIndex; }
> +  const RefPtr<NativeFontResource> & GetNativeFontResource() const { return mNativeFontResource; }

nit: Move the & over one character

::: gfx/thebes/gfxFT2FontList.cpp
@@ +79,5 @@
>  // allocate memory to uncompress a font from omnijar.
>  class AutoFTFace {
>  public:
>      explicit AutoFTFace(FT2FontEntry* aFontEntry)
> +        : mFace(nullptr), mFontDataBuf(nullptr), mDataLength(0), mOwnsFace(false)

Is mDataLength needed? I don't see it used anywhere.

Comment 3

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/4a20ed6e2fee
refactor ScaledFontFreeType for Android WR. r=rhunt

Comment 4

[Lee Salzman [:lsalzman]]

Fixed the nits. mDataLength is however needed as it is still accessed in some places via the DataLength() accessor.

Comment 5

[Eliza Balazs [:ebalazs_]]

https://hg.mozilla.org/mozilla-central/rev/4a20ed6e2fee