[CoordinatedGraphics] Non-overlay scrollbar is not rendered
Created attachment 244256 [details] Patch
RenderLayerCompositor::mainFrameBackingIsTiled is checking the RenderLayerBacking uses tiledbacking dynamically. In the Coordinated Graphics, the main frame backing is always uses tiled backing store. So it should return true. I think we should refactor TILED_BACKING_STORE to match with Apple's tiledBacking at some point, and remove the TILED_BACKING_STORE guard in separated task.
I added gyuyoung to take a look. Informally looks good to me.
Comment on attachment 244256 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=244256&action=review > Source/WebCore/rendering/RenderLayerCompositor.cpp:2927 > +#if USE(TILED_BACKING_STORE) I removed WTF_USE_TILED_BACKING_STORE since r183529 Please change this to USE(COORDINATED_GRAPHICS)
Comment on attachment 244256 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=244256&action=review >> Source/WebCore/rendering/RenderLayerCompositor.cpp:2927 >> +#if USE(TILED_BACKING_STORE) > > I removed WTF_USE_TILED_BACKING_STORE since r183529 > > Please change this to USE(COORDINATED_GRAPHICS) The mainFrameBackingIsTiled() was changed to documentUsesTiledBacking() in r183943. This patch needs to be updated.
Comment on attachment 244256 [details] Patch r- since the patch is old. But are you sure the callers of documentUsesTiledBacking() are expecting this to return true for coordinated graphics?
Created attachment 256346 [details] Patch
Created attachment 256347 [details] Patch
ryuan, gyuyoung thanks for pointing out. I just updated to match current codebase.
(In reply to comment #6) > Comment on attachment 244256 [details] > Patch > > r- since the patch is old. But are you sure the callers of > documentUsesTiledBacking() are expecting this to return true for coordinated > graphics? Patch is updated. In Coordinated Graphics, we are just support overlay scrollbar at this point. To support non-overlay scrollbar, Coordinated Graphics should get true from documentUsesTiledBacking. And I'm pretty sure about it. :)
Comment on attachment 256347 [details] Patch LGTM for EFL port which uses the coordinated graphics. However I would delegate this review to Simon.
Comment on attachment 256347 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=256347&action=review > Source/WebCore/rendering/RenderLayerCompositor.cpp:2948 > + return true; This is pretty weird. Why does backing->usingTiledBacking() not return true? Why would you want this and that to return different things? Seems wrong.
(In reply to comment #12) > Comment on attachment 256347 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=256347&action=review > > > Source/WebCore/rendering/RenderLayerCompositor.cpp:2948 > > + return true; > > This is pretty weird. Why does backing->usingTiledBacking() not return true? > Why would you want this and that to return different things? Seems wrong. Thanks for review. First of all, it is because CoordinatedGraphics uses TiledBackingStore, not a TiledBacking. Coordinated Graphics manages tiles in GraphicsLayer, but TiledBacking is used in RenderLayer. There are several logics in RenderLayerBacking and RenderLayerCompositor which are specific TiledBacking implementation. So if we just return true in RenderLayerBacking::usingTiledBacking, it will introduce undesirable behavior in CoordinatedGraphics. That's why I want to add it this way. In a long-term, it is desirable to merge TiledBackingStore in to TiledBacking. It is weird that we have two similar implementation with similar name. And it will solve these weird stuffs. I'll do this work after turning threaded-compositor on in GTK+ port.
Pretty sure this bug is obsolete now!