1444506 - Update Skia to milestone 66

Status: RESOLVED FIXED

(Core :: Graphics, enhancement)

Description

[Lee Salzman [:lsalzman]]

We're still on milestone 59. Milestone 66 is already out, so it is about time we updated.

Comment 1

[Lee Salzman [:lsalzman]]

Attachment: Moz2D fixes for Skia m66 update

A lot of fixes here to deal with code churn.

Skia no longer fully allocates the padding on the last row, so in some cases I have to do that manually now.
LayerRasterizer is gone, so I had to rewrite Mask().
Some churn in GrGL interface.
Other miscellaneous little things.

Comment 2

[Lee Salzman [:lsalzman]]

Attachment: mozbuild fixes for Skia m66 update

I needed to do a few fixes to deal with their changing gn setup, but otherwise mostly just file churn.

Comment 3

[Lee Salzman [:lsalzman]]

Attachment: GL fixes for Skia m66 update

A few things:

The android emulator does not actually support BGRA8888, so reporting it breaks things. Thusly, we should no longer report it to Skia.

Some GL types and function prototypes had to be matched to more closely match both the khronos platform headers and Skia's GL headers with more closely match khronos'. I checked on all platforms, and our header was the odd man out on definitions of GLboolean/GLintptr/GLsizeiptr and a prototype or two. Otherwise, there were type errors with SkiaGL's function pointer interface.

Some new function pointers that SkiaGL is using.

Comment 4

[Lee Salzman [:lsalzman]]

Attachment: wrap STL mutex header

Skia depends on using std::mutex in a few cases, so we need to expose this.

Comment 5

[Lee Salzman [:lsalzman]]

Attachment: don't attempt to draw an invalid video source in canvas

VideoWidth/VideoHeight can sometimes return 0, which only happens when the video source is invalid. In that case, we'd actually supply source width/height that was Inf and/or NaN, totally screwing up the rendering.

Skia very much doesn't like this anymore and crashes. Therefor, we no longer torture DrawTargetSkia with these non-finite FP values.

Comment 6

[Lee Salzman [:lsalzman]]

Attachment: fuzz for Skia m66 update

Mostly harmless fuzz churn. Some rendering results changes by 1 increment here or there.

Comment 7

[Lee Salzman [:lsalzman]]

Attachment: Update to Skia m66

Updated sources for moz-skia now in my moz-m66 branch.

The main things I had to change here is disabling their new Delta AA for now, which mostly seems broken for our use-cases.

Workarounds for some macOS driver bugs.

Re-enabling some deprecated clipping features.

Had to add support for SkDevice snapshots to support our PushLayer/PopLayer setup until I can properly vet the functionality/performance of their new layer masking support.

Had to backport SkConvolver/SkBitmapScaler support which our imagelib depends on, but which they decided to remove.

Various build fixes to work across all our compiler toolchains and get past static analysis.

... And otherwise just fixing a metric ton of bitrot with our other patches.

Comment 8

[Jeff Muizelaar [:jrmuizel]]

Comment on attachment 8957672 [details] [diff] [review]
Moz2D fixes for Skia m66 update

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

::: gfx/2d/DrawTargetSkia.cpp
@@ +401,5 @@
>  
>  static bool
>  ExtractAlphaBitmap(const sk_sp<SkImage>& aImage, SkBitmap* aResultBitmap)
>  {
>    SkImageInfo info = SkImageInfo::MakeA8(aImage->width(), aImage->height());

Add a comment about why we're doing this stride exercise here.

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/84a6ea4264ca
part 1 - update to Skia m66. r=jrmuizel
https://hg.mozilla.org/integration/mozilla-inbound/rev/ab6b0bafaab4
part 2 - Moz2D fixes for Skia m66 update. r=jrmuizel
https://hg.mozilla.org/integration/mozilla-inbound/rev/12dba47d2024
part 3 - mozbuild fixes for Skia m66 update. r=jrmuizel
https://hg.mozilla.org/integration/mozilla-inbound/rev/614317354b14
part 4 - GL fixes for Skia m66 update. r=jrmuizel
https://hg.mozilla.org/integration/mozilla-inbound/rev/e73b781388a8
part 5 - don't attempt to draw an invalid video source in canvas. r=jrmuizel
https://hg.mozilla.org/integration/mozilla-inbound/rev/f3410beb9eb1
part 6 - fuzz for Skia m66 update. r=jrmuizel

Comment 10

[Ionuț Goldan [:igoldan]]

Big perf improvements won! \o/

== Change summary for alert #12083 (as of Mon, 12 Mar 2018 18:38:01 GMT) ==

Improvements:

154%  rasterflood_gradient linux64 pgo e10s stylo     288.58 -> 733.25
137%  rasterflood_gradient linux64 opt e10s stylo     316.83 -> 750.58
 10%  tsvgx linux64 pgo e10s stylo                    208.25 -> 187.19
 10%  tsvgx linux64 opt e10s stylo                    215.02 -> 194.11
  4%  tscrollx linux64 opt e10s stylo                 0.87 -> 0.83

For up to date results, see: https://treeherder.mozilla.org/perf.html#/alerts?id=12083

Comment 11

[Andreea Pavel [:apavel]]

https://hg.mozilla.org/mozilla-central/rev/84a6ea4264ca
https://hg.mozilla.org/mozilla-central/rev/ab6b0bafaab4
https://hg.mozilla.org/mozilla-central/rev/12dba47d2024
https://hg.mozilla.org/mozilla-central/rev/614317354b14
https://hg.mozilla.org/mozilla-central/rev/e73b781388a8
https://hg.mozilla.org/mozilla-central/rev/f3410beb9eb1

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/9bdc2ccc6f2f
follow-up - use SkMD5. r=me

Comment 13

[Andreea Pavel [:apavel]]

https://hg.mozilla.org/mozilla-central/rev/9bdc2ccc6f2f

Comment 14

[Joel Maher ( :jmaher ) (UTC -8)]

there is one regression that came from this:
 	tsvgx windows7-32 opt e10s stylo  (untriaged)   graph · subtests 	452.97 	< 	478.02 	5.53%

you can see all the other wins:
https://treeherder.mozilla.org/perf.html#/alerts?id=12083

what is interesting is on autoland an improvement landed about the same time and when it all merged we end up with the same results.

:lsalzman, I know win7 runs in different graphics modes than the other OS flavors, is this concerning to you at all or should we accept it?  it is 5%+

Comment 15

[Lee Salzman [:lsalzman]]

(In reply to Joel Maher ( :jmaher) (UTC-5) from comment #14)
> there is one regression that came from this:
>  	tsvgx windows7-32 opt e10s stylo  (untriaged)   graph · subtests 	452.97 
> < 	478.02 	5.53%
> 
> you can see all the other wins:
> https://treeherder.mozilla.org/perf.html#/alerts?id=12083
> 
> what is interesting is on autoland an improvement landed about the same time
> and when it all merged we end up with the same results.
> 
> :lsalzman, I know win7 runs in different graphics modes than the other OS
> flavors, is this concerning to you at all or should we accept it?  it is 5%+

I can look into this and see if there is anything I can do with it today/tomorrow. Regardless, we can't backout the Skia update overall over this, since we're already a year out of date with upstream Skia, and for security reasons and just avoiding the forking of Skia, we need to take the update.

None the less, as noted, I will investigate what's going on and try to solve that since the same subtests show some regressions even on Linux, but to a smaller degree to where they don't poison the summary score.

Comment 16

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/9064f0a3fb62
follow-up - support BGRA in bilerp_clamp_8888 fast-path. r=me

Comment 17

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/8007185c11c2
follow-up - upstream bug fixes. r=me

Comment 18

[Lee Salzman [:lsalzman]]

Okay, that should take care of the performance regression to some extent. We fell off a fast-path in some of their new rasterization code. I added an easy workaround that routes us back into it and puts the tsvgx summary scores at a net-positive again on Win7.

Comment 19

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/e2ae2186dbc9
follow-up - disable explicit GPU resource allocation on all platforms. r=dholbert

Comment 20

[Andreea Pavel [:apavel]]

https://hg.mozilla.org/mozilla-central/rev/9064f0a3fb62
https://hg.mozilla.org/mozilla-central/rev/8007185c11c2

Comment 21

[Stefan Hindli [:stefan_hindli]]

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

Comment 22

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/6591b4cc0933a80c8c1f75b8fc562385b6518a9c
https://hg.mozilla.org/releases/mozilla-beta/rev/fed09e8e9884ea4b5b0cc6faddd568fdc76054e9
https://hg.mozilla.org/releases/mozilla-beta/rev/e4b6d0d4c44329564ad771a707ecf2847f556fe3
https://hg.mozilla.org/releases/mozilla-beta/rev/18cc24b781fb4ca01031677d22fdff10c5e41e80
https://hg.mozilla.org/releases/mozilla-beta/rev/91ea59ff7425cdc67aa77a3a0b46aadba165f10d
https://hg.mozilla.org/releases/mozilla-beta/rev/0c7ea7d9aede9a18cecc52c563d93f4dae449cca
https://hg.mozilla.org/releases/mozilla-beta/rev/2d7dff9cd84c9edfdcbf5b28c2d61058a46e0c26
https://hg.mozilla.org/releases/mozilla-beta/rev/86e67789dfa793bc51da46ce5eb1dfbe8611cdae
https://hg.mozilla.org/releases/mozilla-beta/rev/2bf3d2cdef340f1352ec2a0e91ded10b4e579236
https://hg.mozilla.org/releases/mozilla-beta/rev/23a13199eb6e47552be72347e96d5fab45378209

Comment 23

[Ionuț Goldan [:igoldan]]

New perf improvements since comment 20:

== Change summary for alert #12310 (as of Thu, 22 Mar 2018 05:53:57 GMT) ==

Improvements:

  6%  tsvgx windows7-32 opt e10s stylo     461.62 -> 434.07
  6%  tsvgx windows7-32 pgo e10s stylo     395.45 -> 372.32

For up to date results, see: https://treeherder.mozilla.org/perf.html#/alerts?id=12310