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.
Created attachment 290144 [details] Patch
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.
(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.
Yoon, what do you think?
(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. :)
Created attachment 309477 [details] Patch
(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.
Comment on attachment 309477 [details] Patch Clearing flags on attachment: 309477 Committed r216502: <http://trac.webkit.org/changeset/216502>
All reviewed patches have been landed. Closing bug.