1195435 - image-scrolling-zoom-1.html reftest failure on Linux R-e10s with APZ enabled

Status: RESOLVED FIXED

(Core :: Panning and Zooming, defect)

Description

[Kartikaya Gupta (email:[e-mail])]

With APZ enabled, the test image-scrolling-zoom-1.html fails on Linux R-e10s. :tn looked into this and found that it was related to subpixel scrolling and that adjusting the size of the frame to avoid subpixel issues would fix it.

Comment 1

[Kartikaya Gupta (email:[e-mail])]

tn, didn't you have a WIP to fix this? If you don't have cycles to finish it off please post the patch and I can find somebody else to clean it up.

Comment 2

[Timothy Nikkel (:tnikkel)]

Attachment: integer pixel height for scrollable

This is the patch I used to give the scrollable a integer pixel height to determine that it was likely a subpixel scrolling alignment that was causing the failure. However I didn't land it because we should be expecting a repaint back at a pixel aligned scroll position after the final scroll back to top takes place which should fix the issue even if we get an intermediate subpixel scroll position alignment. So I think the proper fix would then be to add another wait for apz-flush in the reftest harness after the reftest-wait has been be removed.

Comment 3

[Kartikaya Gupta (email:[e-mail])]

Hm, so the apz-flush that exists now in the reftest harness already happens after the reftest-wait has been removed. And looking at the test, since it's changing the scroll position by setting scrollTop, the scroll position change and repaint (if any) should be driven by the main thread, not by APZ, so I'm not really sure that an extra apz flush would even do anything here.

Comment 4

[Timothy Nikkel (:tnikkel)]

Okay, then I can't explain what is causing this test to fail with apz enabled. It seems like it should be passing then.

Comment 5

[Kartikaya Gupta (email:[e-mail])]

Okay, thanks. I'll try to get to the bottom of it.

Comment 6

[Kartikaya Gupta (email:[e-mail])]

Ok, after a lot of false leads I think I figured out what's going on here. I ran the test like so on my Linux machine:

with APZ:
MOZ_DUMP_PAINT=1 MOZ_REFTEST_VERBOSE=1 xvfb-run ./mach reftest --e10s --setpref gfx.xrender.enabled=false --setpref layout.display-list.dump=true --setpref layers.async-pan-zoom.enabled=true layout/reftests/invalidation/image-scrolling-zoom-1.html

without APZ:
MOZ_DUMP_PAINT=1 MOZ_REFTEST_VERBOSE=1 xvfb-run ./mach reftest --e10s --setpref gfx.xrender.enabled=false --setpref layout.display-list.dump=true layout/reftests/invalidation/image-scrolling-zoom-1.html

and saved the outputs. I then looked at the data:image/png dumps in the output, which show the textures that were painted. In the "with APZ" case, because there is a displayport on the entire scrollable container div, the entire content gets painted initially. This shows up like so:

data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAxwAAABDCAYAAAAWNwO3AAACpklEQVR4nO3cMUojABCG0TmU9noAoyaNIQQxwUI9hOgFxNqjhFTWXsNrjFdYlh1mHd4Hr5/+h4mUJEmSpKJis9nkdrvNh4eHfH5+BgAA+GciIhIAAKDE4XDIz8/PfHt7y8zMp6enPBwO+f7+nh8fH/n9/Z2vr695PB7z6+ureZCRJEmS9JuKxWKRy+Uyr66ucr1e58XFRd7d3eVut8uzs7Pc7/d5cnKSy+Uyb25u2icZAADg94j2iQUAAJire2KRJEmSNDdfqgAAgDLRPrEAAABz+VIlSZIkqSpfqgAAgDLRPrEAAABzdU8skiRJkubmSxUAAFAm2icWAABgLl+qJEmSJFXlSxUAAFAm2icWAABgru6JRZIkSdLcfKkCAADKRPvEAgAAzOVLlSRJkqSqfKkCAADKRPvEAgAAzNU9sUiSJEmamy9VAABAmWifWAAAgLl8qZIkSZJUlS9VAABAmWifWAAAgLm6JxZJkiRJc/OlCgAAKBPtEwsAADCXL1WSJEmSqorLy8tcrVZ5fX2dm80mF4tF7na73O/3eX5+nvf393l6epqr1SrX63W+vLwAAAD8kWifWAAAgLm6JxZJkiRJc4vtdpu3t7f5+PjYPrcAAACzRPvEAgAAzOVLlSRJkqSqfKkCAADKRPvEAgAAzNU9sUiSJEmamy9VAABAmWifWAAAgLl8qZIkSZJUlS9VAABAmWifWAAAgLm6JxZJkiRJc/OlCgAAKBPtEwsAADCXL1WSJEmSqvKlCgAAKBPtEwsAADBX98QiSZIkaW6+VAEAAGWifWIBAADm8qVKkiRJUlW+VAEAAGWifWIBAADm6p5YJEmSJM3NlyoAAKBMtE8sAADAXL5USZIkSarKlyoAAKBMtE8sAADAXH89jXQfDgAA/Pd+AK9/CEx6r6Y9AAAAAElFTkSuQmCC

If you look at this with a color meter, you'll notice that of the 22 "black" lines, the first 11 have a 127,127,127 gray and the last 11 have a 129,129,129 gray. (The gray comes from cairo interpolating a 10px tall image down to 3px).

If you look at the equivalent "without APZ" texture, you see that only the visible content gets painted initially, because there is no displayport. This shows up like so:

data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAxwAAAAeCAYAAACmJxieAAABlklEQVR4nO3czanqUBiF4V2UzrUA/5KJIQQx4QzUIkQbEMeWIo4c24ZtfKeFO7hhczbPC08VC1YKSZIkSRqp1LZtdF0Xh8MhzuczAADAf5NSSgEAADCK5/MZ7/c7brdbREScTqd4Pp9xv9/j8XjE9/uN6/Uar9crPp9P5kFGkiRJ0l8qLZfLqKoq1ut1NE0Ti8Ui9vt99H0fs9kshmGIyWQSVVXFdrvNPskAAAB/R8o+sQAAAOXKPbFIkiRJKjcvVQAAwGhS9okFAAAol5cqSZIkSWPlpQoAABhNyj6xAAAA5co9sUiSJEkqNy9VAADAaFL2iQUAACiXlypJkiRJY5VWq1XUdR2bzSbato3lchl938cwDDGfz+Pn5yem02nUdR1N08TlcgEAAPgnKfvEAgAAlCv3xCJJkiSp3FLXdbHb7eJ4PGafWwAAgLKk7BMLAABQLi9VkiRJksbKSxUAADCalH1iAQAAypV7YpEkSZJUbl6qAACA0aTsEwsAAFAuL1WSJEmSxspLFQAAMJqUfWIBAACK9QsT8isjUdmYjQAAAABJRU5ErkJggg==

Again if you look at this with a color meter, you'll notice that of the 10 "black" lines, the first 5 have a 127,127,127 gray and the last 5 have a 129,129,129 gray. My conclusion from this is that when this background image is painted into an area as a tiled texture, cairo (arbitrarily?) chooses to interpolate the top half and bottom half differently. The top half gets a slightly lighter gray than the bottom half.

Now in the "with APZ" case, the reference page only gets painted once, and in that paint the scrollable container has a displayport and is painted fully. Therefore the texture ends up with 11 lines of the lighter gray and 11 lines of the darker gray, and since the container is scrolled to the top, only the top 10 lines are actually visible in the screenshot, all of which are the lighter gray.

In the test page, the texture is similarly painted. However, the test page then scrolls the container from the bottom to the top; this triggers the after-paint listener in the reftest harness, which then decides to repaint the visible area. This time, the displayport does NOT cover the entire scrollable div; it only covers the top half because of the way the displayport margins were initialized. (That is, because the displayport margins were initialized while the container was scrolled to the bottom, the top margin is large and the bottom margin is almost zero.) When the container is scrolled back up to the top, the top part of the scrollable div (11 lines worth) is repainted, and so the top 6 get painted with the light gray and bottom 5 are painted with the dark gray. In the final snapshot the top 10 lines are visible, the bottom 4 of which have the dark gray. This then causes the reftest failure because on the reference case all the visible lines have the light gray.

With APZ disabled the displayport is always empty and only the visible rect ever gets painted, so in both the reference and test pages, the top 5 lines are light gray and the bottom 5 lines are the darker gray.

So in conclusion: this failure is because of a seemingly-arbitrary behaviour in cairo to interpolate the same image to two different gray values depending on where in the paint area it is. I would like to fuzz this test so that any differences in the two grays are basically ignored.

Comment 7

[Kartikaya Gupta (email:[e-mail])]

I checked to see if the subpixel-invalidation at [1] was getting hit, and it only seemed to be getting hit when the page was first loaded, not when the scroll position was changed.

[1] http://mxr.mozilla.org/mozilla-central/source/layout/base/FrameLayerBuilder.cpp?rev=065f00e9b2fd#2301

Comment 8

[Kartikaya Gupta (email:[e-mail])]

Attachment: Fuzz the test when APZ is enabled

Seth, since you added this test, do you have any objections to this patch? See comment 6 for an explanation of what's happening.

Comment 9

[Kartikaya Gupta (email:[e-mail])]

Attachment: Fuzz the test when APZ is enabled

Actually on try it looks like this test fails with 1,3955 rather than the 2,3184 that I'm seeing locally so I adjusted the fuzz numbers to cover both. The symptoms look exactly the same otherwise.

Comment 10

[Seth Fowler [:seth] [:s2h]]

Comment on attachment 8661260 [details] [diff] [review]
Fuzz the test when APZ is enabled

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

Looks OK to me, yeah. The invalidation bug this is a regression test for would produce much bigger differences than this.

Comment 11

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/a2bbfcdd5bf0

Comment 12

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/a2bbfcdd5bf0