Closed Bug 888446 Opened 7 years ago Closed 7 years ago

Some Skia canvas tests fail on Android

Categories

(Core :: Canvas: 2D, defect)

ARM
Android
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla25

People

(Reporter: snorp, Assigned: snorp)

Details

Attachments

(3 files)

I have a couple failures here with software Skia on Android. We are going to use software for a while on non-NVIDIA, so we need this to pass to land SkiaGL. The failure is:

497 ERROR TEST-UNEXPECTED-FAIL | /tests/content/canvas/test/test_canvas.html | pixel 99,0 of c106 is 0,252,0,255; expected 0,255,0,255 +/- 2
498 ERROR TEST-UNEXPECTED-FAIL | /tests/content/canvas/test/test_canvas.html | pixel 0,49 of c106 is 0,252,0,255; expected 0,255,0,255 +/- 2
501 ERROR TEST-UNEXPECTED-FAIL | /tests/content/canvas/test/test_canvas.html | pixel 80,20 of c106 is 0,252,0,255; expected 0,255,0,255 +/- 2
502 ERROR TEST-UNEXPECTED-FAIL | /tests/content/canvas/test/test_canvas.html | pixel 20,30 of c106 is 0,252,0,255; expected 0,255,0,255 +/- 2

Not off by much obviously so I think increasing the fuzz by one should be fine.
Attachment #769108 - Flags: review?(bjacob) → review+
Comment on attachment 777779 [details] [diff] [review]
Added a lot more fuzz

Review of attachment 777779 [details] [diff] [review]:
-----------------------------------------------------------------

Mostly seems ok.

::: layout/reftests/bugs/reftest.list
@@ +1489,5 @@
>  == 552334-1.html 552334-1-ref.html
>  # 553571 depends on MS Indic shaping behavior and Win7 font support;
>  # not expected to be reliable on XP or non-Windows platforms
>  random-if(!winWidget) random-if(/^Windows\x20NT\x205/.test(http.oscpu)) != 553571-1.html 553571-1-notref.html # expect dotted circle in test, not in ref
> +fuzzy-if(!contentSameGfxBackendAsCanvas,2,91) random-if(d2d) skip-if(azureSkiaGL) == 555388-1.html 555388-1-ref.html

We need a bug to track this.

::: layout/reftests/canvas/reftest.list
@@ +36,5 @@
>  == text-space-replace-test.html text-space-replace-ref.html
>  
>  == text-no-frame-test.html text-no-frame-ref.html
>  == text-no-frame-2-test.html text-not-in-doc-ref.html
> +fuzzy-if(azureSkiaGL,10,400) == text-not-in-doc-test.html text-not-in-doc-ref.html

This really shouldn't be fuzzy...? Both the test and the ref are using SkiaGL/canvas to draw the text

::: layout/tools/reftest/reftest.js
@@ +152,5 @@
>  var gVerbose = false;
>  
>  // Only dump the sandbox once, because it doesn't depend on the
>  // manifest URL (yet!).
> +var gDumpedConditionSandbox = true;

Accidentally committed this hunk?
Attachment #777779 - Flags: review?(gwright) → review+
(In reply to George Wright (:gw280) from comment #3)
> Comment on attachment 777779 [details] [diff] [review]
> Added a lot more fuzz
> 
> Review of attachment 777779 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Mostly seems ok.
> 
> ::: layout/reftests/bugs/reftest.list
> @@ +1489,5 @@
> >  == 552334-1.html 552334-1-ref.html
> >  # 553571 depends on MS Indic shaping behavior and Win7 font support;
> >  # not expected to be reliable on XP or non-Windows platforms
> >  random-if(!winWidget) random-if(/^Windows\x20NT\x205/.test(http.oscpu)) != 553571-1.html 553571-1-notref.html # expect dotted circle in test, not in ref
> > +fuzzy-if(!contentSameGfxBackendAsCanvas,2,91) random-if(d2d) skip-if(azureSkiaGL) == 555388-1.html 555388-1-ref.html
> 
> We need a bug to track this.

Agreed. Bug 895729

> 
> ::: layout/reftests/canvas/reftest.list
> @@ +36,5 @@
> >  == text-space-replace-test.html text-space-replace-ref.html
> >  
> >  == text-no-frame-test.html text-no-frame-ref.html
> >  == text-no-frame-2-test.html text-not-in-doc-ref.html
> > +fuzzy-if(azureSkiaGL,10,400) == text-not-in-doc-test.html text-not-in-doc-ref.html
> 
> This really shouldn't be fuzzy...? Both the test and the ref are using
> SkiaGL/canvas to draw the text

Yeah, not sure what's going on. Bug 895731.

> 
> ::: layout/tools/reftest/reftest.js
> @@ +152,5 @@
> >  var gVerbose = false;
> >  
> >  // Only dump the sandbox once, because it doesn't depend on the
> >  // manifest URL (yet!).
> > +var gDumpedConditionSandbox = true;
> 
> Accidentally committed this hunk?

Yeah, fixed.
https://hg.mozilla.org/mozilla-central/rev/cc82635606eb
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla25
We need to do basically disable a couple more tests for SkiaGL. I filed bugs for the two issues here in bugs 896525 and 896527.
Comment on attachment 779230 [details] [diff] [review]
Fuzz/fail more tests for SkiaGL

Review of attachment 779230 [details] [diff] [review]:
-----------------------------------------------------------------

Snorp tells me we already have a bug filed for the latter case (bug 895729) so I'm happy with this as it is.
Attachment #779230 - Flags: review?(gwright) → review+
You need to log in before you can comment on or make changes to this bug.