[webkit-reviews] review denied: [Bug 105787] [Qt][EFL][GTK][TexMap] Refactor code related to debug border and repaint count. : [Attachment 180852] 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 180852: Patch : [TexMap] Refactor code related to debug border and
repaint count. [1/2] 
https://bugs.webkit.org/attachment.cgi?id=180852&action=review

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


I'm doing some work on mask code, which would make it so that we actually
remove the "mask" parameter from textureMapperPlatformLayer.
I'm thinking overloads of TextureMapperLayer should *always* support only
transform and opacity, and for everything else they should provide a
BitmapTexture and let TextureMapperLayer handle it.

>> Source/WebCore/platform/graphics/texmap/TextureMapperLayer.cpp:33
>> +	TextureMapperPaintParameters parameters;
> 
> I don't store targetRect in TextureMapperPaintParameters because it is weird
for TextureMapperPaintOptions to have targetRect as well as the tile related
reason as I mentioned earlier.
> 
> btw, I encounter new problem. I will add new member into
TextureMapperPaintParameters:
>     bool showDebugBorders;
>     Color debugBorderColor;
>     float debugBorderWidth;
>     bool showRepaintCounter;
>     int repaintCount;
> 
> But it is weird for TextureMapperPaintOptions to have those new members.
> 
> IMO, TextureMapperPaintOptions has duplicated members although it collides
TextureMapperPaintParameters because both class's purpose is different.
> How do you think?

I think it's ok to have duplicated members. I meant they collide in a way where
it's hard for a person reading the code to understand which is which.
Regarding the repaint counters - it should not be part of the paint parameters.
It should be part of the "updateContents" process.