707563 - Intermediate surfaces change rendering of 3d transforms

Status: RESOLVED FIXED

(Core :: Graphics, defect)

Description

[Matt Woodrow (:mattwoodrow)]

Attachment: 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)]

Attachment: Layer tree dump showing the incorrect rendering with the testcase

Comment 2

[Matt Woodrow (:mattwoodrow)]

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)]

Attachment: 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)]

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)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/2f9ce0282804

Comment 6

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/2f9ce0282804

Comment 7

[Matt Woodrow (:mattwoodrow)]

Attachment: 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)]

Attachment: 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]]

Is this bug related to this: http://dabblet.com/gist/1489886 rendering like this: http://yfrog.com/gzczrpp ?

Comment 10

[Matt Woodrow (:mattwoodrow)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/0da67b6beb18

Marek: Yes, that's the problem that this patch fixes.

Comment 11

[Marco Bonardo [:mak]]

https://hg.mozilla.org/mozilla-central/rev/0da67b6beb18