Note: There are a few cases of duplicates in user autocompletion which are being worked on.

Intermediate surfaces change rendering of 3d transforms

RESOLVED FIXED in mozilla11

Status

()

Core
Graphics
RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: mattwoodrow, Assigned: mattwoodrow)

Tracking

unspecified
mozilla11
x86
Mac OS X
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(4 attachments, 1 obsolete attachment)

(Assignee)

Description

6 years ago
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).
(Assignee)

Comment 1

6 years ago
Created attachment 578938 [details]
Layer tree dump showing the incorrect rendering with the testcase
(Assignee)

Comment 2

6 years ago
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.
(Assignee)

Comment 3

6 years ago
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.
Attachment #579166 - Flags: review?(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!
Attachment #579166 - Flags: review?(bas.schouten) → review+
(Assignee)

Comment 5

6 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/2f9ce0282804
Assignee: nobody → matt.woodrow

Comment 6

6 years ago
https://hg.mozilla.org/mozilla-central/rev/2f9ce0282804
Status: NEW → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla11
(Assignee)

Comment 7

6 years ago
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.
Attachment #581783 - Flags: review?(bas.schouten)
(Assignee)

Updated

6 years ago
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
(Assignee)

Comment 8

6 years ago
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.
Attachment #581866 - Flags: review?(bas.schouten)
(Assignee)

Updated

6 years ago
Attachment #581783 - Attachment is obsolete: true
Attachment #581783 - Flags: review?(bas.schouten)
Attachment #581866 - Flags: review?(bas.schouten) → review+
Is this bug related to this: http://dabblet.com/gist/1489886 rendering like this: http://yfrog.com/gzczrpp ?

Updated

6 years ago
Depends on: 711809
(Assignee)

Comment 10

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

Marek: Yes, that's the problem that this patch fixes.
https://hg.mozilla.org/mozilla-central/rev/0da67b6beb18
Status: REOPENED → RESOLVED
Last Resolved: 6 years ago6 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.