I have been able to figure out what is going on. The crash is due to a
typo in FreeType which was recently fixed [0]. This change is also
needed. I can confirm in a local build that with this typo fix the
reported Chromium crash (in libfreetype.so.6) is fixed. To be clear,
this FreeType change [0] is needed in addition to the the COLRv1
disable [1].

[0] 
https://gitlab.freedesktop.org/freetype/freetype/-/commit/16f311d72582c117796a23e22074fe9624760ee1
[1] 
https://salsa.debian.org/debian/freetype/-/commit/f65637c7f84fa2ccfb14c11d0ed0f315fe83636d

On Thu, Sep 28, 2023 at 10:36 AM Ben Wagner <bunge...@chromium.org> wrote:
>
> 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

Reply via email to