Bug 100263 - [chromium] Enable fixed position compositing on Android DRT
Summary: [chromium] Enable fixed position compositing on Android DRT
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Sami Kyöstilä
URL:
Keywords:
Depends on: 98647
Blocks: 100139
  Show dependency treegraph
 
Reported: 2012-10-24 09:54 PDT by Sami Kyöstilä
Modified: 2012-11-06 07:32 PST (History)
7 users (show)

See Also:


Attachments
Patch (9.16 KB, patch)
2012-10-24 10:11 PDT, Sami Kyöstilä
no flags Details | Formatted Diff | Diff
Patch (11.95 KB, patch)
2012-10-25 04:26 PDT, Sami Kyöstilä
no flags Details | Formatted Diff | Diff
Patch (9.16 KB, patch)
2012-10-25 08:18 PDT, Sami Kyöstilä
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Sami Kyöstilä 2012-10-24 09:54:51 PDT
[chromium] Enable fixed position compositing on Android DRT
Comment 1 Sami Kyöstilä 2012-10-24 10:11:07 PDT
Created attachment 170419 [details]
Patch
Comment 2 Sami Kyöstilä 2012-10-24 10:14:47 PDT
Having some trouble seeing the effects of this locally, so let's run it through the bots. I did check with debug logs that the appropriate flags are getting set.
Comment 3 WebKit Review Bot 2012-10-24 12:04:18 PDT
Comment on attachment 170419 [details]
Patch

Attachment 170419 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/14563296

New failing tests:
compositing/geometry/fixed-position-transform-composited-page-scale-down.html
Comment 4 Adam Barth 2012-10-25 02:32:54 PDT
This is likely a patch for jamesr to review.
Comment 5 Sami Kyöstilä 2012-10-25 02:50:36 PDT
Looks like we don't run Android layout tests on the bots yet. Running locally, I see 3 types of failures:

- Red/blue color channels are swapped.
- Repaint tests don't work with the compositor.
- Some compositor tiles have have different low-order bits, most likely because of inaccuracies in linear interpolation. I think this is expected because GL doesn't guarantee bit-exactness.

I'll try to fix the red/blue issue first.
Comment 6 Sami Kyöstilä 2012-10-25 04:26:03 PDT
Created attachment 170611 [details]
Patch

Fix red/blue color channel issue.

I also checked the tile texel filtering problem on three different devices and got three different results, so I think we'll have to live with it. Same goes for repaint tests, because all of them need to be rewritten to use the new mechanism from bug 97801.
Comment 7 Peter Beverloo 2012-10-25 05:37:15 PDT
Comment on attachment 170611 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=170611&action=review

Cool, thanks Sami! Do we know how many tests are failing after the color adjustments?

> Tools/DumpRenderTree/chromium/TestShell.cpp:685
> +    // in software rendering mode but BGRA if accelerated compositing is active.

Isn't Alexandre working on software rendering (albeit through the compositor)?
Comment 8 WebKit Review Bot 2012-10-25 06:06:21 PDT
Comment on attachment 170611 [details]
Patch

Attachment 170611 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/14566587

New failing tests:
compositing/geometry/fixed-position-transform-composited-page-scale-down.html
Comment 9 Sami Kyöstilä 2012-10-25 06:13:46 PDT
(In reply to comment #7)
> (From update of attachment 170611 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=170611&action=review
> 
> Cool, thanks Sami! Do we know how many tests are failing after the color adjustments?

I'm still running the tests locally, but from the failures so far I'd estimate about 7100 :)

> > Tools/DumpRenderTree/chromium/TestShell.cpp:685
> > +    // in software rendering mode but BGRA if accelerated compositing is active.
> 
> Isn't Alexandre working on software rendering (albeit through the compositor)?

Yes he is, but at least initially that's not going to be the mode used by Chrome on Android. And as you said, it's still going through the compositor, so we'd need some new mechanism to detect that mode.

Thinking about this a little more, the root issue here is that the compositor readback gives back pixels with the wrong component order. Let me take another pass to figure out why that is happening. Also, I'll see what's up with that failing test on cr-linux.
Comment 10 WebKit Review Bot 2012-10-25 06:25:00 PDT
Comment on attachment 170611 [details]
Patch

Attachment 170611 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/14544604

New failing tests:
compositing/geometry/fixed-position-transform-composited-page-scale-down.html
Comment 11 Sami Kyöstilä 2012-10-25 08:18:28 PDT
Created attachment 170653 [details]
Patch

Fixing R/B ordering issue separately in bug 98647. compositing/geometry/fixed-position-transform-composited-page-scale-down.html fails with and without this patch.
Comment 12 WebKit Review Bot 2012-10-25 11:32:25 PDT
Comment on attachment 170653 [details]
Patch

Attachment 170653 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/14566692

New failing tests:
compositing/geometry/fixed-position-transform-composited-page-scale-down.html
Comment 13 Sami Kyöstilä 2012-10-29 03:26:50 PDT
Anyone care to review?
Comment 14 James Robinson 2012-10-29 09:56:17 PDT
Comment on attachment 170653 [details]
Patch

Sure
Comment 15 Peter Beverloo 2012-10-29 10:06:01 PDT
Comment on attachment 170653 [details]
Patch

The failing test should be unrelated, the CQ should verify that. Setting CQ+ on request.
Comment 16 WebKit Review Bot 2012-10-29 11:04:10 PDT
Comment on attachment 170653 [details]
Patch

Rejecting attachment 170653 [details] from commit-queue.

New failing tests:
compositing/geometry/fixed-position-transform-composited-page-scale-down.html
Full output: http://queues.webkit.org/results/14623401
Comment 17 Peter Beverloo 2012-11-06 06:25:56 PST
Comment on attachment 170653 [details]
Patch

Trying again.
Comment 18 WebKit Review Bot 2012-11-06 07:32:45 PST
Comment on attachment 170653 [details]
Patch

Clearing flags on attachment: 170653

Committed r133607: <http://trac.webkit.org/changeset/133607>
Comment 19 WebKit Review Bot 2012-11-06 07:32:50 PST
All reviewed patches have been landed.  Closing bug.