1460259 - Support custom angles of synthetic-oblique in webrender

Status: RESOLVED FIXED

(Core :: Graphics: WebRender, enhancement, P2)

Description

[Jonathan Kew [:jfkthame]]

In CSS Fonts 4, font-style:oblique is extended to accept an optional angle (in the range -90deg..90deg), which may be passed through to a variation-font slant axis; but if there is no real slant axis or oblique font available, we'll apply a synthetic-oblique effect using the given angle.

Bug 1458004 is about to implement this for the non-webrender case.

Currently, when synthetic-oblique is used, the webrender backend just applies a fixed degree of slanting. (This results in a Windows QR reftest failure on layout/reftests/mathml/table-width-3.html, because webrender paints the glyphs with a different slant than gfx used when measuring.) We need to hook this up to the custom angle now available from gfxFontStyle.

Comment 1

[Lee Salzman [:lsalzman]]

This will be addressed by WR PR https://github.com/servo/webrender/pull/2855

Comment 2

[Lee Salzman [:lsalzman]]

Attachment: send synthetic oblique angle to WR

The main annoyance here is that to route the synthetic oblique angle to WR, we need to go through ScaledFonts first. Since they, at present, don't know this value, we have to make them remember it. That dirty work is done by gfxFont::InitializeScaledFont(), which will now handle common setup of the ScaledFont like this.

Once there, we fill in the WR FontInstanceOptions structure with the angle. This will be used at font creation - as opposed to when we go to draw text, which worked better when synthetic italics was merely a flag. This should be fine since the synthetic italics status really only depends on the style, at present, which is baked into the gfxFont for which the ScaledFont and WR FontInstance are 1:1 with.

Comment 3

[Jonathan Kew [:jfkthame]]

Comment on attachment 8988292 [details] [diff] [review]
send synthetic oblique angle to WR

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

LGTM; just a couple questions/things that might be tweaked, unless there's a good reason not to.

::: gfx/2d/2D.h
@@ +896,5 @@
>    virtual cairo_scaled_font_t* GetCairoScaledFont() { return nullptr; }
>    virtual void SetCairoScaledFont(cairo_scaled_font_t* font) {}
>  
> +  virtual Float GetSyntheticObliqueAngle() const { return 0.0f; }
> +  virtual void SetSyntheticObliqueAngle(Float aAngle) {}

Is there any reason these can't be implemented directly here, rather than having virtual methods and then overriding them in ScaledFontBase?

::: gfx/thebes/gfxFont.cpp
@@ +764,5 @@
>  
>      // If style calls for italic, and face doesn't support it, use default
>      // oblique angle as a simulation.
>      if (mStyle.style.IsItalic()) {
> +        return mFontEntry->SupportsItalic() ? 0.0f : 14.0f;

Please use FontSlantStyle::kDefaultAngle instead of the literal 14.0f (here and each time it occurs).

@@ +777,5 @@
> +gfxFont::SkewForSyntheticOblique() const
> +{
> +    // Precomputed value of tan(14deg), the default italic/oblique slant;
> +    // avoids calling tan() at runtime except for custom oblique values.
> +    const float kTan14deg = 0.249328f;

I know this is pre-existing code, you're just moving it; but I suppose we should really sanity-check this, something like:

    MOZ_ASSERT(kTan14deg == tan(FontSlantStyle::kDefaultAngle * (M_PI / 180.0));

in case the default ever gets tweaked.

Or (better, I think) how about we avoid the whole issue by computing it (once) from kDefaultAngle at runtime instead of hard-coding the number:

    static const float kTanDefaultAngle = tan(....);

That still avoids calling tan() every time we need the standard value.

::: gfx/webrender_bindings/WebRenderTypes.h
@@ +805,5 @@
>  }
>  
> +static inline wr::SyntheticItalics DegreesToSyntheticItalics(float aDegrees) {
> +  wr::SyntheticItalics synthetic_italics;
> +  synthetic_italics.angle = int16_t(std::min(std::max(aDegrees, -89.0f), 89.0f) * 256.0f);

Per spec, the allowed range is supposed to be -90° .. 90° (though it's true that it doesn't make much sense to render anything at a full ±90° oblique angle).

Anyhow, can we clamp to -90/90 here to match the spec/other code, or is that problematic on the WR side?

Comment 4

[Lee Salzman [:lsalzman]]

Attachment: send synthetic oblique angle to WR. r=jfkthame

Fixed nits.

Comment 5

[Lee Salzman [:lsalzman]]

Attachment: send synthetic oblique angle to WR. r=jfkthame

Oops. Fix constructor issue introduced while nit-fixing.

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/860f77039bdf
Send synthetic oblique angle to WR. r=jfkthame

Comment 7

[Cristina Coroiu [:ccoroiu]]

https://hg.mozilla.org/mozilla-central/rev/860f77039bdf