The default bug view has changed. See this FAQ.

[Azure]Subpixel AA is disabled

RESOLVED FIXED in mozilla15

Status

()

Core
Graphics
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: Leman Bennett [Omega], Assigned: bas)

Tracking

Trunk
mozilla15
x86_64
Windows 7
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(5 attachments)

(Reporter)

Description

5 years ago
Bas already knows but I'll file anyway.

Subpixel AA is not available when Azure-Thebes wrapper is enabled.
(Reporter)

Updated

5 years ago
Blocks: 715768
Duplicate of this bug: 718204
Status: UNCONFIRMED → NEW
Ever confirmed: true
(Assignee)

Comment 2

5 years ago
Created attachment 623551 [details] [diff] [review]
Part 1: Add helper for creating DWriteGlyphRuns
Assignee: nobody → bas.schouten
Status: NEW → ASSIGNED
Attachment #623551 - Flags: review?(jmuizelaar)
(Assignee)

Comment 3

5 years ago
Created attachment 623554 [details] [diff] [review]
Part 2: Refactor clip management and add ClippedMask texture

This patch refactors clip management a little to be more readible. It also adds the ClipMaskTexture which is a cached texture that contains the mask defined by the current clip.
(Assignee)

Comment 4

5 years ago
Created attachment 623555 [details] [diff] [review]
Part 3: Cache clipped geometry

As we start using GetClippedGeometry more with the patch to fix this bug, caching it is a good idea.
Attachment #623555 - Flags: review?(jmuizelaar)
(Assignee)

Updated

5 years ago
Attachment #623554 - Flags: review?(jmuizelaar)
(Assignee)

Comment 5

5 years ago
Created attachment 623558 [details] [diff] [review]
Part 4: Add code to draw subpixel-AA to transparent surfaces
Attachment #623558 - Flags: review?(jmuizelaar)
(Assignee)

Comment 6

5 years ago
Created attachment 623563 [details] [diff] [review]
Part 5: Manage subpixel AA permissions
Attachment #623563 - Flags: review?(roc)
Attachment #623563 - Flags: review?(roc) → review+
Comment on attachment 623551 [details] [diff] [review]
Part 1: Add helper for creating DWriteGlyphRuns

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

::: gfx/2d/HelpersD2D.h
@@ +227,5 @@
> +  mat.dy = aMatrix._32;
> +  return mat;
> +}
> +
> +class AutoDWriteGlyphRun : public DWRITE_GLYPH_RUN

Not a huge fan of the Auto in the name but can't think of anything better.

@@ +270,5 @@
> +  run->allocate(aGlyphs.mNumGlyphs);
> +
> +  FLOAT *advances = const_cast<FLOAT*>(run->glyphAdvances);
> +  UINT16 *indices = const_cast<UINT16*>(run->glyphIndices);
> +  DWRITE_GLYPH_OFFSET *offsets = const_cast<DWRITE_GLYPH_OFFSET*>(run->glyphOffsets);

The const_casts here are sort of ugly. Can we do anything about them?

@@ +272,5 @@
> +  FLOAT *advances = const_cast<FLOAT*>(run->glyphAdvances);
> +  UINT16 *indices = const_cast<UINT16*>(run->glyphIndices);
> +  DWRITE_GLYPH_OFFSET *offsets = const_cast<DWRITE_GLYPH_OFFSET*>(run->glyphOffsets);
> +
> +  memset(advances, 0, sizeof(FLOAT) * aGlyphs.mNumGlyphs);

It might be clearer to get rid of the memset() and do advances[i] = 0;
Attachment #623551 - Flags: review?(jmuizelaar) → review+
Attachment #623554 - Flags: review?(jmuizelaar) → review+
Comment on attachment 623558 [details] [diff] [review]
Part 4: Add code to draw subpixel-AA to transparent surfaces

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

::: gfx/2d/DrawTargetD2D.cpp
@@ +895,5 @@
> +      aOptions.mCompositionOp == OP_OVER && aPattern.GetType() == PATTERN_COLOR) {
> +    if (FillGlyphsManual(font, aBuffer,
> +                         static_cast<const ColorPattern*>(&aPattern)->mColor,
> +                         params, aOptions)) {
> +      return;

Do we expect this to fail? If not we should probably at least produce a Warning()
Attachment #623558 - Flags: review?(jmuizelaar) → review+
(Assignee)

Comment 9

5 years ago
(In reply to Jeff Muizelaar [:jrmuizel] from comment #7)
> Comment on attachment 623551 [details] [diff] [review]
> @@ +270,5 @@
> > +  run->allocate(aGlyphs.mNumGlyphs);
> > +
> > +  FLOAT *advances = const_cast<FLOAT*>(run->glyphAdvances);
> > +  UINT16 *indices = const_cast<UINT16*>(run->glyphIndices);
> > +  DWRITE_GLYPH_OFFSET *offsets = const_cast<DWRITE_GLYPH_OFFSET*>(run->glyphOffsets);
> 
> The const_casts here are sort of ugly. Can we do anything about them?

I can't think of anything, the constness is set in the inherited MS class.

> 
> @@ +272,5 @@
> > +  FLOAT *advances = const_cast<FLOAT*>(run->glyphAdvances);
> > +  UINT16 *indices = const_cast<UINT16*>(run->glyphIndices);
> > +  DWRITE_GLYPH_OFFSET *offsets = const_cast<DWRITE_GLYPH_OFFSET*>(run->glyphOffsets);
> > +
> > +  memset(advances, 0, sizeof(FLOAT) * aGlyphs.mNumGlyphs);
> 
> It might be clearer to get rid of the memset() and do advances[i] = 0;

I did this for performance reasons, I'm fine with going the other thing but I thought this would perform slightly better.
(Assignee)

Comment 10

5 years ago
(In reply to Jeff Muizelaar [:jrmuizel] from comment #8)
> Comment on attachment 623558 [details] [diff] [review]
> Part 4: Add code to draw subpixel-AA to transparent surfaces
> 
> Review of attachment 623558 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: gfx/2d/DrawTargetD2D.cpp
> @@ +895,5 @@
> > +      aOptions.mCompositionOp == OP_OVER && aPattern.GetType() == PATTERN_COLOR) {
> > +    if (FillGlyphsManual(font, aBuffer,
> > +                         static_cast<const ColorPattern*>(&aPattern)->mColor,
> > +                         params, aOptions)) {
> > +      return;
> 
> Do we expect this to fail? If not we should probably at least produce a
> Warning()

There's some realistic scenarios where this could fail (when outline drawing is recommended by DWrite for very large fonts and such). Failing is not a big issue as we'll just draw normally without subpixel-AA.
(Assignee)

Updated

5 years ago
Duplicate of this bug: 738106
Attachment #623555 - Flags: review?(jmuizelaar) → review+
(Assignee)

Comment 12

5 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/4ea09dad0f1a
https://hg.mozilla.org/integration/mozilla-inbound/rev/9d3b467dcd3e
https://hg.mozilla.org/integration/mozilla-inbound/rev/4678c61c8974
https://hg.mozilla.org/integration/mozilla-inbound/rev/c6be2987c6ec
https://hg.mozilla.org/integration/mozilla-inbound/rev/c14773057d57
https://hg.mozilla.org/mozilla-central/rev/4ea09dad0f1a
https://hg.mozilla.org/mozilla-central/rev/9d3b467dcd3e
https://hg.mozilla.org/mozilla-central/rev/4678c61c8974
https://hg.mozilla.org/mozilla-central/rev/c6be2987c6ec
https://hg.mozilla.org/mozilla-central/rev/c14773057d57
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla15
(Reporter)

Comment 14

5 years ago
This is causing graphics corruption in chrome and certain content javascript overlays.
I also suspect its crashing my ATI video chip HD3200, system locks hard and system going in/out of video card recovery, except it never recovers forcing a 'hard powerdown'
(Assignee)

Comment 16

5 years ago
(In reply to Jim Jeffery not reading bug-mail 1/2/11 from comment #15)
> I also suspect its crashing my ATI video chip HD3200, system locks hard and
> system going in/out of video card recovery, except it never recovers forcing
> a 'hard powerdown'

If that's normally never happens and it isn't blacklisted it's probably best if you raise a separate bug with your graphics info :(

(In reply to Leman Bennett (Omega X) from comment #14)
> This is causing graphics corruption in chrome and certain content javascript
> overlays.

Any chance you could point me at some STR? It would be great if you could open a bug on it with a screenshot.
(Assignee)

Comment 17

5 years ago
I found the likely cause, I didn't rerun the shader compiler to regenerate the shaders. Pushing a follow-up now.
(Assignee)

Comment 18

5 years ago
Pushed the regenerated files. I chose not to make a separate bug as that would probably confuse things. (This regeneration is really part of this bug and is a recompilation of code reviewed in this bug). If someone disagrees with that I apologize.

https://hg.mozilla.org/integration/mozilla-inbound/rev/5e3bd5a8cb3b
Can you file a bug for (and possibly implement) the build to fail if the shaders and the bytecode get out of sync?

Comment 20

5 years ago
I already did:  https://bugzilla.mozilla.org/show_bug.cgi?id=755741
(In reply to Bas Schouten (:bas) from comment #18)
> Pushed the regenerated files. 

https://hg.mozilla.org/mozilla-central/rev/5e3bd5a8cb3b
Depends on: 755741
Depends on: 756424

Updated

5 years ago
Depends on: 756427

Updated

5 years ago
No longer depends on: 756427

Updated

5 years ago
Depends on: 756427
(Assignee)

Updated

5 years ago
No longer blocks: 715768
(Assignee)

Updated

5 years ago
Blocks: 715768
You need to log in before you can comment on or make changes to this bug.