User Agent: Opera/9.80 (Macintosh; Intel Mac OS X 10.8.5) Presto/2.12.388 Version/12.16 Steps to reproduce: 1. Use Firefox with HW acceleration disabled - either disable it through settings on any platform or use Firefox on Linux or older Windows computers. 2. Visit: http://walk-inside.com/sample1/?format=0&size=2 3. Try dragging panorama with a mouse to look up or down or pan right-left. Actual results: Attached is a screenshot of a cubic panorama viewer where 6 image tiles are placed on sides of a cube and 3D rotation is applied to display spherical panorama. 3D transforms are implemented through CSS 3D styles and they get computed incorrectly with respect to 3D rotation angles. Note in the attached screenshot that one tile is upside down and huge gaps exist between tiles. At some rotation angles some tiles fail to get displayed at all. Also, no antialiasing is available. Expected results: Correctly displayed cubic tiles resulting in a complete and continuous panoramic image with good antialiasing. Chrome used to have garbled 3D display as well when HW acceleration was not available, but got 99% fixed since then - while it is a bit slow without HW acceleration, but displays a proper panorama with good antialiasing and only and 1% of angles has display issues.

I can also see the problem on Windows7, HWA disabled. And it seems happen since Firefox10(CSS 3D Transforms are supported)

Milan, is this on the graphics team's radar?

I think we know about this, although I could be mixing things up. Disabling hardware acceleration gets us into basic compositor, but I believe leaves us in OMTC scenario. David and/or Brian would know the current status of this.

I don't know anything about this.

Sorry, I don't know anything about this either.

Matt is up by now... :)

Ugh, looks like this is yet another 3d transforms visibility computation bug. We're computing that only the left 20 pixels of the image is visible, when that's clearly not the case. With hardware acceleration enabled we don't actually use the visible region for compositing ImageLayers so it doesn't matter, with hardware acceleration disabled it breaks. nsDisplayTransform p=0x12ced8be8 f=0x1314dbc78(ImageFrame(img)(1)) bounds(-782510,-73119,839882,700997) visible(27120,4740,30252,40800) componentAlpha(0,0,0,0) clip(27120,4740,51060,40800) [ -0.931039 -0.809677 0.776088 -0.00172464; -0.223433 0.889978 0.111469 -0.000247709; -2.0251 -0.647558 0.620695 -0.00137932; 1428.78 254.618 -228.093 1.50687; ] layer=0x1312d2000 Image p=0x12ced8b98 f=0x1314dbc78(ImageFrame(img)(1)) bounds(0,0,54000,54000) visible(0,0,1200,54000) componentAlpha(0,0,0,0) clip() (opaque 0,0,54000,54000) layer=0x1312d2400

The reftest added to Bug 1035611 appears to fail when HWA is disabled, and may be a useful test for this bug as well.

I'm taking this, as it is similar to Bug 1035611, which I have just corrected

It appears that gfx3DMatrix::TransformBounds is very similar to Matrix4x4::ProjectRectBounds and may need to be updated in a similar way to the fix in Bug 1035611.

This bug is caused by two issues: - gfx3DMatrix::TransformBounds does not handle the case where a quad crosses the w=0 plane - Skia does not handle rendering quads that cross the w=0 plane, in particular SkMatrix::Persp_pts By updating BasicCompositor::DrawQuad and BasicCompositor::Transform to use Matrix4x4 rather than gfx3DMatrix, I was able to replace the calls to gfx3DMatrix::TransformBounds with Matrix4x4::ProjectRectBounds. This corrected the bounding boxes / clip rects; however, the rendering is still incorrect due to Skia assuming that quads will not cross the w=0 plane. There are two solutions to fix this: a - Clip the quad into pieces that do not cross the w=0 plane before passing them into Skia b - Patch SkMatrix::Persp_pts (and possibly other) functions with the equivalent support for w=0 plane clipping that was added to gfx3DMatrix::TransformBounds. c - Implement a specialized software rasterization function in BasicCompositor to handle these cases outside of Skia.

> There are two solutions to fix this: two => three

Also note, the pixman code paths may be affected by the same issue as Skia.

- Updated BasicCompositor::DrawQuad to use the Matrix4x4 class rather than gfx3DMatrix. - BasicCompositor::DrawQuad and BasicCompositor::Transform now transform the vertices prior to handing them to Skia, avoiding the limitations of Skia not handling the crossing of the w=0 plane (polygons both in front and behind the camera). This geometry is passed as a path, which is filled by the layer's texture with a transform applied to the texture fill only. - This is a WIP, with some artifacts remaining in the Bug 1072898 sample case.

- The BasicCompositor now transforms vertices prior to rendering in Skia, using a function that clips against frustum clipping planes in homogenous coordinate space.

- A reftest added to Bug 1035611 has been enabled for non-accelerated layer rendering, to test the same issue in the BasicCompositor.

The attached patches appear to fix the issue; however, the page in the example is broken currently due to Bug 1157984

Pushed to try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=6726b9562e46

The patches for this bug appear to fix the Linux failure case for Bug 1158562.

Try is green, except for unrelated intermittent tests.

Comment on attachment 8598278 [details] [diff] [review] Bug 1072898: Part 1 - Correct rendering of layers that are both in front and behind the w=0 plane Would you be available to review, Matt? This fix is similar to the one you reviewed in Bug 135611.

Comment on attachment 8598279 [details] [diff] [review] Bug 1072898 - Part 2: Enable reftest for non-accelerated layers Would you be available to review, Matt? This fix is similar to the one you reviewed in Bug 135611.

(In reply to :kip (Kearwood Gilbert) from comment #22) > Comment on attachment 8598279 [details] [diff] [review] > Bug 1072898 - Part 2: Enable reftest for non-accelerated layers > > Would you be available to review, Matt? This fix is similar to the one you > reviewed in Bug 135611. I missed a digit in the bug number. Correct bug is Bug 1035611

- The BasicCompositor now transforms vertices prior to rendering in Skia, using a function that clips against frustum clipping planes in homogenous coordinate space. V2 Patch: - Matrix4x4 now transforms the vertices directly with the matrix rather than using ProjectPoint

Comment on attachment 8600100 [details] [diff] [review] Bug 1072898: Part 1 - Correct rendering of layers that are both in front and behind the w=0 plane (V2 Patch) Review of attachment 8600100 [details] [diff] [review]: ----------------------------------------------------------------- ::: gfx/2d/Matrix.h @@ +493,5 @@ > + * The vertex count is returned by ProjectAndClipRect. It is possible to > + * emit fewer that 3 vertices, indicating that aRect will not be visible > + * within aClip. > + */ > + size_t ProjectAndClipRect(const Rect& aRect, const Rect& aClip, Point* aVerts) const; Can we name this TransformAndClipRect? That way we keep the 'Project' terminology to be specific to the reverse transformation process. I'd happy to move to new names if you have better ideas, as long as they're consistent. ::: gfx/layers/basic/BasicCompositor.cpp @@ +373,5 @@ > newTransform = aTransform.As2D(); > } else { > + // Get the bounds post-transform. > + new3DTransform = aTransform; > + Rect bounds = new3DTransform.ProjectRectBounds(aRect, Rect(offset.x, offset.y, buffer->GetSize().width, buffer->GetSize().height)); This should stay as TransformBounds too shouldn't it?

- The BasicCompositor now transforms vertices prior to rendering in Skia, using a function that clips against frustum clipping planes in homogenous coordinate space. V3 Patch: - Updated with feedback in Comment 25

https://hg.mozilla.org/integration/mozilla-inbound/rev/61bd6e2c6e72 https://hg.mozilla.org/integration/mozilla-inbound/rev/230adc57e016

Backed out for reftest failures. https://hg.mozilla.org/integration/mozilla-inbound/rev/ab4eee1c6d7e https://treeherder.mozilla.org/logviewer.html#?job_id=9605261&repo=mozilla-inbound