Last Comment Bug 707563 - Intermediate surfaces change rendering of 3d transforms
: Intermediate surfaces change rendering of 3d transforms
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: Graphics (show other bugs)
: unspecified
: x86 Mac OS X
: -- normal (vote)
: mozilla11
Assigned To: Matt Woodrow (:mattwoodrow)
:
: Milan Sreckovic [:milan]
Mentors:
Depends on: 711809
Blocks: 701656
  Show dependency treegraph
 
Reported: 2011-12-04 13:51 PST by Matt Woodrow (:mattwoodrow)
Modified: 2011-12-19 05:37 PST (History)
6 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Reduced test case (539 bytes, text/html)
2011-12-04 13:51 PST, Matt Woodrow (:mattwoodrow)
no flags Details
Layer tree dump showing the incorrect rendering with the testcase (301.38 KB, text/html)
2011-12-04 13:53 PST, Matt Woodrow (:mattwoodrow)
no flags Details
Convert vector back into normal coordinate space before applying offset (294.56 KB, patch)
2011-12-05 17:42 PST, Matt Woodrow (:mattwoodrow)
bas: review+
Details | Diff | Splinter Review
Fix v2 (218.16 KB, patch)
2011-12-14 14:14 PST, Matt Woodrow (:mattwoodrow)
no flags Details | Diff | Splinter Review
Fix v3 (286.70 KB, patch)
2011-12-14 20:51 PST, Matt Woodrow (:mattwoodrow)
bas: review+
Details | Diff | Splinter Review

Description Matt Woodrow (:mattwoodrow) 2011-12-04 13:51:55 PST
Created attachment 578937 [details]
Reduced test case

When a 3d transformed layer is being drawn to a parent that requires an intermediate surface, the object is drawn incorrectly.

This is showing up in http://ikilote.net/Programmation/CSS/Test/transform-style.htm because the transformed layers have a ContainerLayer parent that requires an intermediate surface (due to opacity).

This only occurs in the accelerated layer managers, BasicLayers is unaffacted.

Reduced test case attached - requires a debugger. Set mUseIntermediateSurface = true in DefaultComputeEffectiveTransforms for the parent of the transformed layer, or use the debugging patch from bug 700240 (which sets this on every layer).
Comment 1 Matt Woodrow (:mattwoodrow) 2011-12-04 13:53:30 PST
Created attachment 578938 [details]
Layer tree dump showing the incorrect rendering with the testcase
Comment 2 Matt Woodrow (:mattwoodrow) 2011-12-04 14:03:55 PST
When drawn to an intermediate surface, the actual shape is incorrect (as opposed to offset incorrectly).

The only differences in the draw call are the viewport dimensions (and associated projection matrix) and the offset. Looking at the vertex shader, I can't see how either of these values would affect the shape of the transformed object.
Comment 3 Matt Woodrow (:mattwoodrow) 2011-12-05 17:42:26 PST
Created attachment 579166 [details] [diff] [review]
Convert vector back into normal coordinate space before applying offset

The vector was still in homogenous coordinate space when we applied the offset, giving incorrect results.

An alternative here would be to pass the offset as a translation matrix and multiply by that instead.
Comment 4 Bas Schouten (:bas.schouten) 2011-12-08 13:39:54 PST
Comment on attachment 579166 [details] [diff] [review]
Convert vector back into normal coordinate space before applying offset

Review of attachment 579166 [details] [diff] [review]:
-----------------------------------------------------------------

I prefer this approach I think, it's only a vector divide, which is probably cheaper than the 4 dp4s if we passed in a translation matrix. Possibly we could pass along the translation in the mLayerTransform, but that obscures things a little, and we should have cycles to spare on the vertex shaders! Good catch!
Comment 5 Matt Woodrow (:mattwoodrow) 2011-12-08 19:22:45 PST
https://hg.mozilla.org/integration/mozilla-inbound/rev/2f9ce0282804
Comment 6 Ed Morley [:emorley] 2011-12-10 20:46:24 PST
https://hg.mozilla.org/mozilla-central/rev/2f9ce0282804
Comment 7 Matt Woodrow (:mattwoodrow) 2011-12-14 14:14:52 PST
Created attachment 581783 [details] [diff] [review]
Fix v2

Dividing by W doesn't always give us correct results, so I think this is the only choice.

Untested on d3d9 because my laptop is Optimus and won't run them.
Comment 8 Matt Woodrow (:mattwoodrow) 2011-12-14 20:51:46 PST
Created attachment 581866 [details] [diff] [review]
Fix v3

We determined on IRC that this was caused by the perspective-correct interpolation used in the shaders requires the w component to be intact.

This patch doesn't add a new matrix parameter, but instead just remultiplies by w after applying the offset.
Comment 9 Marek Stępień [:marcoos, inactive] 2011-12-17 02:51:10 PST
Is this bug related to this: http://dabblet.com/gist/1489886 rendering like this: http://yfrog.com/gzczrpp ?
Comment 10 Matt Woodrow (:mattwoodrow) 2011-12-18 13:13:22 PST
https://hg.mozilla.org/integration/mozilla-inbound/rev/0da67b6beb18

Marek: Yes, that's the problem that this patch fixes.
Comment 11 Marco Bonardo [::mak] 2011-12-19 05:37:54 PST
https://hg.mozilla.org/mozilla-central/rev/0da67b6beb18

Note You need to log in before you can comment on or make changes to this bug.