573933 - More test changes for bug 564991

Status: RESOLVED FIXED

(Core :: Layout, defect)

Description

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Various parts of bug 564991 change things that tests implicitly depend on. Some of these tests need to be fixed.

Comment 1

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Attachment: Part 1: Fix marquee tests to not depend on subpixel AA

Marquees scroll themselves during setup, even if they have a scrollamount of 0. This makes them an "active scrolling" element, so we create a layer for their contents for a brief time initially, and so the text in the marquee is drawn without subpixel AA.

Comment 2

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Attachment: Part 2: Don't render troublesome native themes

This is a tricky one. I'm seeing a few pixels in the antialiased rounded borders of GTK themes being rendered inconsistently (consistently). Karl and I theorize that what is happening is that in dynamic situations we're painting a theme background with a clip rect we can't handle directly with GTK, so we execute fallback, and have to draw to temporary surfaces and recover alpha values, and the alpha recovery process is losing some precision when it calculates alpha values.

Karl is working on a plan to fix this by avoiding alpha recovery, but in the meantime I'm turning off moz-appearance on the troublesome tests. Hopefully we can back this out when Karl's solution arrives.

Comment 3

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Attachment: Part 3: Fix button-state test to render a margin around the button so we don't get into any weird sampling issues. Also, use a canvas that's just as big as necessary

To be honest, I don't understand this one too well. There was a very small error in a few pixels at the edge of this test on Mac, and it seemed to be because of pixel-sampling. Drawing a few pixels around the button fixed the problem.

Comment 4

[Mats Palmgren (inactive)]

Attachment: more test fixes

FWIW, the following tests fails with rev e318bbaed556 from
http://hg.mozilla.org/users/rocallahan_mozilla.com/layers-patches
layout/reftests/bugs/280708-1a.html
layout/reftests/bugs/280708-1b.html
layout/reftests/bugs/309914-1.xul
layout/reftests/bugs/331809-1.html
layout/reftests/bugs/376375-1.html
layout/reftests/bugs/446100-1f.html
layout/reftests/bugs/513153-1a.html
layout/reftests/bugs/513153-1b.html
layout/reftests/bugs/513153-2a.html
layout/reftests/bugs/513153-2b.html

(Firefox debug build, Ubuntu 10.04 x86-64)
This is a wallpaper patch that makes them succeed.

Comment 5

[David Baron :dbaron:]

Comment on attachment 453327 [details] [diff] [review]
Part 1: Fix marquee tests to not depend on subpixel AA

r=dbaron, although I'm not crazy about having to do this

Comment 6

[David Baron :dbaron:]

Comment on attachment 453345 [details] [diff] [review]
Part 2: Don't render troublesome native themes

This wasn't fixed by that native theme overflow patch I reviewed recently, where you swapped the ordering of some 'overflow' handling with native theme handling?

Comment 7

[David Baron :dbaron:]

Comment on attachment 453548 [details] [diff] [review]
Part 3: Fix button-state test to render a margin around the button so we don't get into any weird sampling issues. Also, use a canvas that's just as big as necessary

This seems ok, but I also wonder if it would have been fixed by that other patch.

Comment 8

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Part 2 definitely wasn't fixed by the clipping patch. Part 3, I'm not sure, but the new test is better anyway.

Comment 9

[David Baron :dbaron:]

Comment on attachment 453345 [details] [diff] [review]
Part 2: Don't render troublesome native themes

I'm ok with this landing, but please make sure there's a bug on file specifically about backing it out (which should depend on the bug on Karl's work).

Comment 10

[Karl Tomlinson (:karlt)]

(In reply to comment #9)
> (which should depend on the bug on Karl's work).

Bug 576143.

Comment 11

[Robert O'Callahan (:roc) (email my personal email if necessary)]

http://hg.mozilla.org/mozilla-central/rev/4d1dc2ea47fd
http://hg.mozilla.org/mozilla-central/rev/acb4d17b70cd
http://hg.mozilla.org/mozilla-central/rev/152633f27b60

Note that I folded Mats' changes into part 2.

Comment 12

[Karl Tomlinson (:karlt)]

Bug 576143 will allow us to back out all of attachment 453345 [details] [diff] [review],
and most of attachment 454226 [details] [diff] [review] (assuming I can trust the try server).

The parts of attachment 454226 [details] [diff] [review] that bug 576143 will not fix are:

layout/reftests/bugs/376375-1 has scrollbars on a div with opacity:0.99999.

  I'm guessing the scrollbars are on a layer with an alpha channel.

  The reftest is meant to test an alpha-recovery path, so the alpha channel is
  probably intentional, so the only ways I can imagine modifying the test are
  to use different widgets or cover the bad bits, as done here.
  https://bugzilla.mozilla.org/attachment.cgi?id=304858&action=diff

In layout/reftests/bugs/331809-1.html,

  the native widget is actually bleeding out of the iframe.  Covering the
  portion of the widget in the iframe as done here causes the
  native widget not to get painted, obscuring the problem.

  I filed 580499 on this.

Comment 13

[Karl Tomlinson (:karlt)]

Back-out described in comment 12:
http://hg.mozilla.org/mozilla-central/rev/02d92df7381d