Last Comment Bug 574976 - enabling direct2d makes fonts appear as Cleartype regardless of system Cleartype prefs
: enabling direct2d makes fonts appear as Cleartype regardless of system Cleart...
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: Graphics (show other bugs)
: unspecified
: x86_64 Windows 7
: -- normal with 9 votes (vote)
: ---
Assigned To: Bas Schouten (:bas.schouten)
:
Mentors:
: 587953 (view as bug list)
Depends on:
Blocks: d2d-beta 594325
  Show dependency treegraph
 
Reported: 2010-06-26 11:42 PDT by Leon Sorokin
Modified: 2013-04-26 07:52 PDT (History)
39 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
final+


Attachments
Read system cleartype pref (3.63 KB, patch)
2010-08-25 12:43 PDT, Bas Schouten (:bas.schouten)
no flags Details | Diff | Splinter Review
Read system cleartype pref v2 (5.55 KB, patch)
2010-08-27 12:08 PDT, Bas Schouten (:bas.schouten)
jmuizelaar: review-
Details | Diff | Splinter Review
Read system cleartype pref v3 (8.67 KB, patch)
2010-08-27 13:40 PDT, Bas Schouten (:bas.schouten)
no flags Details | Diff | Splinter Review
Comparison screenshot, Direct 2d with ClearType system-wide (143.87 KB, image/png)
2010-09-10 01:22 PDT, Danny Thomas
no flags Details
Minefield trunk with system wide Cleartype Off (53.45 KB, image/png)
2010-09-10 03:00 PDT, neb1236
no flags Details

Description Leon Sorokin 2010-06-26 11:42:58 PDT
User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 6.1; Win64; x64; en-US; rv:1.9.3a6pre) Gecko/20100626 Minefield/3.7a6pre
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 6.1; Win64; x64; en-US; rv:1.9.3a6pre) Gecko/20100626 Minefield/3.7a6pre

i decided to give Direct2D a try and enabled it in my nightly. the result was indeed speedier, but the fonts look not-so-great.

it's almost like everything is rendered using Cleartype despite the fact that i have Cleartype disabled for everything on my system cause i find that it often results in fuzzier, not crisper fonts.

Reproducible: Always

Steps to Reproduce:
1. enable direct2d.
Comment 1 Leon Sorokin 2010-06-26 11:47:03 PDT
in case anyone's interested, ATI Radeon HD5850 w/Catalyst 10.6

"Turn On Cleartype" is unchecked in the Cleartype Text Tuner prefs in control panel.
Comment 2 Boris Zbarsky [:bz] 2010-06-26 21:45:08 PDT
Direct2d has its own built-in antialiasing system, which isn't cleartype.  So it'll perform antialiasing regardless of the cleartype pref.  Not sure whether there's a separate pref for controlling it.
Comment 3 Leon Sorokin 2010-06-26 23:45:25 PDT
a pref would help. but pref or no pref, i think the anti-aliasing (for fonts at least) needs to match the cleartype pref set for the system if possible.

right now the rendering differs from every other browser, except of course the hardware accelerated IE9 - no surprise there.
Comment 4 Boris Zbarsky [:bz] 2010-06-27 23:53:27 PDT
> i think the anti-aliasing (for fonts at least) needs to match the cleartype
> pref set for the system if possible

Then you're complaining to the wrong people.... the right ones would be Microsoft.  The antialiasing happens in direct2d/directwrite itself.  We don't do it.  It's not even clear to me that we can disable it as an API consumer (but I cced some people who might know that for sure).
Comment 5 Leon Sorokin 2010-06-28 00:03:37 PDT
i'm not familiar with the direct2d api or how you guys tied it into Cairo. so i have no way of knowing if antialiasing is something that can simply be turned off (and is just on by default), so there's no point in complaining to MS until a Moz dev gives a definitive answer. that's why i asked here first. thanks for looking into it.
Comment 6 Bas Schouten (:bas.schouten) 2010-06-28 04:37:52 PDT
(In reply to comment #4)
> > i think the anti-aliasing (for fonts at least) needs to match the cleartype
> > pref set for the system if possible
> 
> Then you're complaining to the wrong people.... the right ones would be
> Microsoft.  The antialiasing happens in direct2d/directwrite itself.  We don't
> do it.  It's not even clear to me that we can disable it as an API consumer
> (but I cced some people who might know that for sure).

http://msdn.microsoft.com/en-us/library/dd316897%28v=VS.85%29.aspx

Indirectly an API user inside mozilla can tell cairo to use grayscale (or no) AA and our D2D backend will pass the correct option in. I'm not sure if Thebes exposes that though!

(In reply to comment #5)
> i'm not familiar with the direct2d api or how you guys tied it into Cairo. so i
> have no way of knowing if antialiasing is something that can simply be turned
> off (and is just on by default), so there's no point in complaining to MS until
> a Moz dev gives a definitive answer. that's why i asked here first. thanks for
> looking into it.

What you might complain to MS about is that D2D doesn't adhere to system D2D setting my default, on the other hand, maybe it does in some situations. We select cleartype fairly explicitly.
Comment 7 Boris Zbarsky [:bz] 2010-06-28 05:02:57 PDT
Sounds like whatever place we select it we may need to check the OS pref.  :(
Comment 8 WulfTheSaxon [:Wulf] 2010-06-28 19:44:02 PDT
See also: Bug 517642
Comment 9 John Daggett (:jtd) 2010-08-17 14:51:26 PDT
*** Bug 587953 has been marked as a duplicate of this bug. ***
Comment 10 Bas Schouten (:bas.schouten) 2010-08-17 15:02:47 PDT
I just analyzed the return value from CreateRenderingParams, it's supposed to return renderingparams according to the default monitor. However for me it just always returns ClearType level 1 and RENDERING_MODE_DEFAULT, nothing useful coming out of it even if I totally disable cleartype.
Comment 11 AluminumHaste 2010-08-23 10:58:55 PDT
This is terrible, updated to latest release of minefield and the text is so
blurry it makes my eyes water trying to focus.

Here is a screenshot of IE8 compared to Mozilla Minefield. IE8 on the left

http://a.imageshack.us/img690/4227/blurry.png
Comment 12 Paul-Kenji Cahier 2010-08-23 11:13:49 PDT
As far as I can see the only solution is to disable D2D for now.(set mozilla.widget.render-mode to 0)
Comment 13 Leon Sorokin 2010-08-23 11:21:22 PDT
should we change the title of the bug to be more on clear / keywords?

D2D/direct2d/antialiasing/cleartype/fuzzy/blurry/text/fonts
Comment 14 Loïc Yhuel 2010-08-24 14:36:22 PDT
(In reply to comment #10)
> I just analyzed the return value from CreateRenderingParams, it's supposed to
> return renderingparams according to the default monitor. However for me it just
> always returns ClearType level 1 and RENDERING_MODE_DEFAULT, nothing useful
> coming out of it even if I totally disable cleartype.

I get 0.5, but it seems to be the same as 1. (but it's working, as 0.0 disables subpixel AA).
Currently CreateRenderingParams/SetTextRenderingParams are not even called, so I don't know what are the really used values.

I tested DWRITE_RENDERING_MODE_CLEARTYPE_GDI_CLASSIC (with DWRITE_RENDERING_MODE_CLEARTYPE_GDI_CLASSIC/DWRITE_MEASURING_MODE_GDI_CLASSIC for CreateGlyphRunAnalysis, and DWRITE_MEASURING_MODE_GDI_CLASSIC for DrawGlyphRun), and I really like it better than default, it's similar to GDI, the biggest difference being the character spacing which is smaller (sometimes really too small).
Comment 15 Bas Schouten (:bas.schouten) 2010-08-24 15:53:44 PDT
(In reply to comment #14)
> (In reply to comment #10)
> > I just analyzed the return value from CreateRenderingParams, it's supposed to
> > return renderingparams according to the default monitor. However for me it just
> > always returns ClearType level 1 and RENDERING_MODE_DEFAULT, nothing useful
> > coming out of it even if I totally disable cleartype.
> 
> I get 0.5, but it seems to be the same as 1. (but it's working, as 0.0 disables
> subpixel AA).
> Currently CreateRenderingParams/SetTextRenderingParams are not even called, so
> I don't know what are the really used values.

The really used values should be the monitor default anyway, but yes, there's a bug here but since it doesn't seem to make any difference I didn't check it in yet until I found out what's actually going wrong here.
Comment 16 Bas Schouten (:bas.schouten) 2010-08-25 12:43:04 PDT
Created attachment 469141 [details] [diff] [review]
Read system cleartype pref

This largely mimics GDI behavior with Cleartype switched off. It does grayscale AA or not depending on the GASP table. The glyph positioning is a little bit worse than GDI for this in some cases, but it should be a lot closer to what users with cleartype disabled expect.
Comment 17 Loïc Yhuel 2010-08-25 13:50:33 PDT
(In reply to comment #16)
> Created attachment 469141 [details] [diff] [review]
> Read system cleartype pref
> 
> This largely mimics GDI behavior with Cleartype switched off. It does grayscale
> AA or not depending on the GASP table. The glyph positioning is a little bit
> worse than GDI for this in some cases, but it should be a lot closer to what
> users with cleartype disabled expect.

Shouldn't CreateGlyphRunAnalysis take the modified measuring mode too ?

Since you don't call SetTextRenderingParams, which DWRITE_RENDERING_MODE is used ? I suppose it will not change, but use a grayscale AA like in the tab titles and nav bar, which looks like Cleartype without the colors and not GDI AA.
Comment 18 Bas Schouten (:bas.schouten) 2010-08-27 12:08:19 PDT
Created attachment 469954 [details] [diff] [review]
Read system cleartype pref v2

Adjusted as per discussion.
Comment 19 Jeff Muizelaar [:jrmuizel] 2010-08-27 12:55:03 PDT
Comment on attachment 469954 [details] [diff] [review]
Read system cleartype pref v2

>diff --git a/gfx/cairo/cairo/src/cairo-dwrite-font.cpp b/gfx/cairo/cairo/src/cairo-dwrite-font.cpp
>--- a/gfx/cairo/cairo/src/cairo-dwrite-font.cpp
>+++ b/gfx/cairo/cairo/src/cairo-dwrite-font.cpp
>@@ -1232,16 +1232,62 @@ _cairo_dwrite_show_glyphs_on_surface(voi

We should be able to do this detection at the same time as the win32 backend.

> #if CAIRO_HAS_D2D_SURFACE
>+
>+static inline unsigned short
>+read_short(const char *buf)
>+{
>+    return be16_to_cpu(*(unsigned short*)buf);
>+}
>+
>+#define GASP_TAG 0x70736167
>+#define GASP_DOGRAY 0x2
>+
>+static cairo_bool_t
>+do_grayscale(IDWriteFontFace *dwface, unsigned int ppem)
>+{
>+    void *tableContext;
>+    char *tableData;
>+    UINT32 tableSize;
>+    BOOL exists;
>+    dwface->TryGetFontTable(GASP_TAG, (const void**)&tableData, &tableSize, &tableContext, &exists);
>+
>+    if (tableSize < 4) {
>+	return true;
>+    }
>+
>+    if (exists) {
>+	struct gaspRange {
>+	    unsigned short maxPPEM; // Stored big-endian
>+	    unsigned short behavior; // Stored big-endian
>+	};
>+	unsigned short numRanges = read_short(tableData + 2);
>+	if (tableSize < 4 + numRanges * 4) {
>+	    return true;
>+	}
>+	gaspRange *ranges = (gaspRange *)(tableData + 4);
>+	for (int i = 0; i < numRanges; i++) {
>+	    if (be16_to_cpu(ranges[i].maxPPEM) > ppem) {
>+		if (!(be16_to_cpu(ranges[i].behavior) & GASP_DOGRAY)) {
>+		    return false;

Missing ReleaseFontTable here and other places.

>+		}
>+		break;
>+	    }
>+	}
>+	dwface->ReleaseFontTable(tableContext);
>+    }
>+    return true;
>+}
>+
> /* Surface helper function */
> //XXX: this function should probably be in cairo-d2d-surface.cpp
> cairo_int_status_t
> _cairo_dwrite_show_glyphs_on_d2d_surface(void			*surface,
> 					cairo_operator_t	 op,
> 					const cairo_pattern_t	*source,
> 					cairo_glyph_t		*glyphs,
> 					int			 num_glyphs,
>diff --git a/gfx/cairo/cairo/src/cairo-win32-private.h b/gfx/cairo/cairo/src/cairo-win32-private.h
>--- a/gfx/cairo/cairo/src/cairo-win32-private.h
>+++ b/gfx/cairo/cairo/src/cairo-win32-private.h
>@@ -208,16 +208,19 @@ void
> 
>+BYTE
>+_get_system_quality (void);
>+

Now that isn't static it should have a better name like _cairo_win32_get_system_text_quality()
Comment 20 Bas Schouten (:bas.schouten) 2010-08-27 13:40:09 PDT
Created attachment 469999 [details] [diff] [review]
Read system cleartype pref v3
Comment 21 Jonathan Kew (:jfkthame) 2010-08-27 14:08:18 PDT
Comment on attachment 469999 [details] [diff] [review]
Read system cleartype pref v3

>+#define GASP_TAG 0x70736167
....
>+    dwface->TryGetFontTable(GASP_TAG, (const void**)&tableData, &tableSize, &tableContext, &exists);

This feels odd - the "natural" way to define the value would be 0x67617370 ('gasp'), and then the Windows API takes an endian-swapped tag. But I suppose as this is within a Windows-only source file, it doesn't really matter, and this is convenient.

>+    if (tableSize < 4) {
>+	return true;
>+    }

Is TryGetFontTable guaranteed to zero the tableSize value if the table is absent? The MSDN doc doesn't specifically address this, so I'd be inclined to check the exists flag here as well...

>+    if (exists) {

...and then it won't need checking here.

Hmmm. Actually, this would still leak the table reference in the case where tableSize < 4, but the table does exist.

Also, it would be good to check the version field of the 'gasp' table, and bail out if it's not the expected one.

>+	struct gaspRange {
>+	    unsigned short maxPPEM; // Stored big-endian
>+	    unsigned short behavior; // Stored big-endian
>+	};
>+	unsigned short numRanges = read_short(tableData + 2);
>+	if (tableSize < (UINT)4 + numRanges * 4) {
>+	    dwface->ReleaseFontTable(tableContext);
>+	    return true;
>+	}
>+	gaspRange *ranges = (gaspRange *)(tableData + 4);
>+	for (int i = 0; i < numRanges; i++) {
>+	    if (be16_to_cpu(ranges[i].maxPPEM) > ppem) {
>+		if (!(be16_to_cpu(ranges[i].behavior) & GASP_DOGRAY)) {
>+		    dwface->ReleaseFontTable(tableContext);
>+		    return false;
>+		}
>+		break;
>+	    }
>+	}
>+	dwface->ReleaseFontTable(tableContext);
>+    }
>+    return true;

Looks correct, but I'd suggest rearranging it a bit so as to have a single point where ReleaseFontTable is called. Just have a boolean "result" variable that you initialize to true, and set to false if you find the relevant flag. You're already breaking out of the loop at that point, so you can rely on the final Release..., and then end with "return result".


>-static BYTE
>+BYTE
> _get_system_quality (void)

Use a better name?
Comment 22 neb1236 2010-09-03 04:09:55 PDT
Hi,

Is the font spacing problem can be solved from Firefox or is it a d2d bug that only a graphic platform updates (the current one doesn't correct this) can solved?
As a Cleartype-hater guy I'm on the limit to disable d2d to turn back to GDI and its correct letter spacing (And of course there is no way I can turn to cleartype).
Comment 23 Jonathan Guerin 2010-09-07 16:29:49 PDT
I'm just adding my confirmation that enabling D2D makes the text way too blurry. It really hurts my eyes to try to read it...
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:2.0b5) Gecko/20100101 Firefox/4.0b5
Comment 24 Jonathan Jacobs 2010-09-09 15:44:09 PDT
Since it seems like most of this discussion is about ClearType off, I feel like I should probably explicitly mentioned this: font rendering in Firefox with D2D enabled (with ClearType on in Windows) looks different (read worse) than with D2D in Firefox disabled.
Comment 25 cris 2010-09-09 16:48:27 PDT
Just to add, MS's hotfix doesn't solve the problem.
Comment 26 Danny Thomas 2010-09-10 01:22:35 PDT
Created attachment 473985 [details]
Comparison screenshot, Direct 2d with ClearType system-wide

I'd like to echo the previous two poster's points. This certainly isn't as simple as ClearType. CT looks excellent on my DVI connected display, however b5 with Direct2D on significantly changes the fonts themselves and the quality of the font smoothing doesn't come close to CT. See attached comparison screenshot.

I have the Internet Explorer 9 Platform installed so already have the hotfixes discussed. Screenshot is from latest nightly:

Mozilla/5.0 (Windows NT 6.1; WOW64; rv:2.0b6pre) Gecko/20100909 Firefox/4.0b6pre

  Graphics

        Adapter Description
        NVIDIA Quadro FX 570

        Vendor ID
        10de

        Device ID
        040e

        Adapter RAM
        256

        Adapter Drivers
        nvd3dumx,nvwgf2umx,nvwgf2umx nvd3dum,nvwgf2um,nvwgf2um

        Driver Version
        8.17.12.5912

        Driver Date
        7-26-2010

        Direct2D Enabled
        true

        DirectWrite Enabled
        true

      GPU Accelerated Windows
      1/1 Direct3D 9
Comment 27 John Daggett (:jtd) 2010-09-10 01:47:42 PDT
(In reply to comment #26)

> I'd like to echo the previous two poster's points. This certainly isn't as
> simple as ClearType. CT looks excellent on my DVI connected display, however b5
> with Direct2D on significantly changes the fonts themselves and the quality of
> the font smoothing doesn't come close to CT. See attached comparison
> screenshot.

Thanks for attaching the screenshot, that really helps clarify the differences you're seeing.  What you're seeing is *not* D2D vs. classic Cleartype but rather the problem related to bug 579276, namely that the layer system is rendering chrome elements into transparent layers and that hoses subpixel AA.  Magnify you're screenshot and you'll see the colored fringes around text on the left but nothing but grayscale AA on the right.

That's a problem distinct from ignoring Cleartype system settings, the bug here.
Comment 28 Michael Lefevre 2010-09-10 02:15:58 PDT
(In reply to comment #27)
> (In reply to comment #26)
> Thanks for attaching the screenshot, that really helps clarify the differences
> you're seeing.  What you're seeing is *not* D2D vs. classic Cleartype but
> rather the problem related to bug 579276, namely that the layer system is
> rendering chrome elements into transparent layers and that hoses subpixel AA. 

Whatever he is seeing, it cannot be bug 579276, because that was fixed some days before the build he is using.  People using nightly builds are still complaining about the blurriness and saying it is worse than other cleartype.  Others possibly related are bug 590342 and bug 594325, neither of which are really pinned down to a specific issue.  Basically the QA situation at the moment seems to be to see what happens when this bug and others are fixed, and then identify the remaining bugs.  So I think it would be good if this got into a beta rather than just for final.
Comment 29 neb1236 2010-09-10 03:00:42 PDT
Created attachment 474001 [details]
Minefield trunk with system wide Cleartype Off
Comment 30 neb1236 2010-09-10 03:01:05 PDT
Seems like many don't understand what how non-Cleartype look like and what this bug is for.
For your information the patch is already in the trunk for some time already, and it works flawlessly (at least for me), see attachment.
Therefore what’s left is the letter spacing bug (look at the firefox button).
And maybe also implementing what XP was doing, Aliased font, and Antialiasing in gray-scaled mode bigger font (see this extension ca manage to do it https://addons.mozilla.org/en-US/firefox/addon/150952/)
There seems also to be an issues on downloadable font and font without hinting that should be Antialiased that aren’t with this activated
Should we open separate bug (letter spacing, Antialiasing on big font, and Antialiasing on downloaded/non-Hinted font)
Comment 31 Danny Thomas 2010-09-10 05:39:48 PDT
(In reply to comment #27)
> (In reply to comment #26)
> 
> > I'd like to echo the previous two poster's points. This certainly isn't as
> > simple as ClearType. CT looks excellent on my DVI connected display, however b5
> > with Direct2D on significantly changes the fonts themselves and the quality of
> > the font smoothing doesn't come close to CT. See attached comparison
> > screenshot.
> 
> Thanks for attaching the screenshot, that really helps clarify the differences
> you're seeing.  What you're seeing is *not* D2D vs. classic Cleartype but
> rather the problem related to bug 579276, namely that the layer system is
> rendering chrome elements into transparent layers and that hoses subpixel AA. 
> Magnify you're screenshot and you'll see the colored fringes around text on the
> left but nothing but grayscale AA on the right.
> 
> That's a problem distinct from ignoring Cleartype system settings, the bug
> here.

Oops, got lost following dupe issues for the other reports of blurriness and sidetracked by the other comments.
Comment 32 Bas Schouten (:bas.schouten) 2010-10-14 15:12:09 PDT
http://hg.mozilla.org/mozilla-central/rev/f773f74f28d1
Comment 33 Bas Schouten (:bas.schouten) 2010-10-14 15:13:33 PDT
(In reply to comment #30)
> Seems like many don't understand what how non-Cleartype look like and what this
> bug is for.
> For your information the patch is already in the trunk for some time already,
> and it works flawlessly (at least for me), see attachment.
> Therefore what’s left is the letter spacing bug (look at the firefox button).
> And maybe also implementing what XP was doing, Aliased font, and Antialiasing
> in gray-scaled mode bigger font (see this extension ca manage to do it
> https://addons.mozilla.org/en-US/firefox/addon/150952/)
> There seems also to be an issues on downloadable font and font without hinting
> that should be Antialiased that aren’t with this activated
> Should we open separate bug (letter spacing, Antialiasing on big font, and
> Antialiasing on downloaded/non-Hinted font)

Yes, I think any remaining issues that currently exist need separate bugs.
Comment 34 Jeff Muizelaar [:jrmuizel] 2013-04-26 07:52:57 PDT
Comment on attachment 469999 [details] [diff] [review]
Read system cleartype pref v3

This already fixed. Dropping review.

Note You need to log in before you can comment on or make changes to this bug.