The default bug view has changed. See this FAQ.

Scaling for the same (image,size) pair produces non-deterministic results on Android and Windows XP (box-sizing-replaced-001.xht, box-sizing-replaced-003.xht)

RESOLVED FIXED in Firefox 38

Status

()

Core
ImageLib
RESOLVED FIXED
3 years ago
2 years ago

People

(Reporter: philor, Assigned: seth)

Tracking

({intermittent-failure, pp})

Trunk
mozilla39
intermittent-failure, pp
Points:
---
Bug Flags:
in-testsuite -

Firefox Tracking Flags

(firefox37 wontfix, firefox38 fixed, firefox39 fixed, firefox-esr31 wontfix)

Details

Attachments

(1 attachment, 3 obsolete attachments)

(Reporter)

Description

3 years ago
Probably several more have happened during the course of the day, but we... don't have a lot of respect for Android reftest failures.

https://tbpl.mozilla.org/php/getParsedLog.php?id=30234822&tree=Fx-Team
box-sizing-replaced-001.xht | image comparison (==), max difference: 14, number of differing pixels: 271

https://tbpl.mozilla.org/php/getParsedLog.php?id=30244793&tree=Mozilla-Central
box-sizing-replaced-001.xht | image comparison (==), max difference: 27, number of differing pixels: 975

https://tbpl.mozilla.org/php/getParsedLog.php?id=30249778&tree=Mozilla-Central
box-sizing-replaced-001.xht | image comparison (==), max difference: 14, number of differing pixels: 701

https://tbpl.mozilla.org/php/getParsedLog.php?id=30249300&tree=Mozilla-Inbound
box-sizing-replaced-001.xht | image comparison (==), max difference: 14, number of differing pixels: 486
Comment hidden (Treeherder Robot)
(Reporter)

Updated

3 years ago
Summary: Very frequent Android armv6 failures in box-sizing-replaced-001.xht → Very frequent Android failures in box-sizing-replaced-001.xht
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
This test landed just two days ago, feel free to disable it for now.
Priority: -- → P5
Comment hidden (Treeherder Robot)
Lukas, can you take a look please?
Flags: needinfo?(lukasnordin11)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)

Comment 32

3 years ago
Created attachment 829387 [details] [diff] [review]
android.patch

You should try needinfo(dbaron@dbaron.org) he may know more than me on this.
But try this patch to see if that fixes it :)
Flags: needinfo?(lukasnordin11)
Thanks, but box-sizing-replaced-001.xht already contains that change AFAICT:
http://hg.mozilla.org/mozilla-central/diff/1525f72e55ea/layout/reftests/w3c-css/submitted/ui3/box-sizing-replaced-001.xht
It landed 2013-11-05 as part of bug 243412:
http://hg.mozilla.org/mozilla-central/rev/1525f72e55ea
Comment hidden (Treeherder Robot)
Looking at the reftest analyzer results for the R1 orange here:
https://tbpl.mozilla.org/?rev=ea22ac82b970&tree=B2g-Inbound
it appears we're comparing two scaled images that are rather blurry.
I don't think such comparisons can be deterministic enough to serve
as reftests, especially if we plan to submit them to W3C for use
by other UAs.  Wouldn't it be better to use some other kind of
replaced element in this test?
Flags: needinfo?(dbaron)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)

Comment 41

3 years ago
Created attachment 829558 [details] [diff] [review]
android.patch Update

(In reply to Mats Palmgren (:mats) from comment #33)
> Thanks, but box-sizing-replaced-001.xht already contains that change AFAICT:
> http://hg.mozilla.org/mozilla-central/diff/1525f72e55ea/layout/reftests/w3c-
> css/submitted/ui3/box-sizing-replaced-001.xht
> It landed 2013-11-05 as part of bug 243412:
> http://hg.mozilla.org/mozilla-central/rev/1525f72e55ea
My bad, meant to do this!
Attachment #829387 - Attachment is obsolete: true
Comment hidden (Treeherder Robot)
OK, so basically reverting that part, but you're changing #img1 and #img2 which
are the second and third images in this test and that's not the ones that fails.
Go to the link in comment 35, click on the orange "R1", then click on "open
reftest analyzer" in the bottom left corner, then click on the test link,
then click "Circle differences".  It's image 15, 16 and 17 that fails.

Note also that it's not the size of the images that are wrong, it's some
interior pixels in the image that got scaled slightly differently between the
test and the reference.  Using a solid black image might fix it?
(We normally use "image-rendering: -moz-crisp-edges;" to fix image-scaling fuzziness issues like this. That might (?) be undesirable here, though, since that's from a different spec (CSS Image Values and Replaced Content Module Level 4) which is currently a working draft. http://dev.w3.org/csswg/css-images/#the-image-rendering / http://www.w3.org/TR/css4-images/#image-rendering )
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)

Comment 49

3 years ago
(In reply to Mats Palmgren (:mats) from comment #43)
> OK, so basically reverting that part, but you're changing #img1 and #img2
> which
> are the second and third images in this test and that's not the ones that
> fails.
> Go to the link in comment 35, click on the orange "R1", then click on "open
> reftest analyzer" in the bottom left corner, then click on the test link,
> then click "Circle differences".  It's image 15, 16 and 17 that fails.
Neat, didn't know that!

> Note also that it's not the size of the images that are wrong, it's some
> interior pixels in the image that got scaled slightly differently between the
> test and the reference.  Using a solid black image might fix it?
That sounds like an very good idea. I did not write the original test so I don't know if the colored boxes have any function. It should not though, we are testing for width not color.
Comment hidden (Treeherder Robot)
I guess the black dots in the images have the function to show that the image was scaled
rather than cropped so we would loose that feature of the test with a single color.
OTOH, that's not the primary purpose of these tests so I think a single color image
is the way to go, assuming it actually fixes the problem ;-)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
(In reply to Mats Palmgren (:mats) from comment #35)
> Looking at the reftest analyzer results for the R1 orange here:
> https://tbpl.mozilla.org/?rev=ea22ac82b970&tree=B2g-Inbound
> it appears we're comparing two scaled images that are rather blurry.
> I don't think such comparisons can be deterministic enough to serve
> as reftests, especially if we plan to submit them to W3C for use
> by other UAs.  Wouldn't it be better to use some other kind of
> replaced element in this test?

I'd hope that scaling the same image to the same size ought to be deterministic, and I thought that's what this test was doing.  Maybe it's not, though, but if it's not it should be fixable so that that is what it's doing.
Flags: needinfo?(dbaron)
That said, probably best to mark as random-if(Android) while this gets worked out.
Comment hidden (Treeherder Robot)
Created attachment 829730 [details] [diff] [review]
Disable this test on Android
Attachment #829558 - Attachment is obsolete: true
Created attachment 829732 [details] [diff] [review]
Mark this test as random on Android
Attachment #829730 - Attachment is obsolete: true

Updated

3 years ago
Keywords: checkin-needed
Whiteboard: [leave open]
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
https://hg.mozilla.org/integration/mozilla-inbound/rev/94a544d956c6
Keywords: checkin-needed
(In reply to David Baron [:dbaron] (needinfo? me) from comment #58)
> I'd hope that scaling the same image to the same size ought to be
> deterministic, and I thought that's what this test was doing.  Maybe it's
> not, though, but if it's not it should be fixable so that that is what it's
> doing.

And, indeed, it looks to me like the test is just fine, and the underlying problem seems to me to be that image scaling for the same (image,size) pair produces nondeterministic results on Android, which seems to me like a bug.
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)

Updated

3 years ago
No longer blocks: 243412
Component: CSS Parsing and Computation → ImageLib
Keywords: pp
Priority: P5 → --
Summary: Very frequent Android failures in box-sizing-replaced-001.xht → Scaling for the same (image,size) pair produces non-deterministic results on Android
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
https://hg.mozilla.org/mozilla-central/rev/94a544d956c6
(In reply to David Baron [:dbaron] (needinfo? me) from comment #67)
> (In reply to David Baron [:dbaron] (needinfo? me) from comment #58)
> > I'd hope that scaling the same image to the same size ought to be
> > deterministic, and I thought that's what this test was doing.  Maybe it's
> > not, though, but if it's not it should be fixable so that that is what it's
> > doing.
> 
> And, indeed, it looks to me like the test is just fine, and the underlying
> problem seems to me to be that image scaling for the same (image,size) pair
> produces nondeterministic results on Android, which seems to me like a bug.

I haven't looked into this problem, but it sounds like something I have looked into.

We don't scale images specifically, we draw and image using a given transform. If we draw the image to a surface the size of the page we will be using a different transform from the case where we draw an image to a surface the size of just the image (or anything smaller than the whole page). A case that can happen on some backends (depending on how they are implemented) is the image can be drawn with the first paint of the page, so it gets drawn to a surface the size of the whole page. Other times the image isn't ready to display on the first paint of the page, so when it is ready to display we only invalidate a smaller area containing the image, we then draw the image to a surface which is smaller and offset from the page origin (this is the part that depends on the backend), this gives us a different transform, and thus we could be drawing different pixels (off by one or two).

Updated

3 years ago
Duplicate of this bug: 942541

Updated

3 years ago
Summary: Scaling for the same (image,size) pair produces non-deterministic results on Android → Scaling for the same (image,size) pair produces non-deterministic results on Android (box-sizing-replaced-001.xht, box-sizing-replaced-003.xht)
Please mark box-sizing-replaced-003.xht random on Android.

Updated

3 years ago
Duplicate of this bug: 943864

Updated

3 years ago
OS: Android → All
Hardware: ARM → All

Updated

3 years ago
Summary: Scaling for the same (image,size) pair produces non-deterministic results on Android (box-sizing-replaced-001.xht, box-sizing-replaced-003.xht) → Scaling for the same (image,size) pair produces non-deterministic results on Android and Windows XP (box-sizing-replaced-001.xht, box-sizing-replaced-003.xht)
Comment hidden (Treeherder Robot)

Updated

3 years ago
Blocks: 948389
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Whiteboard: [leave open] → [marked as random-if on Android]
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
(Assignee)

Comment 102

2 years ago
A little more fuzz is clearly in order here:

https://hg.mozilla.org/integration/mozilla-inbound/rev/6d051ee01dc8
https://hg.mozilla.org/mozilla-central/rev/6d051ee01dc8
Assignee: nobody → seth
Status: NEW → RESOLVED
Last Resolved: 2 years ago
status-firefox39: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla39
status-firefox37: --- → wontfix
status-firefox38: --- → affected
status-firefox-esr31: --- → wontfix
https://hg.mozilla.org/releases/mozilla-aurora/rev/4e6dbab8eb4e
status-firefox38: affected → fixed
Flags: in-testsuite-
Whiteboard: [marked as random-if on Android]
You need to log in before you can comment on or make changes to this bug.