[Webkit-unassigned] [Bug 45092] [chromium] Accelerated Compositing: screen garbage when scrolling

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=45092





--- Comment #25 from Darin Fisher (:fishd, Google) <fishd at chromium.org>  2010-09-09 09:42:20 PST ---
(In reply to comment #15)
> > it seems like scrollRootLayer should also end with "Rect" to match the style of invalidateRootLayerRect.
> 
> Interesting thought. The one difference is that scrollLayer isn't actually sending a rectangle. I personally lean away from adding a rect suffix, but I'm ok either way. :)

Well, there is the clipping rectangle.  It is "scrolling the root layer within the given rectangle."


> > nit: elsewhere you enclose the isAcceleratedCompositingActive() call within #if USE(ACCELERATED_COMPOSITING).  might be good to be consistent with that.
> The rule I'm [trying] to follow is as follows...
> 
> If the code depends on accelerated compositing being ON, but does not itself use any code that is conditional on USE(ACCELERATED_COMPOSITING), I use isAcceleratedCompositingActive. E.g. render_widget.cc::DoDeferredUpdate's uses isAccelerated because the methods it dispatches to are always present.
> 
> In contrast, if the code uses functions or variables that are conditional on USE(ACCELERATED_COMPOSITING), I enclose the just the "dependent" part of the function in a USE(ACCELERERATED_COMPOSITING) test, leaving the isAccelerated test outside. E.g.:
>   if(isAcceleratedCompositing()) {
> #if USE(ACCELERATED_COMPOSITING)
>     someCompositorOnlyFunction();
> #endif
>   } else {
>     // software path
>   }
> To me at least, this seemed better than:
> #if USE(ACCELERATED_COMPOSITING)
>   if(isAcceleratedCompositing()) {
>     someCompositorOnlyFunction();
>   } else {
> #endif
>     // software path
> #if USE(ACCELERATED_COMPOSITING)
>   }
> #endif
> 
> I can change approach if there are strong opinions. :)

OK, that's fine.  Thanks for the explanation.


> > WebKit/chromium/src/WebViewImpl.cpp:956
> > +        invalidateRootLayerRect(rect);
> > why do we need to invalidate here?  it seems strange to invalidate during painting.
> I have no clue! Deleted! :)
> 
> > in the case where this paint method is being called to capture a thumbnail
> > for the new tab page in chromium, shouldn't we avoid the present step?
> 
> Good idea. I'll push the present() that is in the doComposite flow up to the ::paint function.
> The placeholder for James' readback code will also move up to ::paint.

OK


> > WebViewImpl::themeChanged:  you could also call FrameView::invalidate() on the main FrameView
> I switched over to this code. Afaik, I can call this directly, and it will route back to the view if needed. Does this seem right?

Yes.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.