On Mon, 2 Mar 2026 05:38:22 GMT, Phil Race <[email protected]> wrote: > > > > Below files were present in 2.13.2 upstream but our build worked fine > > > > without them. But with FreeType 2.14.1 our build >fails without these > > > > files, so added them also: > > > > src/java.desktop/share/native/libfreetype/src/autofit/ft-hb.c > > > > src/java.desktop/share/native/libfreetype/src/autofit/ft-hb.h > > > > > > > > > That sounds wrong. These should only be needed if > > > FT_CONFIG_OPTION_USE_HARFBUZZ is defined. And it isn't. All uses of the > > > code in those files is protected by an ifdef .. unless upstream made a > > > mistake somewhere. > > > > > > @prrace If i remove the addition of these files, we see following build > > error: > > ``` > > * For target support_native_java.desktop_libfreetype_afadjust.o: > > In file included from > > src/java.desktop/share/native/libfreetype/src/autofit/afadjust.c:20: > > In file included from > > src/java.desktop/share/native/libfreetype/src/autofit/afadjust.h:26: > > In file included from > > src/java.desktop/share/native/libfreetype/src/autofit/afglobal.h:25: > > src/java.desktop/share/native/libfreetype/src/autofit/afmodule.h:25:10: > > fatal error: 'ft-hb.h' file not found > > #include "ft-hb.h” > > ``` > > > > > > > > > > > > > > > > > > > > > > > > This include hits multiple files and we see failure at many places. > > Even if we make this import conditional using > > `FT_CONFIG_OPTION_USE_HARFBUZZ` Or `FT_CONFIG_OPTION_USE_HARFBUZZ && > > FT_CONFIG_OPTION_USE_HARFBUZZ_DYNAMIC`(as done in same file in this > > update). We continue to see errors like : > > ``` > > * For target support_native_java.desktop_libfreetype_afcjk.o: > > src/java.desktop/share/native/libfreetype/src/autofit/afcjk.c:104:12: > > error: call to undeclared function 'ft_hb_enabled'; ISO C99 and later do > > not support implicit function declarations [-Wimplicit-function-declaration] > > if ( ft_hb_enabled( metrics->root.globals ) ) > > ``` > > > > > > > > > > > > > > > > > > > > > > > > Build passes only after adding the new files > > `src/java.desktop/share/native/libfreetype/src/autofit/ft-hb.c` & > > `src/java.desktop/share/native/libfreetype/src/autofit/ft-hb.h` as-is > > without any modifications. > > Hmm. I see what they've done. I don't like it much. > > (1) an include not protected by the ifdef > > (2) You have to pull in those files to get the version of ft_hb_enabled() > which returns false (see below) when they should have been consistent and > used macros instead of doing this. So seems we now need the files even if we > don't use them. I'd be tempted to file a bug, but they may not care about > subsetting as a use case. > > #else /* !FT_CONFIG_OPTION_USE_HARFBUZZ_DYNAMIC */ > > FT_LOCAL_DEF( FT_Bool ) ft_hb_enabled( struct AF_FaceGlobalsRec_ *globals ) { > FT_UNUSED( globals ); > > #ifdef FT_CONFIG_OPTION_USE_HARFBUZZ return TRUE; #else return FALSE; #endif }
Thanks for pointing out the exact code. Yes these files are needed even-though it is not related to the subset of FreeType scalar code that we pick from upstream. I can't think of enough reasoning using which we can push for refactoring in upstream code and its better to not deviate and introduce JDK specific FreeType changes. So just adding these files is better for maintaining FreeType in JDK code. ------------- PR Comment: https://git.openjdk.org/jdk/pull/29954#issuecomment-3982479603
