Bug 90308

Summary: [TextureMapper] The TextureMapper should support edge-distance anti-antialiasing
Product: WebKit Reporter: Martin Robinson <mrobinson>
Component: PlatformAssignee: Martin Robinson <mrobinson>
Status: RESOLVED FIXED    
Severity: Normal CC: alex, dongseong.hwang, enne, noam, reveman, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Screenshot with edge-distance anti-aliasing enabled
none
Screenshot without edge-distance anti-aliasing enabled
none
Patch
none
Try to fix the Qt WebKit2 port
none
Try to fix the Qt WebKit2 port
none
Try one more time to fix the Qt WebKit2 build none

Description Martin Robinson 2012-06-29 13:58:37 PDT
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.
Comment 1 Martin Robinson 2012-06-29 15:33:25 PDT
Created attachment 150265 [details]
Patch
Comment 2 Noam Rosenthal 2012-06-29 16:00:24 PDT
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
Comment 3 Martin Robinson 2012-06-29 17:41:30 PDT
Created attachment 150280 [details]
Screenshot with edge-distance anti-aliasing enabled
Comment 4 Martin Robinson 2012-06-29 17:41:49 PDT
Created attachment 150281 [details]
Screenshot without edge-distance anti-aliasing enabled
Comment 5 Martin Robinson 2012-06-29 17:42:19 PDT
Created attachment 150282 [details]
Patch
Comment 6 Martin Robinson 2012-06-29 17:45:45 PDT
(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.
Comment 7 Early Warning System Bot 2012-06-29 17:54:04 PDT
Comment on attachment 150282 [details]
Patch

Attachment 150282 [details] did not pass qt-wk2-ews (qt):
Output: http://queues.webkit.org/results/13119535
Comment 8 Noam Rosenthal 2012-06-30 16:34:20 PDT
Comment on attachment 150282 [details]
Patch

Doesn't compile. at least according to EWS :)
Comment 9 Martin Robinson 2012-07-01 16:35:41 PDT
Created attachment 150341 [details]
Try to fix the Qt WebKit2 port
Comment 10 Early Warning System Bot 2012-07-01 16:42:07 PDT
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
Comment 11 Martin Robinson 2012-07-01 16:44:09 PDT
Created attachment 150344 [details]
Try to fix the Qt WebKit2 port
Comment 12 Early Warning System Bot 2012-07-01 17:05:31 PDT
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
Comment 13 Martin Robinson 2012-07-01 17:54:57 PDT
Created attachment 150349 [details]
Try one more time to fix the Qt WebKit2 build
Comment 14 Noam Rosenthal 2012-07-01 18:18:26 PDT
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.
Comment 15 WebKit Review Bot 2012-07-02 21:25:53 PDT
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>
Comment 16 WebKit Review Bot 2012-07-02 21:25:58 PDT
All reviewed patches have been landed.  Closing bug.
Comment 17 Dongseong Hwang 2013-02-25 16:25:01 PST
awesome mrobinson :)
http://abandonedwig.info/blog/2013/02/24/edge-distance-anti-aliasing.html