Bug 90308 – [TextureMapper] The TextureMapper should support edge-distance anti-antialiasing

• Status: RESOLVED
• Resolution: FIXED
• Component: Platform

[Martin Robinson]

Currently textures are drawn with no anti-aliasing, which can lead to jaggies on the edges of projected layers. The Chromium compositor has a really slick edge-distance antialiasing approach that we can port to the TextureMapper.

[Martin Robinson]

Created attachment 150265 [details]
Patch

[Noam Rosenthal]

Comment on attachment 150265 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=150265&action=review

> Source/WebCore/ChangeLog:10
> +        Add an edge-distance anti-aliasing implementation for the TextureMapper. Currently
> +        this implementation is not active for tiled layers.
> +

Please explain more about the effect this produces.

> Source/WebCore/platform/graphics/texmap/TextureMapper.h:121
> +        AllEdges = LeftEdge + RightEdge + TopEdge + BottomEdge,

Use | instead of +

> Source/WebCore/platform/graphics/texmap/TextureMapper.h:125
> +    virtual void drawTexture(const BitmapTexture&, const FloatRect& target, const TransformationMatrix& modelViewMatrix = TransformationMatrix(), float opacity = 1.0f, const BitmapTexture* maskTexture = 0, const unsigned exposedEdges = AllEdges) = 0;

You don't need the word const

> Source/WebCore/platform/graphics/texmap/TextureMapperBackingStore.cpp:114
> +        exposedEdges += TextureMapper::LeftEdge;

|=

> Source/WebCore/platform/graphics/texmap/TextureMapperGL.cpp:416
> +    if (drawTextureWithAntialiasing(texture, flags, targetRect, modelViewMatrix, opacity, maskTexture, exposedEdges))
> +       return;
> +

do we really want to

> Source/WebCore/platform/graphics/texmap/TextureMapperGL.cpp:432
> +    TransformationMatrix screen;

The name screen is confusing, how about just "matrix" or "newMatrix"

> Source/WebCore/platform/graphics/texmap/TextureMapperShaderManager.cpp:197
> +PassRefPtr<TextureMapperShaderProgramSimpleAntialiased> TextureMapperShaderManager::simpleAntialiasedProgram()

Nothing simple about this :) Maybe antialiasedNoMask?

> Source/WebCore/platform/graphics/texmap/TextureMapperShaderManager.h:185
> +

Extra line

[Martin Robinson]

Created attachment 150280 [details]
Screenshot with edge-distance anti-aliasing enabled

[Martin Robinson]

Created attachment 150281 [details]
Screenshot without edge-distance anti-aliasing enabled

[Martin Robinson]

Created attachment 150282 [details]
Patch

[Martin Robinson]

(In reply to comment #2)
> (From update of attachment 150265 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=150265&action=review
> 
> > Source/WebCore/ChangeLog:10
> > +        Add an edge-distance anti-aliasing implementation for the TextureMapper. Currently
> > +        this implementation is not active for tiled layers.
> > +
> 
> Please explain more about the effect this produces.

Okay I explained a bit more what it does (and attached screenshots here). I also gave credit to David Raveman for his initial implementation of this in the Chromium compositor.


> > Source/WebCore/platform/graphics/texmap/TextureMapper.h:121
> > +        AllEdges = LeftEdge + RightEdge + TopEdge + BottomEdge,
> 
> Use | instead of +

Done.

> > Source/WebCore/platform/graphics/texmap/TextureMapper.h:125
> > +    virtual void drawTexture(const BitmapTexture&, const FloatRect& target, const TransformationMatrix& modelViewMatrix = TransformationMatrix(), float opacity = 1.0f, const BitmapTexture* maskTexture = 0, const unsigned exposedEdges = AllEdges) = 0;
> 
> You don't need the word const

Removed.

> > Source/WebCore/platform/graphics/texmap/TextureMapperBackingStore.cpp:114
> > +        exposedEdges += TextureMapper::LeftEdge;
> 
> |=

Done.

> > Source/WebCore/platform/graphics/texmap/TextureMapperGL.cpp:416
> > +    if (drawTextureWithAntialiasing(texture, flags, targetRect, modelViewMatrix, opacity, maskTexture, exposedEdges))
> > +       return;
> > +
> 
> do we really want to

I believe so. If drawing the layer with anti-aliasing succeeded then we shouldn't draw it again.

> > Source/WebCore/platform/graphics/texmap/TextureMapperGL.cpp:432
> > +    TransformationMatrix screen;
> 
> The name screen is confusing, how about just "matrix" or "newMatrix"

I changed it to "matrix."

> > Source/WebCore/platform/graphics/texmap/TextureMapperShaderManager.cpp:197
> > +PassRefPtr<TextureMapperShaderProgramSimpleAntialiased> TextureMapperShaderManager::simpleAntialiasedProgram()
> 
> Nothing simple about this :) Maybe antialiasedNoMask?

Fair enough. :) I changed it to "antialiasingNoMask"

> > Source/WebCore/platform/graphics/texmap/TextureMapperShaderManager.h:185
> > +
> 
> Extra line

Removed.

I made a few other changes to this patch. I added a setting on TextureMapperGL to turn this on and off and made it off by default. This will allow us to experiment with it and turn it on gradually. I also modified the code so that when the model view transform is just an integer translation anti-aliasing is not active.

[Early Warning System Bot]

Comment on attachment 150282 [details]
Patch

Attachment 150282 [details] did not pass qt-wk2-ews (qt):
Output: http://queues.webkit.org/results/13119535

[Noam Rosenthal]

Comment on attachment 150282 [details]
Patch

Doesn't compile. at least according to EWS :)

[Martin Robinson]

Created attachment 150341 [details]
Try to fix the Qt WebKit2 port

[Early Warning System Bot]

Comment on attachment 150341 [details]
Try to fix the Qt WebKit2 port

Attachment 150341 [details] did not pass qt-wk2-ews (qt):
Output: http://queues.webkit.org/results/13115946

[Martin Robinson]

Created attachment 150344 [details]
Try to fix the Qt WebKit2 port

[Early Warning System Bot]

Comment on attachment 150344 [details]
Try to fix the Qt WebKit2 port

Attachment 150344 [details] did not pass qt-wk2-ews (qt):
Output: http://queues.webkit.org/results/13120877

[Martin Robinson]

Created attachment 150349 [details]
Try one more time to fix the Qt WebKit2 build

[Noam Rosenthal]

Comment on attachment 150349 [details]
Try one more time to fix the Qt WebKit2 build

Awesome. We can experiment later with enabling it for the Qt port.

[WebKit Review Bot]

Comment on attachment 150349 [details]
Try one more time to fix the Qt WebKit2 build

Clearing flags on attachment: 150349

Committed r121729: <http://trac.webkit.org/changeset/121729>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.

[Dongseong Hwang]

awesome mrobinson :)
http://abandonedwig.info/blog/2013/02/24/edge-distance-anti-aliasing.html