search

kyamagu / skia-python

Python binding to Skia Graphics Library
https://kyamagu.github.io/skia-python/
BSD 3-Clause "New" or "Revised" License
245 stars 43 forks source link

m122: SkFontMgr::RefDefault() has been deleted #231

Closed HinTak closed 6 months ago

HinTak commented 9 months ago

Skia m122 has a rather troubling change:

    Milestone 122
    -------------
      * `SkFontMgr::RefDefault()` has been deleted. Clients should instantiate and manage their own
        `SkFontMgr`s and use them to explicitly create `SkTypeface`s

This functionality is being moved into Chromium itself:

https://github.com/chromium/chromium/commit/a6166e825abc9b0b0359115fc9d95de4bbcd5b9c

I see no good alternative except essentially copying most of chromium:skia/ext/font_utils.cc and stick it near the top of our src/skia/Font.cpp, emulating the removed SkFontMgr::RefDefault(). Without a workingSkFontMgr::RefDefault() or equivalent (SkFontMgr::RefEmpty() is interface-identical and available, but it does nothing ; if you want something, you need to call SkFontMgr_New_FCI [linux fontconfig] etc directly, for each platform ), we are getting an extra of 44 failed and 62 errors compared to two days ago.

This is essentially all the difference between upstream skia m121 and m122, as far as skia-python is concerned, moving the "central font mgr api" into chromium. @kyamagu

HinTak commented 9 months ago

@kyamagu I have a m122 that compiles, and I think I can emulate the deleted SkFontMgr::RefDefault(), but it is copying maybe about a 100 lines from chromium more or less, and it won't be pretty. Mostly this is a fore-warning.

kyamagu commented 9 months ago

Hmm, so you mean there won't be logic equivalent to chromium:skia/ext/font_utils.cc in the skia codebase? I agree copying from chromium is not future-proof. Even if doing so, there should be a mechanism to pull the latest code from the chromium repository via sparse checkout or equivalent.

HinTak commented 9 months ago

It doesn't look like it. They have simply moved the centralized functionality of "give me a font matching a description in the current OS platform" from skia to chromium; on skia you are left with a few per-platform optionally compiled in font managers, Linux build of skia has fontconfig, Windows build of skia has directwrite, mac build of skia has coretext. It is basically skia's src/core/SkFontMgr_default.cc or something modified to chromium's skia/ext/font_utils.cc (chromium has skia submodule under third_party/skia, so the bottom skia location is for skia-interacting code within chromium).

Chromium is much larger than skia even without submodules... besides, the file expects the rest of skia to be under third_party/skia, so it cannot be used as it is.

kyamagu commented 9 months ago

Sure, it is okay if the copying won't cause a big maintenance effort in the future.

HinTak commented 9 months ago

This is the only change between 121 and 122, plus the removal/emulation of Typeface.MakeFrom*, which is for the same reason - those assume a "default" font manager to get typeface matching descriptions. The added code for emulating FontMgr.Refdefault is about 50 lines - slightly simplified from the the skia 121 or chrome 122, since we don't care about android etc. It segfaults at the moment... I 'll post it as a pull at some point, but since it has no advantage over 121, I would say we do the same as 118, not merge until we need to add something interesting, or fix some broken things since 87.