Bug 80471 – [chromium] wrong transform causing incorrect culling

• Status: RESOLVED
• Resolution: FIXED
• Component: Layout and Rendering

[Shawn Singh]

To reproduce: 
  1. run chromium with --enable-partial-swap on osx   (should probably repro on cros and linux osmesa, too)
  2. visit www.apple.com/iphone

Things disappear awkwardly.   Its likely this is either a culling or damage tracking issue.

I also visualized paint and damage rects, and the rects go a little crazy.  Because of that, its reasonable to assume that its a damage tracking bug (or two).  If it becomes clear that its a culling issue I'll hand it off to Dana.

[Shawn Singh]

Figured out the problem: in CCRenderPass, where the quad was being created for a RenderSurface, the quadTransforms was being initialized by the surface's drawTransform.   I believe this should actually be the originTransform instead, following the same pattern as other layers.

Without culling/damage tracking, it did not affect rendering because renderSurfaces draw themselves separately using the drawTransform, and they never used the quadTransform.

With both culling and damage tracking, however, things go wrong.  Culling (correctly) uses the quadTransform to determine the quad in target surface space; and, because the quadTransform was wrong, the quad becomes shifted incorrectly.  Therefore, when intersecting the damageRect, incorrect regions get culled.

Patch coming in just a moment.

[Shawn Singh]

Created attachment 131699 [details]
Patch

This patch also shuffles code around so that the fix can be more easily tested

[Dana Jansens]

Comment on attachment 131699 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=131699&action=review

+1

> Source/WebCore/platform/graphics/chromium/cc/CCRenderSurface.h:41
>  #include "cc/CCLayerQuad.h"
> +#include "cc/CCRenderPass.h"

Can this file forward-declare class CCSharedQuadState, and the .cpp includes CCSharedQuadState.h?

[James Robinson]

Comment on attachment 131699 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=131699&action=review

R=me. I like the forward decl idea as well.

> Source/WebKit/chromium/tests/CCRenderSurfaceTest.cpp:105
> +    // for reference, remove before committing
> +    // return CCSharedQuadState::create(originTransform(), drawTransform(), contentRect(), clipRect(), drawOpacity(), isOpaque);

don't forget to do this

[Shawn Singh]

Created attachment 131707 [details]
Patch for landing

[Shawn Singh]

Cool - thanks to both of you for the quick review =)

[WebKit Review Bot]

Comment on attachment 131707 [details]
Patch for landing

Clearing flags on attachment: 131707

Committed r110635: <http://trac.webkit.org/changeset/110635>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.