Closed Bug 574976 Opened 14 years ago Closed 14 years ago

enabling direct2d makes fonts appear as Cleartype regardless of system Cleartype prefs

Categories

(Core :: Graphics, defect)

x86_64
Windows 7
defect
Not set
normal

Tracking

()

RESOLVED FIXED
Tracking Status
blocking2.0 --- final+

People

(Reporter: leeoniya, Assigned: bas.schouten)

References

Details

Attachments

(3 files, 2 obsolete files)

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.
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.
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.
Component: General → Graphics
QA Contact: general → thebes
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.
> 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).
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.
(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.
Sounds like whatever place we select it we may need to check the OS pref.  :(
Status: UNCONFIRMED → NEW
blocking2.0: --- → ?
Ever confirmed: true
See also: Bug 517642
blocking2.0: ? → final+
Assignee: nobody → bas.schouten
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.
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
As far as I can see the only solution is to disable D2D for now.(set mozilla.widget.render-mode to 0)
should we change the title of the bug to be more on clear / keywords?

D2D/direct2d/antialiasing/cleartype/fuzzy/blurry/text/fonts
(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).
(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.
Attached patch Read system cleartype pref (obsolete) — Splinter Review
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.
Attachment #469141 - Flags: review?(jmuizelaar)
(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.
Attached patch Read system cleartype pref v2 (obsolete) — Splinter Review
Adjusted as per discussion.
Attachment #469141 - Attachment is obsolete: true
Attachment #469954 - Flags: review?(jmuizelaar)
Attachment #469141 - Flags: review?(jmuizelaar)
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()
Attachment #469954 - Flags: review?(jmuizelaar) → review-
Attachment #469954 - Attachment is obsolete: true
Attachment #469999 - Flags: review?(jmuizelaar)
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?
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).
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
Blocks: 594325
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.
Just to add, MS's hotfix doesn't solve the problem.
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
(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.
(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.
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)
(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.
http://hg.mozilla.org/mozilla-central/rev/f773f74f28d1
Status: NEW → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
(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 on attachment 469999 [details] [diff] [review]
Read system cleartype pref v3

This already fixed. Dropping review.
Attachment #469999 - Flags: review?(jmuizelaar)
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: