[webkit-reviews] review denied: [Bug 105787] [Qt][EFL][GTK][TexMap] Refactor code related to debug border and repaint count. : [Attachment 180783] Patch : [TexMap] Refactor code related to debug border and repaint count. [1/2]

[bugzilla-daemon at webkit.org]

Noam Rosenthal <noam at webkit.org> has denied Huang Dongsung
<luxtella at company100.net>'s request for review:
Bug 105787: [Qt][EFL][GTK][TexMap] Refactor code related to debug border and
repaint count.
https://bugs.webkit.org/show_bug.cgi?id=105787

Attachment 180783: Patch : [TexMap] Refactor code related to debug border and
repaint count. [1/2]
https://bugs.webkit.org/attachment.cgi?id=180783&action=review

------- Additional Comments from Noam Rosenthal <noam at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=180783&action=review


I wanted to do this for ages but was lazy :)
However I have some comments.

> Source/WebCore/platform/graphics/texmap/TextureMapperLayer.cpp:121
> +    TextureMapperPaintParameters parameters;
> +    parameters.modelViewMatrix = transform;
> +    parameters.opacity = options.opacity;
> +    parameters.mask = options.mask.get();

Parameters and options are colliding here.
We should move TextureMapperPaintOptions to be defined in
TextureMapperLayer.cpp, and have an internal member of
TextureMapperPaintParameters.

> Source/WebCore/platform/graphics/texmap/TextureMapperPlatformLayer.h:32
> +class TextureMapperPaintParameters {

This should be in TextureMapper.h, maybe later we'd want to change the
signature of TextureMapper functions to use it.

> Source/WebCore/platform/graphics/texmap/TextureMapperPlatformLayer.h:34
> +    TransformationMatrix modelViewMatrix;

transform instead of modelViewMatrix.

> Source/WebCore/platform/graphics/texmap/TextureMapperPlatformLayer.h:36
> +    BitmapTexture* mask;

RefPtr

> Source/WebCore/platform/graphics/texmap/TextureMapperPlatformLayer.h:38
> +    TextureMapperPaintParameters()

Any reason why targetRect is left out?