I will take a look into this, but I am confused. FT_Get_Color_Glyph_Paint cannot be NULL as it is a regular exported function. This change will affect its behavior to always return 0 (false) but that often happens anyway even without this change (most fonts don't have COLRv1 tables). For now it's fine to to revert the change as the original issue does not currently affect many users, but it is an issue that will need to be addressed.
On Thu, Sep 28, 2023 at 10:22 AM Hugh McMaster <h...@debian.org> wrote: > > On Thu, 28 Sep 2023 at 21:44, Hugh McMaster wrote: >> >> Hi Andres, >> >> On Thu, 28 Sept 2023 at 18:49, Andres Salomon wrote: >> > >> > Control: affects -1 chromium >> > >> > >> > On Thu, 28 Sep 2023 01:24:00 +0900 SuperCat wrote: >> > > Hi, >> > > >> > > In chromium source code, function SkScalerContext::GlyphMetrics >> > > SkScalerContext_FreeType::generateMetrics() will call >> > > FT_Get_Color_Glyph_Paint() if macro TT_SUPPORT_COLRV1 exists. Somehow >> > > FT_Get_Color_Glyph_Paint will be a NULL pointer if this patch applied, >> > > and >> > > chromium will not be able to run. >> > >> > >> > This smells like an ABI change that doesn't really seem appropriate for a >> > point release update. I can patch TT_SUPPORT_COLRV1 out of bookworm's >> > Chromium, but I wonder if any other packages are using it on bookworm? >> > >> > For the record, Skia has the following code: >> > >> > #ifdef TT_SUPPORT_COLRV1 >> > >> > // So undefine TT_SUPPORT_COLRV1 before 2.11.1 but not if FT_STATIC_CAST >> > is defined. >> > #if (((FREETYPE_MAJOR) < 2) || \ >> > ((FREETYPE_MAJOR) == 2 && (FREETYPE_MINOR) < 11) || \ >> > ((FREETYPE_MAJOR) == 2 && (FREETYPE_MINOR) == 11 && (FREETYPE_PATCH) >> > < 1)) && \ >> > !defined(FT_STATIC_CAST) >> > # undef TT_SUPPORT_COLRV1 >> > >> > >> > So on bullseye (with freetype 2.10) it doesn't try to use COLRV1. On sid >> > (with freetype 2.13) it will use COLRV1. If freetype's COLRV1 is going to >> > remain disabled on bookworm via the proposed-update (with chromium being >> > the only broken package), then I'll probably just bump that version check >> > to only allow TT_SUPPORT_COLRV1 with FREETYPE_MINOR >= 13. >> >> FreeType 2.12.1 was released with experimental COLRv1 support enabled. >> This was unintentional, as the implementation shipped in this release >> was incomplete and incompatible with the final COLRv1 API. >> >> Upstream's intention was to enable COLRv1 support in FreeType 2.13.0, >> which has a stable and complete COLRv1 API. >> >> I'm surprised Chromium actually used an experimental API, although >> this version check copied above seems like a bug. >> >> Grepping for TT_SUPPORT_COLRV1 yields a small number of packages. >> firefox*, godot and paraview are fine. Most of the openjdk-* packages >> aren't in bookworm. > > > After discussing the timing of Debian 12.2 with a release manager, I’ll > revert the change shortly. > > Hugh
