[webkit-reviews] review denied: [Bug 90308] [TextureMapper] The TextureMapper should support edge-distance anti-antialiasing : [Attachment 150265] Patch

[bugzilla-daemon at webkit.org]

Noam Rosenthal <noam.rosenthal at nokia.com> has denied Martin Robinson
<mrobinson at webkit.org>'s request for review:
Bug 90308: [TextureMapper] The TextureMapper should support edge-distance
anti-antialiasing
https://bugs.webkit.org/show_bug.cgi?id=90308

Attachment 150265: Patch
https://bugs.webkit.org/attachment.cgi?id=150265&action=review

------- Additional Comments from Noam Rosenthal <noam.rosenthal at nokia.com>
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