1468801 - Disable SkiaGL

Status: VERIFIED FIXED

(Core :: Graphics: Canvas2D, defect)

Description

[Lee Salzman [:lsalzman]]

SkiaGL is only used to accelerate Canvas2D, and only on Android and macOS by default. It exposes us to a large amount of extra code and churn from Skia upstream.

If we just relied on software Skia instead, we would avoid this problem and simplify Skia maintenance.

Software Skia performance is competitive and on some benchmarks (i.e. CanvasMark) our performance is actually better in software than with SkiaGL.

Comment 1

[Lee Salzman [:lsalzman]]

Attachment: deprecate SkiaGL for Canvas2D

Pref Skia off as a preliminary step before we remove any code.

Comment 2

[Sotaro Ikeda [:sotaro]]

I worries that it hugely decrease fps of some website like testdrive-archive.azurewebsites.net/performance/fishbowl See Bug 1411481.

Comment 3

[Kelsey Gilbert [:jgilbert]]

We might be interested on the perf impact of real websites, but we aren't interested in benchmarks which really should be written in WebGL.

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/2719ceb0166e
deprecate SkiaGL for Canvas2D. r=snorp

Comment 5

[Noemi Erli[:noemi_erli]]

https://hg.mozilla.org/mozilla-central/rev/2719ceb0166e

Comment 6

[(inactive) Jim Chen [:jchen] [:darchons]]

Can we uplift this to Beta? We're seeing bug 1472356 in Focus (which uses the Beta branch of GeckoView) that is fixed by this bug .

Comment 7

[Chris Peterson [:cpeterson]]

To avoid the uplift risk for macOS, we could uplift with an Android #ifdef.

Comment 8

[Lee Salzman [:lsalzman]]

(In reply to Jim Chen [:jchen] [:darchons] from comment #6)
> Can we uplift this to Beta? We're seeing bug 1472356 in Focus (which uses
> the Beta branch of GeckoView) that is fixed by this bug .

That puts us in a bind, because if we use this as a "fix" for SkiaGL bugs, then that means we're committed to not backing this out, regardless of any potential performance impacts it has. If we did for some reason backout the SkiaGL deprecation, then suddenly all the bugs that were "fixed" by getting rid of SkiaGL come raging back. 

But given that we've been living with this for 3 weeks so far in nightly with things having been quiet, and there is sufficient time left in beta, it could make sense to uplift.

Comment 9

[(inactive) Jim Chen [:jchen] [:darchons]]

Yeah. I guess the other choice is to actually fix the SkiaGL bug on Beta, which I'm not sure we have the resources to do at the moment. I think uplifting this bug is the way to go.

Comment 10

[Lee Salzman [:lsalzman]]

Comment on attachment 8985427 [details] [diff] [review]
deprecate SkiaGL for Canvas2D

Approval Request Comment
[Feature/Bug causing the regression]: Pre-existing condition.
[User impact if declined]: Buggy OpenGL drivers on Android preventing Google Maps from working.
[Is this code covered by automated tests?]: yes
[Has the fix been verified in Nightly?]: yes
[Needs manual test from QE? If yes, steps to reproduce]: no 
[List of other uplifts needed for the feature/fix]:
[Is the change risky?]: not very
[Why is the change risky/not risky?]: Since this disables OpenGL-accelerated canvas, we will no longer have to deal with OpenGL driver bugs. Instead, we just use the same reliable software renderer we use on Windows and Linux which, while possibly being somewhat slower, at least should always work.
[String changes made/needed]: none

Comment 11

[Liz Henry (:lizzard) (relman/hg->git project)]

Comment on attachment 8985427 [details] [diff] [review]
deprecate SkiaGL for Canvas2D

Let's give this a try on beta. From discussion with Lee, this was intended to land in 62 nightly. If we see issues on MacOS in 62 or 63, we can flip the pref back for Mac and leave it in place for mobile (as cpeterson suggested).

Comment 12

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/releases/mozilla-beta/rev/3c6db05ccd3d

Comment 13

[Bogdan Maris, Desktop Test Engineering]

We can do a spotcheck here on macOS using some Canvas demos to make sure that this did not caused any issues.

Comment 14

[ovidiu boca[:Ovidiu]]

I verified this issue on Mac OS X 10.12 with FF Nightly 63.0a1(2018-07-19) and FF beta 62.0b10 using some test pages that can be found here: http://www.kevs3d.co.uk/dev/, https://html5demos.com/canvas/, google maps. I didn't spot any issues during this verification, based on that I will mark this as verified fixed.

Comment 15

[Clemens Eisserer]

Is this really what I am thinking - effectivly turing canvas rendering back to software again?
Where are the times when browser vendors were proud of providing hardware accelerated canvas implementations?

Comment 16

[Darkspirit]

(In reply to Clemens Eisserer from comment #15)
At first I had a similar feeling as you, but it means: Less active non-Rust code, less maintenance cost, less attack surface.
And, as comment 0 says, it isn't that bad. Let's stare on this message: https://mozilla.logbot.info/gfx/20180724#c15062845 ;)

Comment 17

[Shmerl]

Did anyone benchmark Vulkan backend for Skia? See https://skia.org/user/special/vulkan

I opened a feature request to enable it (bug 1502740).

Comment 18

[Clemens Eisserer]

to be honest, I actually don't get this change.

Blit-heavy canvas workloads suffer a lot by the decision to perform software-only rendering (justr look at the fishbowl results representative for so many canvas-based games).

There were times when browser-vendors (including mozilla) were proud to have hardware accelerated canvas - now it is disbled because of a few broken android drivers.

Comment 19

[Lee Salzman [:lsalzman]]

(In reply to Clemens Eisserer from comment #18)

to be honest, I actually don't get this change.

Blit-heavy canvas workloads suffer a lot by the decision to perform software-only rendering (justr look at the fishbowl results representative for so many canvas-based games).

There were times when browser-vendors (including mozilla) were proud to have hardware accelerated canvas - now it is disbled because of a few broken android drivers.

For blit-heavy workloads, you will get better performance, availability, flexibility, and control with WebGL. There are a number of popular JS libraries to help work with WebGL and ease the transition. This is the direction that games especially and the web in general are moving.

For other workloads, the spotty availability, instability, and security concerns are a major detractor for SkiaGL Canvas2D for various business or productivity oriented apps. Better stability and reliability are a boon there. Also, for text and filter intensive workloads, performance in software is actually better than the SkiaGL accelerated canvas (famously or infamously in CanvasMark).

Comment 20

[Shmerl]

Since Firefox is moving to WebRender (hopefully soon?) which isn't going to be using Skia either way, this is not a big deal really.

Comment 21

[Clemens Eisserer]

All Chrome-based browsers offer hw-accelerated canvas rendering, Firefox now doesn't.
This change just makes me sad, it completly throws away hw-accelerated canvas rendering, which once was a major selling point.

| For other workloads, the spotty availability, instability, and security concerns are a major detractor
| for SkiaGL Canvas2D for various business or productivity oriented apps.

What I like about canvas is, that it is guaranteed to work.
If supported, you get a hw-accelerated one, if not - you get a slower software fallback - but you can count on it to work.

With the proposed migration to WebGL, all you do, is to move the compatibility burden down to the developers.
You game does not work on card X with driver Y - not our fault - all we do is pass down your WebGL calls to the host driver.
(and, by the way, you a proposing a rewrite of all the existing, performance sensitive canvas games/applications)

Comment 22

[Darkspirit]

Please test gfx.canvas.accelerated (bug 1741501), it seems to be similar to https://github.com/jagenjo/Canvas2DtoWebGL.
Hopefully Mozilla can replace CPU canvas (skia) and GPU canvas with some Rust implementations at some point.

IIUC, it's hard to gpu-accelerate the Canvas API, developers should rather demand a software fallback for WebGL.
(Chromium seems to have replaced old SwiftShader (GL) with GL-to-Vulkan ANGLE on top of the new SwiftShader (Vulkan).)