On Wed, 1 Nov 2023 18:52:15 GMT, Phil Race <p...@openjdk.org> wrote:

>> make/modules/java.desktop/lib/Awt2dLibraries.gmk line 499:
>> 
>>> 497:    LIBFONTMANAGER_EXCLUDE_FILES += hb-ft.cc hb-subset-cff-common.cc \
>>> 498:        hb-subset-cff1.cc hb-subset-cff2.cc hb-subset-input.cc 
>>> hb-subset-plan.cc \
>>> 499:        hb-subset.cc hb-subset-instancer-solver.cc gsubgpos-context.cc 
>>> hb-style.cc
>> 
>> I'm quite confused from where you got gsubgpos-context.cc. I don't see any 
>> file of that name.
>> Also I just realised that since the Nov 2020 import of freetype it has a 
>> bunch of handly ifdefs that
>> you can see in hb-config.h
>> And so probably a better way of doing this, even if it means one of these 
>> files (the instance-solver one) doesn't seem to get the ifdef treatment is 
>> to use the specific defines passed to the build.
>> Then I'd expect harfbuzz to apply the same ifdef even if these files got 
>> refactored.
>> I further noticed that the same 2020 update adds HAVE_FREETYPE and hb-ft.cc 
>> isn't compiled unless you actually set that. Which we don't
>> So this whole exclude can go away and be replaced by a few -D defines.
>> You could probably go overboard and experiment with all sorts of 
>> combinations from hb-config.h but I recommend you just apply the minimal set 
>> that makes your build time happier.
>> hb-style.cc isn't one I'd have realised we don't currently need .. and for 
>> many of them it isn't clear to me without research what the consequences 
>> would be of excluding it. Without that support would something non-optimal 
>> happen in some cases - you can't trust the tests we have to come near 
>> proving it.
>> I know we don't subset, and that's the only one I consider "safe" to use 
>> here without actual research.
>
> These two
> #define HB_NO_SUBSET_LAYOUT
> #define HB_NO_SUBSET_CFF

You can find the file here:
https://github.com/openjdk/jdk/blob/f262f06c97b9ea94cd6119b3a8beb16bf804d083/src/java.desktop/share/native/libharfbuzz/graph/gsubgpos-context.cc

Hb-subset files are not guarded by ifdefs; HB_NO_SUBSET_LAYOUT only excludes 
some functions, but not others. I'll check the impact of that define when I'm 
back in office.

I checked the list of HB APIs 
[here](https://harfbuzz.github.io/reference-manual.html) ; hb-subset and 
hb-style belong to non-core API sets, so I decided to check if hb-style was 
needed. It was not.

We can safely stop compiling the files listed because:
- none of the harfbuzz functions are exported, and they are not accessible 
outside of libfontmanager.
- if we excluded any file that was actually used, linker would refuse to link 
libfontmanager

Given that we only copy an arbitrary subset of harfbuzz files anyway (see 
UPDATING.txt), would it make sense to remove these files instead?

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/16440#discussion_r1379353297

Reply via email to