[webkit-reviews] review granted: [Bug 56237] [chromium] backface visibility doesn't work correctly on perspective-projected layers : [Attachment 85563] Patch

[bugzilla-daemon at webkit.org]

Kenneth Russell <kbr at google.com> has granted Vangelis Kokkevis
<vangelis at chromium.org>'s request for review:
Bug 56237: [chromium] backface visibility doesn't work correctly on
perspective-projected layers
https://bugs.webkit.org/show_bug.cgi?id=56237

Attachment 85563: Patch
https://bugs.webkit.org/attachment.cgi?id=85563&action=review

------- Additional Comments from Kenneth Russell <kbr at google.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=85563&action=review

Looks good to me. One comment.

> Source/WebCore/platform/graphics/chromium/LayerRendererChromium.cpp:815
> +	   FloatQuad mappedLayer =
combinedDrawMatrix.mapQuad(FloatQuad(layerRect));
> +	   FloatSize horizontalDir = mappedLayer.p2() - mappedLayer.p1();
> +	   FloatSize verticalDir = mappedLayer.p4() - mappedLayer.p1();
> +	   FloatPoint3D xAxis(horizontalDir.width(), horizontalDir.height(),
0);
> +	   FloatPoint3D yAxis(verticalDir.width(), verticalDir.height(), 0);
> +	   FloatPoint3D zAxis = xAxis.cross(yAxis);

It seems odd to do these operations in 2D. Intuitively I would transform the
four points of the quad in 3D (probably without the perspective portion of the
matrix) and then do the appropriate vector differences and cross products.
However I gather that TransformationMatrix doesn't provide enough control to do
this. As long as you've tested this math to your satisfaction I'm sure it's OK.