Closed Bug 935850 Opened 11 years ago Closed 9 years ago

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)

Categories

(Core :: Graphics: ImageLib, defect)

Product:
Core
Component:
Graphics: ImageLib
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla39
Tracking Flags:
Tracking Status
firefox37 --- wontfix
firefox38 --- fixed
firefox39 --- fixed
firefox-esr31 --- wontfix

People

(Reporter: philor, Assigned: seth)

References

Details

(Keywords: intermittent-failure, platform-parity)

Attachments

(1 file, 3 obsolete files)

Mark this test as random on Android
1.24 KB, patch
Details | Diff | Splinter Review 
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
Summary: Very frequent Android armv6 failures in box-sizing-replaced-001.xht → Very frequent Android failures in box-sizing-replaced-001.xht
This test landed just two days ago, feel free to disable it for now.
Priority: -- → P5
Lukas, can you take a look please?
Flags: needinfo?(lukasnordin11)
Attached patch android.patch (obsolete) — — Splinter Review 
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)
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)
Attached patch android.patch Update (obsolete) — — Splinter Review 
(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
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 )
(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.
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 ;-)
(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.
Attached patch Disable this test on Android (obsolete) — — Splinter Review 

        Attachment #829558 -
        Attachment is obsolete: true

    
    

    
  


  

        Attachment #829730 -
        Attachment is obsolete: true
Keywords: checkin-needed
Whiteboard: [leave open]

    
    

    
      
  

    
    

    
      
  

    
    

    
      
  

    
    

    
  
(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.

    
    

    
      
  

    
    

    
      
  

    
    

    
      
  
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

    
    

    
      
  

    
    

    
      
  

    
    

    
  
(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).
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.
OS: Android → All
Hardware: ARM → All
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)

    
    

    
      
  
Blocks: 948389

    
    

    
      
  

    
    

    
      
  

    
  
Whiteboard: [leave open] → [marked as random-if on Android]

    
    

    
      
  

    
    

    
      
  

    
    

    
      
  

    
    

    
      
  

    
    

    
      
  

    
    

    
      
  

    
    

    
      
  

    
    

    
      
  

    
    

    
      
  

    
    

    
      
  

    
    

    
      
  

    
    

    
      
  

    
    

    
      
  

    
    

    
      
  

    
    

    
      
  

    
    

    
      
  

    
    

    
      
  

    
    

    
      
  

    
    

    
      
  

    
    

    
      
  

    
    

    
      
  

    
    

    
  
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
Closed: 9 years ago

          status-firefox39:
          --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla39

    
    

    
  
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.
        






  

    
  







  

    

      
Attachment

      

      
      
      
      
    

    

      

        

          

            
General

            

              Creator: 



            

            
Created: 

            
Updated: 

            
Size: