turn on Skia for canvas on OSX

RESOLVED FIXED in Firefox 40

Status

()

RESOLVED FIXED
5 years ago
3 years ago

People

(Reporter: vlad, Assigned: mstange)

Tracking

(Depends on: 2 bugs, Blocks: 1 bug)

unspecified
mozilla40
x86
Mac OS X
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox40 fixed)

Details

MozReview Requests

()

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(1 attachment, 2 obsolete attachments)

See bug 932615.

Given that we're going to be shipping SkiaGL on B2G, we need to find issues there faster -- let's turn on SkiaGL for Canvas on OSX right now, and deal with the fallout.

(Optimistically assigning to gw280, but could go elsewhere.)
Let's let George finish the koi+ bugs first.  Let's also prioritize all the items, so that we know where this fits in the list.  If it's #1, we'll find somebody to do it.
Assignee: gwright → nobody
Seems reasonable to me.
Also I can take this.

Comment 5

5 years ago
Any idea when this will happen now?
Assignee: nobody → gwright
I thought bug 996611 was tracking content issues - this is just canvas.
Oh, oops. My bad!
No longer depends on: 996611
Forgot to change a unit test in the try run mentioned in comment 7, this one is more relevant https://tbpl.mozilla.org/?tree=Try&rev=6e18f132df45
We skip this test with accelerated Skia: test_2d_gradient_radial_inside1
Looks like all the dependent bugs are fixed now. Anything else keeping us from flipping this pref?
Flags: needinfo?(gwright)
We didn't enter bugs for all the failures: https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=fc75af631b63, there are still problems.
Flags: needinfo?(gwright)

Updated

4 years ago
Blocks: 1101483
Assignee: gwright → nobody
As of https://treeherder.mozilla.org/#/jobs?repo=try&revision=4cfa5824cb47 (SkiaGL):

These are the failures:

REFTEST TEST-UNEXPECTED-FAIL | file:///builds/slave/talos-slave/test/build/tests/reftest/tests/layout/reftests/w3c-css/submitted/variables/variable-supports-06.html | assertion count 1 is more than expected 0 assertions

REFTEST TEST-UNEXPECTED-FAIL | file:///builds/slave/talos-slave/test/build/tests/reftest/tests/layout/reftests/border-radius/curved-stripe-border.html | image comparison (==), max difference: 4, number of differing pixels: 582

REFTEST TEST-UNEXPECTED-PASS | file:///builds/slave/talos-slave/test/build/tests/reftest/tests/layout/reftests/canvas/672646-alpha-radial-gradient.html | image comparison (==)

REFTEST TEST-UNEXPECTED-FAIL | file:///builds/slave/talos-slave/test/build/tests/reftest/tests/layout/reftests/css-blending/background-blending-darken.html | assertion count 1 is more than expected 0 assertions

REFTEST TEST-UNEXPECTED-FAIL | file:///builds/slave/talos-slave/test/build/tests/reftest/tests/layout/reftests/css-blending/background-blending-hard-light.html | image comparison (==), max difference: 1, number of differing pixels: 1600

REFTEST TEST-UNEXPECTED-FAIL | file:///builds/slave/talos-slave/test/build/tests/reftest/tests/layout/reftests/css-gradients/linear-diagonal-9a.html | image comparison (==), max difference: 4, number of differing pixels: 29437

REFTEST TEST-UNEXPECTED-FAIL | file:///builds/slave/talos-slave/test/build/tests/reftest/tests/layout/reftests/css-gradients/radial-1a.html | image comparison (==), max difference: 6, number of differing pixels: 5606

REFTEST TEST-UNEXPECTED-FAIL | file:///builds/slave/talos-slave/test/build/tests/reftest/tests/layout/reftests/css-gradients/radial-1b.html | image comparison (==), max difference: 6, number of differing pixels: 5606

REFTEST TEST-UNEXPECTED-FAIL | file:///builds/slave/talos-slave/test/build/tests/reftest/tests/layout/reftests/css-gradients/radial-1c.html | image comparison (==), max difference: 6, number of differing pixels: 5606

REFTEST TEST-UNEXPECTED-FAIL | file:///builds/slave/talos-slave/test/build/tests/reftest/tests/layout/reftests/css-gradients/radial-2a.html | image comparison (==), max difference: 3, number of differing pixels: 7056

REFTEST TEST-UNEXPECTED-FAIL | file:///builds/slave/talos-slave/test/build/tests/reftest/tests/layout/reftests/css-gradients/radial-2b.html | image comparison (==), max difference: 3, number of differing pixels: 7056

REFTEST TEST-UNEXPECTED-FAIL | file:///builds/slave/talos-slave/test/build/tests/reftest/tests/layout/reftests/css-gradients/radial-2c.html | image comparison (==), max difference: 3, number of differing pixels: 7056

REFTEST TEST-UNEXPECTED-FAIL | file:///builds/slave/talos-slave/test/build/tests/reftest/tests/layout/reftests/css-gradients/radial-2d.html | image comparison (==), max difference: 3, number of differing pixels: 7056 


REFTEST TEST-UNEXPECTED-FAIL | file:///builds/slave/talos-slave/test/build/tests/reftest/tests/layout/reftests/css-gradients/radial-2e.html | image comparison (==), max difference: 3, number of differing pixels: 7056

REFTEST TEST-UNEXPECTED-FAIL | file:///builds/slave/talos-slave/test/build/tests/reftest/tests/layout/reftests/css-gradients/radial-2f.html | image comparison (==), max difference: 3, number of differing pixels: 7056

REFTEST TEST-UNEXPECTED-FAIL | file:///builds/slave/talos-slave/test/build/tests/reftest/tests/layout/reftests/css-gradients/radial-shape-closest-corner-1a.html | image comparison (==), max difference: 4, number of differing pixels: 22301

REFTEST TEST-UNEXPECTED-FAIL | file:///builds/slave/talos-slave/test/build/tests/reftest/tests/layout/reftests/css-gradients/radial-shape-closest-corner-1b.html | image comparison (==), max difference: 4, number of differing pixels: 22565

REFTEST TEST-UNEXPECTED-FAIL | file:///builds/slave/talos-slave/test/build/tests/reftest/tests/layout/reftests/css-gradients/radial-shape-farthest-corner-1b.html | image comparison (==), max difference: 2, number of differing pixels: 41281

REFTEST TEST-UNEXPECTED-FAIL | file:///builds/slave/talos-slave/test/build/tests/reftest/tests/layout/reftests/image-element/gradient-html-07a.html | image comparison (==), max difference: 2, number of differing pixels: 42305

REFTEST TEST-UNEXPECTED-FAIL | file:///builds/slave/talos-slave/test/build/tests/reftest/tests/layout/reftests/svg/as-image/canvas-drawImage-alpha-1.html | image comparison (==), max difference: 1, number of differing pixels: 10000

REFTEST TEST-UNEXPECTED-FAIL | file:///builds/slave/talos-slave/test/build/tests/reftest/tests/layout/reftests/svg/as-image/canvas-drawImage-alpha-2.html | image comparison (==), max difference: 1, number of differing pixels: 676 


REFTEST TEST-UNEXPECTED-FAIL | file:///builds/slave/talos-slave/test/build/tests/reftest/tests/layout/reftests/svg/filters/svg-filter-chains/clip-input.svg | assertion count 1 is more than expected 0 assertions

REFTEST TEST-UNEXPECTED-FAIL | file:///builds/slave/talos-slave/test/build/tests/reftest/tests/layout/reftests/svg/non-scaling-stroke-01.svg | image comparison (==), max difference: 1, number of differing pixels: 122

REFTEST TEST-UNEXPECTED-FAIL | file:///builds/slave/talos-slave/test/build/tests/reftest/tests/layout/reftests/svg/opacity-and-gradient-02.svg | image comparison (==), max difference: 1, number of differing pixels: 99974

REFTEST TEST-UNEXPECTED-FAIL | file:///builds/slave/talos-slave/test/build/tests/reftest/tests/layout/reftests/svg/radialGradient-basic-03.svg | image comparison (==), max difference: 4, number of differing pixels: 15982

REFTEST TEST-UNEXPECTED-FAIL | file:///builds/slave/talos-slave/test/build/tests/reftest/tests/layout/reftests/text-svgglyphs/svg-glyph-objectgradient-zoom.svg | image comparison (==), max difference: 1, number of differing pixels: 7007
Tracking SkiaGL for Shumway canvas perf.
Blocks: 1038057
(Assignee)

Comment 17

3 years ago
New try push with Skia software and updated reftest annotations:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=5c2af84f226f

Wherever there was a fuzzy-if(azureQuartz,*,*) annotation that was needed also with Skia, I replaced azureQuartz with cocoaWidget.

It's possible there are still failures in test_canvas.html; it doesn't run during reftests so it wasn't included in the last push.
(Assignee)

Comment 18

3 years ago
Created attachment 8587772 [details] [diff] [review]
Use Skia Software by default for canvas on OS X.

Looks like I missed updating the annotation on non-scaling-stroke-01.svg in the previous push. This patch fixes that.
Attachment #8587772 - Flags: review?(jmuizelaar)
Blocks: 1150944
I filed bug 1150944 for skiagl. Let's keep this one for the skia landing.
Summary: turn on Skia & SkiaGL for canvas on OSX → turn on Skia for canvas on OSX
(Assignee)

Comment 20

3 years ago
Created attachment 8588730 [details]
MozReview Request: bz://932958/mstange

/r/6663 - Bug 932958 - Use the Skia Software backend by default for canvas on OS X. r=jrmuizel

Pull down this commit:

hg pull -r 503c3c2d430f7a0812b99b9950dd822c3b93d184 https://reviewboard-hg.mozilla.org/gecko/
Attachment #8588730 - Flags: review?(jmuizelaar)
(Assignee)

Updated

3 years ago
Attachment #8587772 - Attachment is obsolete: true
Attachment #8587772 - Flags: review?(jmuizelaar)
Comment on attachment 8588730 [details]
MozReview Request: bz://932958/mstange

https://reviewboard.mozilla.org/r/6661/#review5547

Ship It!
Attachment #8588730 - Flags: review?(jmuizelaar) → review+
Backed out in https://hg.mozilla.org/integration/mozilla-inbound/rev/76df12cb260f because we apparently enjoy setting you up for failure.

You missed a few 10.10 things from your try push that did include 10.8 and 10.10, https://treeherder.mozilla.org/logviewer.html#?job_id=8510877&repo=mozilla-inbound

You also, unsurprisingly, missed realizing that we now spell reftest "reftest[10.6,10.8,10.10],web-platform-tests-reftests[10.10]" because we both run reftests (some of them just copy-pasted) in a separate suite with a different name, and we also do not run that suite on 10.6, so -a or any of its variants won't run it for you on try, only explicitly saying that you want 10.10 tests will.
Oops, silly of me, it was actually web-platform-tests-1 rather than web-platform-tests-reftests where you had some more bustage (https://treeherder.mozilla.org/logviewer.html#?job_id=8515689&repo=mozilla-inbound). Still a 10.10-only suite, but maybe in a way less painful than it being in the other reftest suite with a different name.

Updated

3 years ago
Duplicate of this bug: 1151626
(Assignee)

Updated

3 years ago
Depends on: 1152044
(Assignee)

Comment 27

3 years ago
That push is looking good. The 10.6 failure was because I accidentally included a debugging change to the failing test (which I'm going to revert before landing), and the 10.8 failure is an existing failure from a test that was added a week ago in bug 1146754 and was only fuzzed for 10.10, because we don't run 10.8 tests on inbound / mozilla-central.
https://hg.mozilla.org/mozilla-central/rev/3337571fa164
Status: NEW → RESOLVED
Last Resolved: 3 years ago
status-firefox40: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla40
Blocks: 857895
(Assignee)

Updated

3 years ago
Depends on: 1160789
(Assignee)

Comment 29

3 years ago
Comment on attachment 8588730 [details]
MozReview Request: bz://932958/mstange
Attachment #8588730 - Attachment is obsolete: true
Attachment #8618052 - Flags: review+
(Assignee)

Comment 30

3 years ago
Created attachment 8618052 [details]
MozReview Request: Bug 932958 - Use the Skia Software backend by default for canvas on OS X. r=jrmuizel
(Assignee)

Updated

3 years ago
Depends on: 1209480
You need to log in before you can comment on or make changes to this bug.