Last Comment Bug 735178 - CSS3 animation at http://www.cssplay.co.uk/menu/cssplay-3d-box-slide.html blinks
: CSS3 animation at http://www.cssplay.co.uk/menu/cssplay-3d-box-slide.html blinks
Status: RESOLVED FIXED
: regression
Product: Core
Classification: Components
Component: Layout: View Rendering (show other bugs)
: 11 Branch
: x86_64 All
: -- normal (vote)
: mozilla15
Assigned To: Matt Woodrow (:mattwoodrow) (PTO until 27 June)
:
Mentors:
Depends on: 996738
Blocks: 701656
  Show dependency treegraph
 
Reported: 2012-03-13 02:31 PDT by Daniel Desira
Modified: 2014-04-15 10:58 PDT (History)
6 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
reduced html (non animation) (3.29 KB, application/x-zip)
2012-03-26 10:10 PDT, Alice0775 White
no flags Details
Add error tolernace values for sorting (835 bytes, patch)
2012-03-27 16:03 PDT, Matt Woodrow (:mattwoodrow) (PTO until 27 June)
roc: review+
Details | Diff | Review
Add coloring of sorted 3d layers to help identification (6.80 KB, patch)
2012-03-27 16:03 PDT, Matt Woodrow (:mattwoodrow) (PTO until 27 June)
roc: review+
Details | Diff | Review
Add coloring of sorted 3d layers to help identification v2 (6.47 KB, patch)
2012-03-27 16:41 PDT, Matt Woodrow (:mattwoodrow) (PTO until 27 June)
roc: review+
Details | Diff | Review

Description Daniel Desira 2012-03-13 02:31:53 PDT
User Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:13.0) Gecko/20120312 Firefox/13.0a1
Build ID: 20120312031136

Steps to reproduce:

I ran http://www.cssplay.co.uk/menu/cssplay-3d-box-slide.html on Nightly.


Actual results:

I noticed this example blinks.


Expected results:

It seems like it should not blink as I also tested in Chrome Canary. I haven't looked at the code yet though.
Comment 1 Alice0775 White 2012-03-13 04:35:57 PDT
I can reproduce in
http://hg.mozilla.org/releases/mozilla-beta/rev/0df2e93ff663
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:11.0) Gecko/20100101 Firefox/11.0 ID:20120310173008
http://hg.mozilla.org/releases/mozilla-aurora/rev/8f1eb954ff37
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:12.0a2) Gecko/20120312 Firefox/12.0a2 ID:20120312042012
http://hg.mozilla.org/mozilla-central/rev/5ec9524de1af
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:13.0) Gecko/20120312 Firefox/13.0a1 ID:20120312031136
http://hg.mozilla.org/releases/mozilla-beta/rev/0df2e93ff663
Mozilla/5.0 (X11; Linux i686; rv:11.0) Gecko/20100101 Firefox/11.0 ID:20120310173008
http://hg.mozilla.org/releases/mozilla-aurora/rev/8f1eb954ff37
Mozilla/5.0 (X11; Linux i686; rv:12.0a2) Gecko/20120312 Firefox/12.0a2 ID:20120312042012
http://hg.mozilla.org/mozilla-central/rev/1ca7a94573f2
Mozilla/5.0 (X11; Linux i686; rv:13.0) Gecko/20120313 Firefox/13.0a1 ID:20120313031112

Regression window(m-c),
Not flickers:
http://hg.mozilla.org/mozilla-central/rev/30161b298513
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:11.0a1) Gecko/20111117 Firefox/11.0a1 ID:20111117030939
Flickers:
http://hg.mozilla.org/mozilla-central/rev/e7d5dd9efeca
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:11.0a1) Gecko/20111117 Firefox/11.0a1 ID:20111117015450
Pushlog:
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=30161b298513&tochange=e7d5dd9efeca

Regression window(m-i),
Not flickers:
http://hg.mozilla.org/integration/mozilla-inbound/rev/aa05d607c95e
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:11.0a1) Gecko/20111116 Firefox/11.0a1 ID:20111116173248
Flickers:
http://hg.mozilla.org/integration/mozilla-inbound/rev/b201f2434265
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:11.0a1) Gecko/20111116 Firefox/11.0a1 ID:20111116194849
Pushlog:
http://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=aa05d607c95e&tochange=b201f2434265

First bad changest:
17c9363d801a	Matt Woodrow — Bug 701656 - Include the preserve-3d parent content in preserve-3d sorting. r=roc
Comment 2 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2012-03-19 00:37:51 PDT
Fixed by bug 730166?
Comment 3 Alice0775 White 2012-03-19 01:06:05 PDT
(In reply to Robert O'Callahan (:roc) (Mozilla Corporation) from comment #2)
> Fixed by bug 730166?
No, bug 730166 did not fix this issue.
I can reproduce in latest Nightly.
http://hg.mozilla.org/mozilla-central/rev/e94edfdb1f5b
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:14.0) Gecko/20120318 Firefox/14.0a1 ID:20120318031036
Comment 4 Matt Woodrow (:mattwoodrow) (PTO until 27 June) 2012-03-19 17:58:59 PDT
Interestingly, this regression makes our behaviour the same as chrome.

This looks like a bug (or more likely a limitation) of our sorting code, which is only showing up now since previously we (incorrectly) didn't include the grid background in the sorting list.

If anyone can make a reduced test case (without animations), I can take a closer look at this, I haven't been able to reproduce the problem without the animations.

This could likely be a dupe of bug 689498.
Comment 5 Alice0775 White 2012-03-26 10:10:47 PDT
Created attachment 609362 [details]
reduced html (non animation)
Comment 6 Alice0775 White 2012-03-26 10:48:08 PDT
When page zoom-in/out, cubes appears and disappears
Comment 7 Matt Woodrow (:mattwoodrow) (PTO until 27 June) 2012-03-27 16:03:02 PDT
Created attachment 609932 [details] [diff] [review]
Add error tolernace values for sorting

(In reply to Alice0775 White from comment #5)
> Created attachment 609362 [details]
> reduced html (non animation)

Amazing. Thank you!

When sorting 3d layers, we determine a collection of 2d points (points contained within the other line, and line intersections - when both layers are projected to a 2d plane), and then find which point has the greatest z difference.

We determine an ordering between 2 layers based on this point.

In the case where the biggest difference is tiny (less than 0.1 pixels), then we should assume that it was caused by rounding differences and treat the planes as being parallel (using the original ordering).

This fixes this site by getting the bottom edges of the cubes onto the correct side of the grid, and then stops cycles being formed which fixes the rest of the test case.

This still isn't a brilliant solution, having smarter resolution of cycles (current we just break the cycles arbitrarily) would help, and having layer splitting would be the best (and most involved) fix.
Comment 8 Matt Woodrow (:mattwoodrow) (PTO until 27 June) 2012-03-27 16:03:44 PDT
Created attachment 609933 [details] [diff] [review]
Add coloring of sorted 3d layers to help identification
Comment 9 Matt Woodrow (:mattwoodrow) (PTO until 27 June) 2012-03-27 16:07:26 PDT
Comment on attachment 609933 [details] [diff] [review]
Add coloring of sorted 3d layers to help identification

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

::: gfx/layers/LayerSorter.cpp
@@ +174,5 @@
> +#define MAGENTA     5
> +#define CYAN        6
> +#define WHITE       7
> +
> +#define USE_XTERM_COLORING

Will remove this line, since it probably won't work on a lot of machines.
Comment 10 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2012-03-27 16:18:02 PDT
Comment on attachment 609933 [details] [diff] [review]
Add coloring of sorted 3d layers to help identification

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

::: gfx/layers/LayerSorter.cpp
@@ +213,5 @@
> +
> +static void print_layer(FILE* aFile, Layer* aLayer)
> +{
> +  print_layer_internal(aFile, aLayer, aLayer->GetDebugColorIndex());
> +}

This is kind of cool but it will mess with people who use terminal with black backgrounds. I don't know how to fix that.

::: gfx/layers/Layers.h
@@ +946,5 @@
>    PRUint32 mContentFlags;
>    bool mUseClipRect;
>    bool mUseTileSourceRect;
>    bool mIsFixedPosition;
> +  PRUint32 mDebugColorIndex;

DebugOnly<>

::: gfx/layers/basic/BasicLayers.cpp
@@ +2018,5 @@
> +            case 4: color = gfxRGBA(0.0, 0.0, 1.0, 1.0); break;
> +            case 5: color = gfxRGBA(1.0, 0.0, 1.0, 1.0); break;
> +            case 6: color = gfxRGBA(0.0, 1.0, 1.0, 1.0); break;
> +            case 7: color = gfxRGBA(1.0, 1.0, 1.0, 1.0); break;
> +          }

how about
  color = gfxRGBA((aLayer & 1) ? 1.0 : 0.0,
                  (aLayer & 2) ? 1.0 : 0.0,
                  (aLayer & 4) ? 1.0 : 0.0,
                  1.0);
Comment 11 Matt Woodrow (:mattwoodrow) (PTO until 27 June) 2012-03-27 16:36:20 PDT
(In reply to Robert O'Callahan (:roc) (Mozilla Corporation) from comment #10)
> This is kind of cool but it will mess with people who use terminal with
> black backgrounds. I don't know how to fix that.
> 

I'll remove #define USE_XTERM_COLORING, so that it's not enabled by default. I doubt it'll work well for people other than me really.
Comment 12 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2012-03-27 16:39:36 PDT
Comment on attachment 609933 [details] [diff] [review]
Add coloring of sorted 3d layers to help identification

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

OK, r+ with that taken out and the other stuff fixed
Comment 13 Matt Woodrow (:mattwoodrow) (PTO until 27 June) 2012-03-27 16:41:43 PDT
Created attachment 609949 [details] [diff] [review]
Add coloring of sorted 3d layers to help identification v2
Comment 14 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2012-03-27 17:47:24 PDT
Comment on attachment 609949 [details] [diff] [review]
Add coloring of sorted 3d layers to help identification v2

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

::: gfx/layers/LayerSorter.cpp
@@ +166,5 @@
>  #ifdef DEBUG
>  static bool gDumpLayerSortList = getenv("MOZ_DUMP_LAYER_SORT_LIST") != 0;
>  
> +#define BLACK       0
> +#define RED     1

Fix indent

@@ +179,5 @@
> +#ifdef USE_XTERM_COLORING
> +
> +#define RESET       0
> +#define BRIGHT      1
> +#define DIM     2

Fix indent
Comment 15 Alice0775 White 2012-04-17 07:23:47 PDT
2weeks Ping
Comment 18 mgalli 2012-11-11 05:41:28 PST
I see this regression back i Mac OS X mountain lion with Gecko 19.0a1 (2012-11-09)

Can someone test and check perhaps reopen — I do not have right to reopen this bug.
Comment 19 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2012-11-12 11:48:32 PST
Can you file a new bug for that?
Comment 20 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2012-11-12 11:49:27 PST
Oh, Alice filed bug 810685

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