Ensure we use only frozen Skia APIs
Closed this issue · 11 comments
Skia seems to somewhat-regularly change its API. In theory, some classes are annotated SK_API, which indicates that they are frozen, while others are not frozen. We should ideally use only frozen APIs. That said, even "frozen" APIs can change; for example, SkPaint::getBlendMode was replaced with SkPaint::getBlendMode_or recently.
Still, perhaps we can assume that the SK_API classes and methods will not change much, and that skia-python can maybe paper over changes if they do (as they did with getBlendMode_or). Then the question is: what else do we need to avoid using? I did an exhaustive audit of our source code and here's what I got:
Definitely needs changing:
- Everything related to
FilterQuality, namely its use inPaint, its use indrawImageRect, and then the actual values of that enum, were all removed way back, like in m90 or something.
Maybe will change in the future:
-
BlendModeis a public enum but isn'tSK_API. (I'm not even sure if enums can beSK_APIbut in any caseBlendModeseems to have the same status asFilterQuality.) The fact thatgetBlendModechanged doesn't make me feel better—looks like Skia is moving toward a newBlenderabstraction. - A lot of the GPU-related APIs are from the "legacy GL API" and so are on their way out. There's now I think a new GPU driver? Maybe using Vulkan? I found this code pretty confusing.
- Am I correct that this changed in Skia? Do the original high/medium/low constants exist somewhere or do we need to do all the steps above to construct it?
Not sure, I'll check with the Skia team.
- Does Skia regularly make breaking changes? Is there any way to avoid? (This is already the second, after they removed
getBlendModein favor ofgetBlendMode_or.
They do sometimes make breaking changes. Did the Skia Python module already update and break out app? It seems to work locally.
It's a little tricky. The Skia-Python module has not put out a new "official" release in a while, but the last "official" release doesn't work with Python 3.12 so I've been installing their "beta" releases and those track API changes, including the breaking ones.
The SamplingOptions change is the first item in the release note: https://github.com/kyamagu/skia-python/blob/main/relnotes/README.m116.md#general-overview-of-changes-between-m87-and-m116
SkFilterQuality was removed June 2021, a bit under 3 years ago, in m94 : google/skia@aebe248
Anyway, the migration guide from FilterQuality to SamplingOptions is in:
kyamagu/skia-python#240
You'll need the upcoming skia-python m124 kyamagu/skia-python#236 to migrate FilterQuality related python code.
To be pedantic, it is actually 4 settings - high, medium, low and none. None is even lower than low, as far as I see, but it is the default (instead of medium, which one might assume is the case).
Hi @HinTak, apologies for the long delay—the semester just ended so it's been hectic. I'm working on getting skia-python compiled from source and I'll first work on filing bugs for all of our uses of old APIs. Then I'll work on converting them to tests.
I can confirm that, on m124, our browser test suite passes without issue. However, actually running the browser doesn't work due to (maybe?) kyamagu/skia-python#241, because all the text disappears due to the font metrics all returning zero:
@pavpanchekha yes, you are on mac os? I tried "Times" and it doesn't work either (reset the push after the failed try). If you can see what incantation that gives a usable font on mac os, that would be nice.
Ok, I found the issue: HinTak/skia-python#1
With this fix, I can successfully run the browser and it looks bit-for-bit identical to 87.5:
I'm going to test images with SamplingOptions and then think about what remains to do on this.
@pavpanchekha brilliant! That code is a bit confusing - how could it returns 5 zeros instead of none... I'll add that to the m124 pile now.
Closing this since we have now pinned the Skia version.