Bug 90308 - [TextureMapper] The TextureMapper should support edge-distance anti-antialiasing
Summary: [TextureMapper] The TextureMapper should support edge-distance anti-antialiasing
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Platform (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Martin Robinson
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-06-29 13:58 PDT by Martin Robinson
Modified: 2013-02-25 16:25 PST (History)
6 users (show)

See Also:


Attachments
Patch (34.23 KB, patch)
2012-06-29 15:33 PDT, Martin Robinson
no flags Details | Formatted Diff | Diff
Screenshot with edge-distance anti-aliasing enabled (61.67 KB, image/png)
2012-06-29 17:41 PDT, Martin Robinson
no flags Details
Screenshot without edge-distance anti-aliasing enabled (48.17 KB, image/png)
2012-06-29 17:41 PDT, Martin Robinson
no flags Details
Patch (35.45 KB, patch)
2012-06-29 17:42 PDT, Martin Robinson
no flags Details | Formatted Diff | Diff
Try to fix the Qt WebKit2 port (37.30 KB, patch)
2012-07-01 16:35 PDT, Martin Robinson
no flags Details | Formatted Diff | Diff
Try to fix the Qt WebKit2 port (37.30 KB, patch)
2012-07-01 16:44 PDT, Martin Robinson
no flags Details | Formatted Diff | Diff
Try one more time to fix the Qt WebKit2 build (37.76 KB, patch)
2012-07-01 17:54 PDT, Martin Robinson
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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