[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 #5 from Nat Duca <nduca at chromium.org>  2010-09-03 11:07:21 PST ---
>> -        ASSERT(rootLayerWidth == updateRect.width() && rootLayerHeight == updateRect.height());
> I'm not sure I follow here.  Is the updateRect supposed to match the root layer size? I thought update rects
> can be smaller. Shouldn't rootLayerWidth == m_rootLayerTextureWidth?  You are asserting a different condition
> in the lines above.

Oops, bad variable naming. What should be asserted here is that the canvas containing the update bitmap should match the updateRect that we were handed. I renamed these variables to be bitmapWidth/etc to make the intent clearer.

> Don't you need to bind the root layer texture before calling this? 
Good point.


>> +    // ASSERT(static_cast<int>(CGBitmapContextGetWidth(m_rootLayerCGContext.get())) == updateRect.width() && static_cast<int>(CGBitmapContextGetHeight(m_rootLayerCGContext.get())) == updateRect.height());
>> +    void* pixels = m_rootLayerBackingStore.data();

>If this ASSERT is not necessary, please remove rather than commenting out.
This assert came from the existing code --- I'm simply moved this function rather than added the assertion.
I suggest we leave it there for the time being --- it looks like it is analogous to the SKIA "check the bitmap is the correct size" function.




> Does viewport specify the region of the WebWidget that will be grabbed or the place where it's
> going to end up on the canvas? The comment seems to indicate both but I'm not sure that's correct.
Updated my comments to be clearer on this. Net/net, the rectangle "viewport" on WebWidget will be drawn to viewport.x,viewport.y on the provided canvas. Put another way, the viewport will *not* be translated to the origin during drawing.


> I don't have the code handy but I seem to remember that isAcceleratedCompositingActive() is defined only if USE(ACCELERATED_COMPOSTING) is.
Thankfully, it is available at all times. I actually think this really helpful since it allows us to avoid monstrosities like:
#if USE(ACCELERATED_COMPOSITING)
if(isAcceleratedCompositingEnabled()) {
} else {
#endif
  ...
#if USE(ACCELERATED_COMPOSITING)
}
#endif



>Is it not possible that you could have a scrollDelta with non-zero width _and_ height? 
Great catch. I will invalidate in this case.

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