Bug 162704 – [Coordinated Graphics] Debug Visuals don't hide

• Component: Layout and Rendering
• Status: RESOLVED FIXED

[Yoshiaki Jitsukawa]

In the EFL mini browser, the debug visuals (borders and repaint counters) show when the "show compositing borders" button on the inspector is pressed. However the visuals don't hide when the button is pressed again.

[Yoshiaki Jitsukawa]

Created attachment 290144 [details]
Patch

[Gwang Yoon Hwang]

Comment on attachment 290144 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=290144&action=review

> Source/WebCore/platform/graphics/texmap/coordinated/CoordinatedGraphicsLayer.cpp:478
> +    m_layerState.debugVisuals.showDebugBorders = show;

Nice finding! But I think it is enough to remove this line:
m_layerState.showDebugBorders = true;

Because CoordinatedGraphicsLayer::syncLayerState will set the proper value to the m_layerState.showDebugBorder

> Source/WebCore/platform/graphics/texmap/coordinated/CoordinatedGraphicsLayer.cpp:490
> +    m_layerState.debugVisuals.showRepaintCounter = show;

Same here.

> Source/WebCore/platform/graphics/texmap/coordinated/CoordinatedGraphicsState.h:79
> +    float debugBorderWidth;

Nowadays, we initialize members like this:
float debugBorderWidth { 0 };

> Source/WebCore/platform/graphics/texmap/coordinated/CoordinatedGraphicsState.h:87
> +};

BTW, do you have any other reason to make a new structure for debug visual?
Even in current implementation, we sent these changes to the compositing thread with a single commit.

[Yoshiaki Jitsukawa]

(In reply to Gwang Yoon Hwang from comment #2)
> Comment on attachment 290144 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=290144&action=review
> 
> > Source/WebCore/platform/graphics/texmap/coordinated/CoordinatedGraphicsLayer.cpp:478
> > +    m_layerState.debugVisuals.showDebugBorders = show;
> 
> Nice finding! But I think it is enough to remove this line:
> m_layerState.showDebugBorders = true;
> 
> Because CoordinatedGraphicsLayer::syncLayerState will set the proper value
> to the m_layerState.showDebugBorder
> 

Thanks for reviewing. Yes, actually the minimum fix is like:

-    m_layerState.showDebugBorders = true;
+    m_layerState.debugBorderColorChanged = true; // This forces the debug visuals update

Without setting the debugBorderColorChanged flag the debug visuals are not updated because the code to change the visuals refers to only the "debugBorderColorChanged" and "debugBorderWidthChanged" flags

   if (layerState.debugBorderColorChanged || layerState.debugBorderWidthChanged)
	        layer->setDebugVisuals(layerState.showDebugBorders, layerState.debugBorderColor, layerState.debugBorderWidth, layerState.showRepaintCounter);


> BTW, do you have any other reason to make a new structure for debug visual?
> Even in current implementation, we sent these changes to the compositing
> thread with a single commit.

So, at least, with the current implementation, we need to add two update flags for the "showDebugBorders" and "showRepaintCounter" state. But instead, I made a new structure for debug visuals and an update flag for it.

[Michael Catanzaro]

Yoon, what do you think?

[Gwang Yoon Hwang]

(In reply to Yoshiaki Jitsukawa from comment #3)
> (In reply to Gwang Yoon Hwang from comment #2)
> > Comment on attachment 290144 [details]
> > Patch
> > 
> > View in context:
> > https://bugs.webkit.org/attachment.cgi?id=290144&action=review
> > 
> 
> Without setting the debugBorderColorChanged flag the debug visuals are not
> updated because the code to change the visuals refers to only the
> "debugBorderColorChanged" and "debugBorderWidthChanged" flags
> 
Oh, nice. Than I agree with your change.
If you update the initialize code, I like the patch. :)

[Yoshiaki Jitsukawa]

Created attachment 309477 [details]
Patch

[Yoshiaki Jitsukawa]

(In reply to Gwang Yoon Hwang from comment #5)
> Oh, nice. Than I agree with your change.
> If you update the initialize code, I like the patch. :)

Thank you! The init code updated.

[WebKit Commit Bot]

Comment on attachment 309477 [details]
Patch

Clearing flags on attachment: 309477

Committed r216502: <http://trac.webkit.org/changeset/216502>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.