1691141 - backplate-bg-image-008.html unexpectedly fails on macOS with non-native theme turned on

Status: RESOLVED FIXED

(Core :: Widget: Cocoa, defect, P2)

Description

[Stephen A Pohl [:spohl]]

Emilio, I'm not sure what's going on here. The test fails with:

REFTEST TEST-UNEXPECTED-FAIL | layout/reftests/high-contrast/backplate-bg-image-008.html == layout/reftests/high-contrast/backplate-bg-image-008-ref.html | image comparison, max difference: 255, number of differing pixels: 20

My understanding is that the maximum permissible pixel difference is 255, and the actual difference is only 20. So shouldn't this test be considered as passing..?

The difference is clearly visible in the reftest analyzer. I just don't know if we should be addressing that pixel difference there, or if there is some kind of error in recognizing the fact that the test actually passed (since only 20px of the permissible 255px differed):

https://hg.mozilla.org/mozilla-central/raw-file/tip/layout/tools/reftest/reftest-analyzer.xhtml#logurl=https://firefox-ci-tc.services.mozilla.com/api/queue/v1/task/P8QOtxWlTfSQ6BxHOdlQ2Q/runs/0/artifacts/public/logs/live_backing.log&only_show_unexpected=1

Comment 1

[Markus Stange [:mstange]]

Multiple fuzzy-if specifiers on the same line are combined in non-intuitive ways. I think the fuzzy-if(!nativeThemePref,0-21,0-6) actually overrides the first fuzziness declaration, so you're only allowed 6 pixels that differ for up to 21.

Comment 2

[Stephen A Pohl [:spohl]]

So is this an exercise in correcting our fuzzy-if specifiers, or should we actually investigate the test itself and why we have this fuzziness in the first place..?

Comment 3

[Stephen A Pohl [:spohl]]

...and I guess we might be running into a similar issue with anonymous-block.html:

REFTEST TEST-UNEXPECTED-FAIL | layout/reftests/text-overflow/anonymous-block.html == layout/reftests/text-overflow/anonymous-block-ref.html | image comparison, max difference: 1, number of differing pixels: 227

reftest.list reads as follows for this test:

fuzzy(0-16,0-454) fails-if(gtkWidget) fuzzy-if(webrender&&winWidget,49-85,454-499) fuzzy-if(webrender&&OSX&&!nativeThemePref,7-7,143-143) skip-if(OSX&&!isDebugBuild&&verify) == anonymous-block.html anonymous-block-ref.html # gtkWidget:bug 1309103, fuzzy: subpixel aa

I'm not sure where the allowed max difference of 1 is coming from here...

Fyi, the full try run with non-native theme enabled is here:

https://treeherder.mozilla.org/jobs?repo=try&revision=860636cce4a9a29832226e2de82fd8512b112937&selectedTaskRun=RqW9vCVvRD-zyn54V_wn_w.0

Comment 4

[Markus Stange [:mstange]]

(In reply to Stephen A Pohl [:spohl] from comment #2)

So is this an exercise in correcting our fuzzy-if specifiers, or should we actually investigate the test itself and why we have this fuzziness in the first place..?

In our case, since the change we're making is expected to change rendering, yes; this is an exercise in finding fuzzy-if specifiers that make the tests green.

In general, it depends:

• If the test had no fuzziness before, a failure usually means that something unintentional happened. It might indicate a bug.
• If the test already had fuzziness, this indicates that there's some existing cause that would cause slightly different rendering between the test and the reference. So, unless the reftest analyzer reveals an unintended change, most of the time the fuzziness values just need to be adjusted.

These tests are made to catch bugs where rendering changes even though the patch was intended to keep the existing behavior. They're made to catch bugs in the rendering engine.

Adjusting fuzziness is a tedious exercise, but the manual step ensures that unintended rendering changes don't go unnoticed.

Comment 5

[Stephen A Pohl [:spohl]]

Thank you for clarifying! This seems like a good time for me to wrap my head around our fuzzy-if specifiers. :-)

Comment 6

[Markus Stange [:mstange]]

(In reply to Stephen A Pohl [:spohl] from comment #3)

...and I guess we might be running into a similar issue with anonymous-block.html:

REFTEST TEST-UNEXPECTED-FAIL | layout/reftests/text-overflow/anonymous-block.html == layout/reftests/text-overflow/anonymous-block-ref.html | image comparison, max difference: 1, number of differing pixels: 227

reftest.list reads as follows for this test:

fuzzy(0-16,0-454) fails-if(gtkWidget) fuzzy-if(webrender&&winWidget,49-85,454-499) fuzzy-if(webrender&&OSX&&!nativeThemePref,7-7,143-143) skip-if(OSX&&!isDebugBuild&&verify) == anonymous-block.html anonymous-block-ref.html # gtkWidget:bug 1309103, fuzzy: subpixel aa

I'm not sure where the allowed max difference of 1 is coming from here...

The log includes this line:

REFTEST INFO | REFTEST fuzzy test (7, 143) <= (1, 227) <= (7, 143)

So somehow it's applying the fuzziness values from this rule: fuzzy-if(webrender&&OSX&&!nativeThemePref,7-7,143-143).
I'm not sure why it's doing that, nativeThemePref should be true. Maybe it's not, and that's the bug?

Comment 7

[Markus Stange [:mstange]]

Oh, oops, I read that upside down. nativeThemePref should indeed be false if the non-native theme is used.

Comment 8

[Stephen A Pohl [:spohl]]

Attachment: Bug 1691141: Ensure that previous fuzziness still applies to backplate-bg-image-008.html test on macOS when the non-native theme is enabled. r=emilio,mstange

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/9729e533e57f
Ensure that previous fuzziness still applies to backplate-bg-image-008.html test on macOS when the non-native theme is enabled. r=mstange

Comment 10

[Atila Butkovits]

https://hg.mozilla.org/mozilla-central/rev/9729e533e57f