Bug 93673 – NRWT: Make it possible to run layout tests on Retina MBP

• Component: Tools / Tests
• Status: RESOLVED FIXED

[Ryosuke Niwa]

We need a separate baseline on machines with deviceScaleFactor > 1 such as Retina MacBook Pro.

[Ryosuke Niwa]

A big question is how we're going to support both WK2 and high resolution as the same time.

Say we're on Retina MBP with Lion and running tests under WK1. Fallback path will probably look like:

mac-lion-highresolution > mac-lion > ... > mac

But what if we were running tests under WK2?

mac-lion-highresolution-wk2 > mac-lion-highresolution > mac-lion > ... > mac?
or mac-lion-wk2-highresolution > mac-lion-wk2 > mac-lion > ... > mac?
or something crazy like mac-lion-wk2-highresolution > mac-highresolution > mac-lion-wk2 > mac-lion > ... > mac?

[Ryosuke Niwa]

Created attachment 157603 [details]
work in progress

[Nico Weber]

See also bug 93647 (no patch)

[Ryosuke Niwa]

What is the canonical name for these sorts of displays? "high resolution" and "high DPI" are not good descriptions because they just mean that screen size is big when measured in device pixels. It's the scaling factor that's unique.

Maybe "scaled"?

[Dirk Pranke]

(In reply to comment #4)
> What is the canonical name for these sorts of displays? "high resolution" and "high DPI" are not good descriptions because they just mean that screen size is big when measured in device pixels. It's the scaling factor that's unique.
> 
> Maybe "scaled"?

This may be beside the point, since I'm not sure that a different implementation of the display would have the same characteristics or matter ... maybe we should just call it "retina" ?

[Nico Weber]

On Windows, we might end up with 1.4 and 1.8 scale factors for metro as opposed to 2x, so just "retina" might not be a good idea. Maybe "scaled-200"?

[Ryosuke Niwa]

As I understand it, Retina is Apple's trademark and I'd rather not take a risk of using that term.

Also, I really hope we don't have to support multiple scaling factors on our tests.

[Terry Anderson]

(In reply to comment #3)
> See also bug 93647 (no patch)

Can you please clarify the relationship between the two bugs? AFAICT, I still need to fix bug 93647 to ensure that bug 90192 can land (i.e., to check the correctness of high dpi pixel tests in chromium for both the software and hardware paths).

[Beth Dakin]

(In reply to comment #8)
> (In reply to comment #3)
> > See also bug 93647 (no patch)
> 
> Can you please clarify the relationship between the two bugs? AFAICT, I still need to fix bug 93647 to ensure that bug 90192 can land (i.e., to check the correctness of high dpi pixel tests in chromium for both the software and hardware paths).


If my understanding is correct, bug 93647 is about making the tests in fast/hidpi work. Those tests use WTR/DRT to simulate HiDPI mode on ANY device, so the results are expected to always be in 2x whether they are run on a 1x device, a 2x device, or a magical 16x device! 

This bug is about running ALL of the tests on a retina mac, which is a 2x device. There are many tests in other directories that have results checked in that expect a 1x device, so they fail on a 2x machine. See these bugs to get an idea of the kind of tests that require a platform/retina or platform/2x directory:

https://bugs.webkit.org/show_bug.cgi?id=89709
https://bugs.webkit.org/show_bug.cgi?id=89711
https://bugs.webkit.org/show_bug.cgi?id=89713
https://bugs.webkit.org/show_bug.cgi?id=89715

[Ryosuke Niwa]

Not only that but a lot of pixel tests baselines for Chromium Mac don't match :(

[Dirk Pranke]

(In reply to comment #7)
> As I understand it, Retina is Apple's trademark and I'd rather not take a risk of using that term.

Good point.

> Also, I really hope we don't have to support multiple scaling factors on our tests.

Yes, that would be nice, but so far I'm not optimistic :)

Do we know what is failling when deviceScaleFactor > 1? Is it anything we could potentially work around or fuzz for in the test? I assume it's mostly differences due to interpolating things differently, and maybe we could do a fuzzy image compare for that?

[Philip Rogers]

(In reply to comment #11)
> (In reply to comment #7)
> > As I understand it, Retina is Apple's trademark and I'd rather not take a risk of using that term.
> 
> Good point.
> 
> > Also, I really hope we don't have to support multiple scaling factors on our tests.
> 
> Yes, that would be nice, but so far I'm not optimistic :)
> 
> Do we know what is failling when deviceScaleFactor > 1? Is it anything we could potentially work around or fuzz for in the test? I assume it's mostly differences due to interpolating things differently, and maybe we could do a fuzzy image compare for that?

Primarily font aliasing but there are also scrollbar differences (where scrollbars appear now, but they didn't before). I don't think we could get away with fuzzy image compare, sadly.

[Beth Dakin]

(In reply to comment #11)
> (In reply to comment #7)
> > As I understand it, Retina is Apple's trademark and I'd rather not take a risk of using that term.
> 
> Good point.
> 
> > Also, I really hope we don't have to support multiple scaling factors on our tests.
> 
> Yes, that would be nice, but so far I'm not optimistic :)
> 
> Do we know what is failling when deviceScaleFactor > 1? Is it anything we could potentially work around or fuzz for in the test? I assume it's mostly differences due to interpolating things differently, and maybe we could do a fuzzy image compare for that?

If you look at the links I pasted into comment 9, you will see a number of things that are failing. I think there is a legitimate need for a platform directory here because not all of the failures represent bugs. For example, there is no way to test media queries that query device-pixel-ratio without having different expectations for 1x and 2x. There are also a lot of legitimate differences with <canvas> when you are on a  1x versus a 2x device, and the only way to test those is again allowing for different expectations.

[Tim Horton]

Created attachment 194513 [details]
patch to make DRT+WKTR windows 1x

Attaching a hacky patch that implements an alternative solution: running the tests at 1x always. I realize we probably want to test 2x in the future, but if we want a quick fix that makes existing test results match on Retina hardware, we could do this...

[Tim Horton]

Comment on attachment 194513 [details]
patch to make DRT+WKTR windows 1x

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

> Tools/WebKitTestRunner/mac/PlatformWebViewMac.mm:131
> -    NSRect windowRect = NSOffsetRect(rect, -10000, [(NSScreen *)[[NSScreen screens] objectAtIndex:0] frame].size.height - rect.size.height + 10000);
> +    NSRect windowRect = NSOffsetRect(rect, 0, [(NSScreen *)[[NSScreen screens] objectAtIndex:0] frame].size.height - rect.size.height);

Pretend this part isn't here, it was just for testing.

[Ryosuke Niwa]

(In reply to comment #14)
> Created an attachment (id=194513) [details]
> patch to make DRT+WKTR windows 1x
> 
> Attaching a hacky patch that implements an alternative solution: running the tests at 1x always. I realize we probably want to test 2x in the future, but if we want a quick fix that makes existing test results match on Retina hardware, we could do this...

Nice! Do all tests pass with this change? Or do some tests still fail? If some of them do fail, then we probably want to do this and what I proposed earlier so that we may check in -expected.txt as needed.

[Ryosuke Niwa]

Nice! It seems like all the tests pass.

[Levi Weintraub]

Awesome!!!

[Ryosuke Niwa]

Comment on attachment 194513 [details]
patch to make DRT+WKTR windows 1x

Tim, could you add a change log to this? r=me with that and reverting the change you mentioned were only for testing.

[Beth Dakin]

Hopefully this patch does not break testRunner.setBackingScaleFactor(). That's a mode that I added a while ago to test 2X from a 1X machine. The tests that use it are in fast/hidpi.

[Ryosuke Niwa]

(In reply to comment #20)
> Hopefully this patch does not break testRunner.setBackingScaleFactor(). That's a mode that I added a while ago to test 2X from a 1X machine. The tests that use it are in fast/hidpi.

I’ve ran it locally and they seem to pass still (I seem to have some color profile isssues thouh might be Lion vs. ML difference)

[Beth Dakin]

(In reply to comment #21)
> (In reply to comment #20)
> > Hopefully this patch does not break testRunner.setBackingScaleFactor(). That's a mode that I added a while ago to test 2X from a 1X machine. The tests that use it are in fast/hidpi.
> 
> I’ve ran it locally and they seem to pass still (I seem to have some color profile isssues thouh might be Lion vs. ML difference)
 
Cool! (Well, the colorspace issue is not cool, but I'm glad that generally the mode works.)

[Tim Horton]

(In reply to comment #19)
> (From update of attachment 194513 [details])
> Tim, could you add a change log to this? r=me with that and reverting the change you mentioned were only for testing.

Will do, shortly. Glad that it helps :D

[Tim Horton]

http://trac.webkit.org/changeset/146650

Tests will now run at 1x on all devices (for the Mac port).