New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 804492 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Feb 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 3
Type: Bug



Sign in to add a comment

R and B channels are swapped in pixel layout tests output on Android.

Project Member Reported by sergeyu@chromium.org, Jan 22 2018

Issue description

Currently layout_tests pixel results have R and B channels swapped because BlinkTestController::OnImageDump() ignores pixel format in SkBitmap and Android normally uses BGR instead of RGB, which cases R and B channels to be swapped in the test output. This issue is clear when comparing Linux and Android test exceptions, e.g. see the two following images.
 https://codesearch.chromium.org/chromium/src/third_party/WebKit/LayoutTests/platform/linux/editing/pasteboard/paste-line-endings-005-expected.png?q=paste-line-endings-005
 https://codesearch.chromium.org/chromium/src/third_party/WebKit/LayoutTests/platform/android/editing/pasteboard/paste-line-endings-005-expected.png

Handling pixel format correctly should allow removing many expectation files for Android.
 
Cc: pdr@chromium.org foolip@chromium.org robertma@chromium.org

Comment 2 by foolip@chromium.org, Jan 22 2018

If both tests and baselines are inverted, should we expect fixing this to affect any test results? Are there non-Android baselines that would work if not for this issue?
Android layout-tests fallback to Linux expectations.
Only smoke tests are enabled on Android by default. That's the reason we don't have Android-specific expectations for all tests. I expect all non-smoke tests to fail on Android, except those that have black-and-white output. I think fixing this issue will require removing most (if not all) Android-specific expectation files
There's only 96 android-specific PNGs. If the right thing to do is to change the interpretation so that we can reuse the linux results generally, we should just do that and rebaseline the affected files.
sergeyu, would you like to fix the bug in BlinkTestController?

I don't think it'd be a problem to rebaseline all affected tests once the bug is fixed (get rid of Android-specific baselines in this case). In fact, it's great!

It's also pretty simple to rebaseline. Once you have a CL fixing the bug, you can run `WebKit/Tools/Scripts/webkit-patch rebaseline-cl --builder=android_blink_rel` in that branch.

Comment 6 by foolip@chromium.org, Jan 24 2018

Status: Available (was: Untriaged)
Owner: sergeyu@chromium.org
Status: Started (was: Available)
Project Member

Comment 8 by bugdroid1@chromium.org, Jan 31 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/06faf3de3d76f5efc37a9a833ce3df0e2278bd3d

commit 06faf3de3d76f5efc37a9a833ce3df0e2278bd3d
Author: Sergey Ulanov <sergeyu@chromium.org>
Date: Wed Jan 31 19:54:39 2018

Fix layout tests on Android to dump correct pixel results.

Previously layout tests were not handling pixel format correctly,
so R and B channels were swapped as the result.

Bug:  804492 
Change-Id: Id178cc2707e501e5e044c058acaf9bc3fc846158
Reviewed-on: https://chromium-review.googlesource.com/892468
Commit-Queue: Sergey Ulanov <sergeyu@chromium.org>
Reviewed-by: Peter Beverloo <peter@chromium.org>
Cr-Commit-Position: refs/heads/master@{#533367}
[modify] https://crrev.com/06faf3de3d76f5efc37a9a833ce3df0e2278bd3d/content/shell/browser/layout_test/blink_test_controller.cc

Status: Fixed (was: Started)
The issue has been fixed now. Rebaslining should be tracked separately because all pixel tests on Android were already disabled when I opened this bug. 
Cc: jeffcarp@chromium.org msarett@chromium.org qyears...@chromium.org
 Issue 663368  has been merged into this issue.

Sign in to add a comment