[Webkit-unassigned] [Bug 87167] [chromium] Cleanup scissor rect computation/use with damage

[bugzilla-daemon at webkit.org]

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





--- Comment #26 from Dana Jansens <danakj at chromium.org>  2012-05-30 19:47:59 PST ---
(In reply to comment #25)
> (In reply to comment #23)
> > rootSurfaceDamageInLocalSurfaceSpace == surfaceSwapRect, right? Can we call it that instead? Then we're saying scissor = intersection(clip, swapRect) which is more precise yes?
> > 
> 
> so based on my previous comment - I think they are not the same at all... surfaceSwap in my opinion doesn't have meaning, the swap concept doesn't exist on surfaces.  So I would say it is more precise to say intersection(clip, rootSurfaceDamageInLocalSurfaceSpace).

Shawn and I spoke offline about this naming stuff.. calling it "swap" rect is correct now, but he has plans that will make it less-so in the future. We arrived at the following naming scheme:

- the rect in root space that is the aggregate of all damage is the "rootScissorRect". (this is currently called "rootDamageRect" in the code which is a misnomer.)

- this same rect, transformed to a non-root surface is called "rootScissorRectInCurrentSurface" (or ...TargetSurface etc if appropriate). This indicates that its the rootScissorRect transformed to some other surface.

Can we use these names throughout this patch? Sorry for the name churn :/

> > > At the very least, how about we put this inside of calculateRenderSurfaceLayerList() so that we don't need to return that local rootDamageRect?  It makes sense for that line to be as closely grouped to calculateVisibleAndScissorRects as possible, if its not actually be computed inside of it.
> > 
> > The return value is actually the rootSwapRect right now, not the rootDamageRect I guess. This return value could also be used by the CCOcclusionTracker instead of recomputing it again below.
> > 
> > I agree the rootLayer's scissor should be set in CCLayerTreeHostCommon::calculateVisibleAndScissorRects. Right now it's not being set on the main thread right? If it's inside there then it would be always set when the others are set.
> > 
> 
> Is it possible the swap/scissor/damage definition clarification changes what you're saying here?  If you still think we need this return value, lets discuss offline and then update this bug with a summary of our discussion?

We use the rootScissorRect as input for the CCOcclusionTracker, so it'd be nice to get that from the calculateRenderSurfaceLayerList function somehow instead of computing it again. Perhaps just using a reference parameter would feel nicer instead of a return value, as it has a name. The meaning of a FloatRect being returned from calculateRSLL is a bit vague.

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