[Azure] Implement more advanced DWrite text rendering parameters

RESOLVED FIXED in mozilla14

Status

()

Core
Graphics
RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: bas, Assigned: bas)

Tracking

Trunk
mozilla14
x86_64
Windows 7
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(3 attachments, 1 obsolete attachment)

(Assignee)

Description

6 years ago
With cairo drawing we have a complicated set of code which selects our text rendering parameters. In Azure we currently don't support any of that, we should though.
(Assignee)

Comment 1

6 years ago
Created attachment 606255 [details] [diff] [review]
Part 1: Add GlyphRenderingOptions API

Add an API for creating glyph rendering options that can be used in glyph rendering calls.
Assignee: nobody → bas.schouten
Status: NEW → ASSIGNED
Attachment #606255 - Flags: review?(jmuizelaar)
(Assignee)

Comment 2

6 years ago
Created attachment 606258 [details] [diff] [review]
Part 2: Add rendering params objects to windows platform

Add the renderingparams objects to gfxWindowsPlatform and initialize them independent of Azure content being on/off. This only takes only 0.1ms and will be needed anyway when Azure becomes the default for D2D.
Attachment #606258 - Flags: review?(jmuizelaar)
(Assignee)

Comment 3

6 years ago
Created attachment 606261 [details] [diff] [review]
Part 3: Use GlyphRenderingOptions API

This makes the font rendering code use the GlyphRenderingOptions Azure API to select the correct glyph rendering options.
Attachment #606261 - Flags: review?(jmuizelaar)
(Assignee)

Comment 4

6 years ago
These patches mostly solve the issued. Font rendering looks a -lot- more consistent with them, although inside the azure code presumably we still need to do some work to use the right measuring mode corresponding with the chosen render mode, we can deal with that in a follow-up though.
Comment on attachment 606255 [details] [diff] [review]
Part 1: Add GlyphRenderingOptions API

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

::: gfx/2d/2D.h
@@ +488,5 @@
>  protected:
>    ScaledFont() {}
>  };
>  
> +class GlyphRenderingOptions : public RefCounted<GlyphRenderingOptions>

It would be good to add a bit of documentation about what this class is to be used for and why it's designed the way it is.

::: gfx/2d/DrawTargetD2D.cpp
@@ +896,5 @@
>  
> +  IDWriteRenderingParams *params = NULL;
> +  if (aRenderOptions) {
> +    if (aRenderOptions->GetType() != FONT_DWRITE) {
> +      gfxDebug() << *this << ": Ignoring incompatible GlyphRenderingOptions.";

I think we can assert this condition.
Attachment #606255 - Flags: review?(jmuizelaar) → review+
Comment on attachment 606258 [details] [diff] [review]
Part 2: Add rendering params objects to windows platform

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

::: gfx/thebes/gfxWindowsPlatform.cpp
@@ +1407,5 @@
>              mMeasuringMode = DWRITE_MEASURING_MODE_NATURAL;
>              break;
>          }
> +
> +        nsRefPtr<IDWriteRenderingParams> defaultRenderingParams;

Do we usually use nsRefPtr's on COM objects?

@@ +1445,5 @@
> +	    (DWRITE_PIXEL_GEOMETRY)geometry, (DWRITE_RENDERING_MODE)mode,
> +            getter_AddRefs(mRenderingParams[TEXT_RENDERING_NORMAL]));
> +        GetDWriteFactory()->CreateCustomRenderingParams(gamma, contrast, level,
> +	    (DWRITE_PIXEL_GEOMETRY)geometry, DWRITE_RENDERING_MODE_CLEARTYPE_GDI_CLASSIC,
> +            getter_AddRefs(mRenderingParams[TEXT_RENDERING_GDI_CLASSIC]));

I found this hunk unreadable. Can you make it better?
Attachment #606258 - Flags: review?(jmuizelaar) → review-
Attachment #606261 - Flags: review?(jmuizelaar) → review+
(Assignee)

Comment 7

6 years ago
(In reply to Jeff Muizelaar [:jrmuizel] from comment #6)
> Comment on attachment 606258 [details] [diff] [review]
> Part 2: Add rendering params objects to windows platform
> 
> Review of attachment 606258 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: gfx/thebes/gfxWindowsPlatform.cpp
> @@ +1407,5 @@
> >              mMeasuringMode = DWRITE_MEASURING_MODE_NATURAL;
> >              break;
> >          }
> > +
> > +        nsRefPtr<IDWriteRenderingParams> defaultRenderingParams;
> 
> Do we usually use nsRefPtr's on COM objects?

Other than where we use mozilla::RefPtr, yes.

> 
> @@ +1445,5 @@
> > +	    (DWRITE_PIXEL_GEOMETRY)geometry, (DWRITE_RENDERING_MODE)mode,
> > +            getter_AddRefs(mRenderingParams[TEXT_RENDERING_NORMAL]));
> > +        GetDWriteFactory()->CreateCustomRenderingParams(gamma, contrast, level,
> > +	    (DWRITE_PIXEL_GEOMETRY)geometry, DWRITE_RENDERING_MODE_CLEARTYPE_GDI_CLASSIC,
> > +            getter_AddRefs(mRenderingParams[TEXT_RENDERING_GDI_CLASSIC]));
> 
> I found this hunk unreadable. Can you make it better?

Will do.
(Assignee)

Comment 8

6 years ago
Created attachment 607216 [details] [diff] [review]
Part 2: Add rendering params objects to windows platform v2
Attachment #606258 - Attachment is obsolete: true
Attachment #607216 - Flags: review?(jmuizelaar)
Comment on attachment 607216 [details] [diff] [review]
Part 2: Add rendering params objects to windows platform v2

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

::: gfx/thebes/gfxWindowsPlatform.cpp
@@ +1433,5 @@
> +                gamma : defaultRenderingParams->GetGamma();
> +
> +        level =
> +            level >= 0.0 && level <= 1.0 ?
> +level : defaultRenderingParams->GetClearTypeLevel();

This line's indentation is broken and maybe use if blocks.
Attachment #607216 - Flags: review?(jmuizelaar) → review+
(Assignee)

Comment 10

6 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/4d21d1ad9b35
https://hg.mozilla.org/integration/mozilla-inbound/rev/191e76bacf77
https://hg.mozilla.org/integration/mozilla-inbound/rev/bc0a61adc10d
https://hg.mozilla.org/mozilla-central/rev/063352a44541
Status: ASSIGNED → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla14
Version: unspecified → Trunk
Previous changeset was a follow-up.

Original changesets are:
https://hg.mozilla.org/mozilla-central/rev/4d21d1ad9b35
https://hg.mozilla.org/mozilla-central/rev/191e76bacf77
https://hg.mozilla.org/mozilla-central/rev/bc0a61adc10d

Comment 13

6 years ago
Setting "gfx.font_rendering.cleartype_params.rendering_mode" to 1 causes all text to disappear.
You need to log in before you can comment on or make changes to this bug.