635302 - Content flips vertically when scrolled with opacity on two levels

Status: RESOLVED FIXED

(Core :: Graphics, defect)

Description

[Matthew N. [:MattN]]

Attachment: Reduced testcase

When a semi-transparent element contains another semi-transparent element, and the inner element has content that scrolls (ie. overflow:auto), the inner content will flip vertically when scrolled if accelerated layers is on.

See reduced testcase based on usage in the Angel LMS.

Comment 1

[Boris Zbarsky [:bzbarsky]]

Uh... yeah.  This is bad.  This needs to block.

Comment 2

[Bas Schouten (:bas.schouten)]

This probably has something to do with obscure y axis logic in OpenGL when using framebuffer. This testcase I believe but lead to two container layers both using intermediate surfaces.

Comment 3

[Mehdi Mulani [:mmm] (I don't check this)]

Did some regression testing on a Mac, found:

2010 11 15 030805 edf41ff32f08 good
2010 11 16 030753 a42e9b001bc8 bad

http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=edf41ff32f08&tochange=a42e9b001bc8

Comment 4

[Matt Woodrow (:mattwoodrow)]

Attachment: Better flipping logic

Reasoning:

When double buffering is enabled, we have a Y-flip set on the viewport transform *at all times*. Since our layout co-ordinate system is already the opposite to GL's, this actually results in drawing happening the right way up.

When blitting between framebuffers, or framebuffers and the window (there should be no difference) we want to have no flip. For this reason we need to pass true to BindAndDrawQuad at all times in the blit pass to add a second flip (on top of the viewport transform) to cancel each other out. Passing flip=true to disable flipping is crazy, we need to rewrite this.

For the single buffered case (not used in any supported configuration), all drawing is down upside-down (without the underlying Y flip), so we need to pass false to BindAndDrawQuad when blitting to stop inversions happening. The only flip in this situation is when blitting the final data to the window, which is handled by LayerManager.

Comment 5

[Matt Woodrow (:mattwoodrow)]

Filed bug 635383 since I'm sick of these bugs that keep appearing.

Comment 6

[Joe Drew (not getting mail)]

Comment on attachment 513588 [details] [diff] [review]
Better flipping logic

aaaaaaaaaaaahhhhhhhhhhhhhhhhhhh


test please

Comment 7

[Matt Woodrow (:mattwoodrow)]

Attachment: Tests

First attempt at a test.

This catches the actual logic issue, but still fails (on mac at least) due to bizarre padding differences between <canvas> and <div>.

Can someone else please take a look at this and see if they can figure it out, making guesses at css code isn't very productive for me.

Comment 8

[Joe Drew (not getting mail)]

We need to add 

canvas {
    display: block;
}

For some reason, canvases are inline by default, and therefore they get placed on the line along with everything else. We then put, below the canvas, room for the descender.

Comment 9

[Mike Shaver (:shaver emeritus)]

I can help land this tonight, if I'm not incapacitated by my birthday party.

Comment 10

[Matt Woodrow (:mattwoodrow)]

Attachment: Tests v2

Tests passes on mac now. \o/

Comment 11

[Joe Drew (not getting mail)]

Comment on attachment 513657 [details] [diff] [review]
Tests v2


>diff --git a/layout/reftests/bugs/635302-1.html b/layout/reftests/bugs/635302-1.html

>\ No newline at end of file

May as well fix this before landing.

woohoo! thanks, Matt!

Comment 12

[Mike Shaver (:shaver emeritus)]

http://hg.mozilla.org/mozilla-central/rev/6bad6cd2dded both patches, with newline added.

Added a plausible-to-me commit message.  Easier when people provide their own in the patch, though! :-)

Comment 13

[Mike Shaver (:shaver emeritus)]

Backed out because it was failing its own test: http://hg.mozilla.org/mozilla-central/rev/61c6c43d9630

Comment 14

[Matt Woodrow (:mattwoodrow)]

What platforms did the test fail on?

Comment 15

[Mike Shaver (:shaver emeritus)]

OS X 64.

Comment 16

[Matt Woodrow (:mattwoodrow)]

Attachment: Combined patch

Combined both patches.

Changed the logic to check IsDrawingFlipped() since our reftest mode is happier that way.

Changed some of the opacity/colour values in the test to stop differences in blending causing single bit colour differences.

Comment 17

[(no longer active)]

Should this be landed again?

Comment 18

[Matt Woodrow (:mattwoodrow)]

http://hg.mozilla.org/mozilla-central/rev/77e1fe783407