Closed Bug 1468801 Opened 6 years ago Closed 6 years ago

Disable SkiaGL

Categories

(Core :: Graphics: Canvas2D, defect)

defect
Not set
normal

Tracking

()

VERIFIED FIXED
mozilla63
Tracking Status
firefox-esr52 --- wontfix
firefox-esr60 --- wontfix
firefox61 --- wontfix
firefox62 --- verified
firefox63 --- verified

People

(Reporter: lsalzman, Assigned: lsalzman)

References

(Regressed 2 open bugs)

Details

(Whiteboard: [geckoview:klar:p2])

Attachments

(1 file)

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.
Attached patch deprecate SkiaGL for Canvas2D (deleted) — Splinter Review
Pref Skia off as a preliminary step before we remove any code.
Attachment #8985427 - Flags: review?(snorp)
I worries that it hugely decrease fps of some website like testdrive-archive.azurewebsites.net/performance/fishbowl See Bug 1411481.
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.
Attachment #8985427 - Flags: review?(snorp) → review+
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla63
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 .
Flags: needinfo?(lsalzman)
To avoid the uplift risk for macOS, we could uplift with an Android #ifdef.
Whiteboard: [geckoview:klar:p2]
(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.
Flags: needinfo?(lsalzman)
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.
Flags: needinfo?(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
Flags: needinfo?(lsalzman)
Attachment #8985427 - Flags: approval-mozilla-beta?
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).
Attachment #8985427 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
We can do a spotcheck here on macOS using some Canvas demos to make sure that this did not caused any issues.
Flags: qe-verify+
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.
Status: RESOLVED → VERIFIED
Flags: qe-verify+
Did anyone benchmark Vulkan backend for Skia? See https://skia.org/user/special/vulkan I opened a feature request to enable it (bug 1502740).
Blocks: 1530471

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.

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

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.

Regressions: 1590515
Regressions: 1597937
Regressions: 1602299
Regressions: 1759155

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)

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

You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: