Closed Bug 736134 Opened 8 years ago Closed 8 years ago

[Azure] Implement more advanced DWrite text rendering parameters

Categories

(Core :: Graphics, defect)

x86_64
Windows 7
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla14

People

(Reporter: bas.schouten, Assigned: bas.schouten)

Details

Attachments

(3 files, 1 obsolete file)

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.
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)
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)
This makes the font rendering code use the GlyphRenderingOptions Azure API to select the correct glyph rendering options.
Attachment #606261 - Flags: review?(jmuizelaar)
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+
(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.
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+
https://hg.mozilla.org/mozilla-central/rev/063352a44541
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla14
Version: unspecified → Trunk
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.