Bug 110299

Summary: [WK2] add setNeedsDisplay in DrawingArea to mark whole layer as dirty.
Product: WebKit Reporter: Dongseong Hwang <dongseong.hwang>
Component: WebKit2Assignee: Dongseong Hwang <dongseong.hwang>
Status: RESOLVED FIXED    
Severity: Normal CC: andersca, benjamin, cmarcelo, kenneth, luiz, noam, simon.fraser, webkit.review.bot, zeno
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 110066, 110311    
Attachments:
Description Flags
Patch none

Description Dongseong Hwang 2013-02-19 21:41:31 PST
Currently, we call setNeedsDisplay with the size of WebPage to mark whole
backing store as dirty. However, the size of non compositing layer can be
inconsistent with the size of WebPage. For example, in Coordinated Graphics, the
size of non compositing layer is contents size.

So, WebPage should not assume the size of non compositing layer, and should just
call setNeedsDisplay when marking whole layer as dirty.

In addition, this patch renames from setNeedsDisplay() with a rect argument to
setNeedsDisplayInRect(), which matches to the terms of GraphicsLayer.
Comment 1 Dongseong Hwang 2013-02-19 21:46:38 PST
Created attachment 189234 [details]
Patch
Comment 2 Anders Carlsson 2013-02-20 09:45:16 PST
Comment on attachment 189234 [details]
Patch

Looks good. Longer term I'd recommend that you'd get rid of the non-composited contents and just always enter accelerated compositing mode - this is what we do on Mac. That'd allow us to simplify a lot of code.
Comment 3 WebKit Review Bot 2013-02-20 10:12:05 PST
Comment on attachment 189234 [details]
Patch

Clearing flags on attachment: 189234

Committed r143472: <http://trac.webkit.org/changeset/143472>
Comment 4 WebKit Review Bot 2013-02-20 10:12:08 PST
All reviewed patches have been landed.  Closing bug.
Comment 5 Noam Rosenthal 2013-02-20 10:22:50 PST
(In reply to comment #2)
> (From update of attachment 189234 [details])
> Looks good. Longer term I'd recommend that you'd get rid of the non-composited contents and just always enter accelerated compositing mode - this is what we do on Mac. That'd allow us to simplify a lot of code.

I agree. Created but https://bugs.webkit.org/show_bug.cgi?id=110355
Comment 6 Dongseong Hwang 2013-02-26 16:54:37 PST
(In reply to comment #2)
> (From update of attachment 189234 [details])
> Looks good. Longer term I'd recommend that you'd get rid of the non-composited contents and just always enter accelerated compositing mode - this is what we do on Mac. That'd allow us to simplify a lot of code.

It seems right direction! could you explain more detail?

After I read your comment, I thought LayerTreeHostMac does not have nonCompositedContentsLayer and RenderLayerCompositor::rootLayer deals with non-composited contents. But in upstream code, LayerTreeHostMac is very similar to CoordinatedLayerTreeHost; both have nonCompositedContentsLayer.
Could you let me know what bug or revision simplified a lot of code on Mac?

Thanks in advance.
Comment 7 Anders Carlsson 2013-02-26 16:59:47 PST
(In reply to comment #6)
> (In reply to comment #2)
> > (From update of attachment 189234 [details] [details])
> > Looks good. Longer term I'd recommend that you'd get rid of the non-composited contents and just always enter accelerated compositing mode - this is what we do on Mac. That'd allow us to simplify a lot of code.
> 
> It seems right direction! could you explain more detail?

Noam filed a radar about this in https://bugs.webkit.org/show_bug.cgi?id=110355 - it should have some information.

> 
> After I read your comment, I thought LayerTreeHostMac does not have nonCompositedContentsLayer and RenderLayerCompositor::rootLayer deals with non-composited contents. But in upstream code, LayerTreeHostMac is very similar to CoordinatedLayerTreeHost; both have nonCompositedContentsLayer.
> Could you let me know what bug or revision simplified a lot of code on Mac?

Sure! We actually only use LayerTreeHostMac on OS X Lion. On Mountain Lion we use a different drawing area, the TiledCoreAnimationDrawingArea.
Comment 8 Dongseong Hwang 2013-02-26 17:16:16 PST
(In reply to comment #7)
> (In reply to comment #6)
> Sure! We actually only use LayerTreeHostMac on OS X Lion. On Mountain Lion we use a different drawing area, the TiledCoreAnimationDrawingArea.

aha, TiledCoreAnimationDrawingArea is key. Thanks.