888446 - Some Skia canvas tests fail on Android

Status: RESOLVED FIXED

(Core :: Graphics: Canvas2D, defect)

Description

[James Willcox (:snorp) ([e-mail]) (he/him)]

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.

Comment 1

[James Willcox (:snorp) ([e-mail]) (he/him)]

Attachment: Add some fuzz to make Skia canvas pass on Android

Comment 2

[James Willcox (:snorp) ([e-mail]) (he/him)]

Attachment: Added a lot more fuzz

Comment 3

[George Wright (:gw280) (needinfo me!)]

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?

Comment 4

[James Willcox (:snorp) ([e-mail]) (he/him)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/cc82635606eb

Comment 5

[James Willcox (:snorp) ([e-mail]) (he/him)]

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

Comment 6

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/cc82635606eb

Comment 7

[James Willcox (:snorp) ([e-mail]) (he/him)]

Attachment: Fuzz/fail more tests for SkiaGL

Comment 8

[James Willcox (:snorp) ([e-mail]) (he/him)]

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 9

[George Wright (:gw280) (needinfo me!)]

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.

Comment 10

[James Willcox (:snorp) ([e-mail]) (he/him)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/fd715c78c7fd

Comment 11

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/fd715c78c7fd