Closed Bug 1246756 Opened 4 years ago Closed 4 years ago

Update Skia to m49 branch

Categories

(Core :: Graphics, enhancement)

enhancement
Not set

Tracking

()

RESOLVED FIXED
mozilla47
Tracking Status
firefox47 --- fixed

People

(Reporter: lsalzman, Assigned: lsalzman)

References

(Blocks 1 open bug)

Details

Attachments

(3 files)

We should update to the stable m49 branch to incorporate fixes, as we forked m48 in the middle of a development cycle.
The main changes are:

SkCanvas drawSprite got removed, but drawBitmap was changed so that its behavior in the circumstances we used it now mimics drawSprite.

SkShader::CreateLocalMatrixShader -> newWithLocalMatrix 

They added SaveLayerRec to SkCanvas, which allows a "backdrop" to be specified when a new layer is saved. The backdrop is an image filter run over the previous top layer used to initialize the new top layer. Using this allows us to slowly remove some of our deprecated SkBitmap usage in PushLayer. They don't have a standard identity image filter yet, so I had to make one for this. Mentioned this to upstream so eventually in m50 or later we might get one.

Some type fixes in SkDashPathEffect for API changes.
Attachment #8717127 - Flags: review?(jmuizelaar)
Attachment #8717129 - Flags: review?(jmuizelaar)
Attachment #8717127 - Flags: review?(jmuizelaar) → review+
Attachment #8717129 - Flags: review?(jmuizelaar) → review+
This patch just includes only, for review, the new commits I had to make to get us up and running:

* add new swizzler optimizations to SkOpts_sse2
* replace C++11 standard library usage with MFBT
* work around bug in gcc < 4.7.3 used for hazard builds: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=54947

The full git repo branch with commit history of these changes is here: https://github.com/lsalzman/moz-skia/commits/moz-m49

Some older commits were removed because they have now been incorporated upstream.

I moved our previous branch with our forked m48 to:
https://github.com/lsalzman/moz-skia/commits/moz-m48

Main parts of the changes are the MFBT usage:

Needed to tweak SkToBool to not draw in ambiguous conversions from Windows system headers that only occurred with our UniquePtr. I tried a lot of workarounds, but none actually worked other than to just change SkToBool to not compare against 0.

math.h has some unfortunate side-effects like defining isfinite and friends as preprocessor defines. But this does not agree with our unified sources and <algorithm>, which pulls in <cmath> implicitly. No other way to work around that one than to just use <cmath> instead.

skstd::unique_ptr was by far the largest and most varied consumer of new C++11 functionality in public headers, so just replacing it with UniquePtr was a big win in terms of getting the amount of template shimmage small.

Currently will leak unique_ptr and forward as preprocessor defines in public headers, and the rest are confined only to when Skia is built. The amount of preprocessor renaming leaking into public headers will unfortunately grow in later releases, but for now we got away relatively lucky.

Due to the fact that other C++ headers can implicitly draw in headers like <functional>, it is not safe to place empty stubs for these. I had to just remove the includes of them for now and relocate their inclusion to Skia's template omnibus header.
Attachment #8717135 - Flags: review?(jmuizelaar)
Attachment #8717135 - Flags: review?(jmuizelaar) → review+
https://hg.mozilla.org/mozilla-central/rev/7efdcd187a97
https://hg.mozilla.org/mozilla-central/rev/e6731baa8120
https://hg.mozilla.org/mozilla-central/rev/159e0a5a653f
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla47
This resulted in a minor tcanvasmark improvement. \o/

https://treeherder.allizom.org/perf.html#/alerts?id=85
(In reply to William Lachance (:wlach) from comment #7)
> This resulted in a minor tcanvasmark improvement. \o/
> 
> https://treeherder.allizom.org/perf.html#/alerts?id=85

Awesome, one of the few times an update didn't uniformly regress us. Celebration!
Depends on: 1248228
Depends on: 1248416
Blocks: 1229977
You need to log in before you can comment on or make changes to this bug.