RFR: 8328896: Fontmetrics for large Fonts has zero width

[Phil Race]

On Thu, 18 Apr 2024 04:03:38 GMT, Tejesh R <tr at openjdk.org> wrote:

>> The main problem here is that we do not curate what font point size we passed to freetype,
>> and would pass in one larger than freetype's maximum of FT_USHORT_MAX which is USHRT_MAX.
>> This isn't documented, SFAICS, and is checked a couple of calls deep from the specific API we use.
>> But generally anywhere near that size seems to cause freetype to choke as it uses signed 16.16
>> values, so 32767 is really the max.
>> But we have no need to go anywhere near that - 16384 seems like a plenty large enough pt size.
>> And we can request bigger sizes than that by making use of the transform.
>> At normal size ranges we use that just to account for rotation and decompose the glyph transform
>> into point size and that rotation.
>> 
>> But at large sizes - which are not usefully rendered anyway - there are no hints etc to be lost
>> from not specifying the target point size. So we can extend the range of sizes we allow.
>> 
>> If  this is still too large to be held decomposed into a pt size in the range less than 16384 and a scale of up to 32766 then we substitute the null scaler, as we generally do when values are out of range, such
>> a for a NaN transform.
>> 
>> These extreme values aren't useful.
>> 
>> In looking at this I did find that getGlyphPixelBounds doesn't follow the maximum image size we use
>> for rendering. Which is not useful and could lead to inconsistent results. I fixed that.
>> 
>> Also whilst macOS didn't have these problems it could be cleaned up a bit for consistency in the reported sizes for some cases.
>> 
>> The test is mainly around making sure that "sensible" things are returned for not sensible input.
>> There's no 100% right answer to these extreme unrenderable sizes.
>
> src/java.desktop/share/native/libfontmanager/freetypeScaler.c line 537:
> 
>> 535:     if (TOO_LARGE(dmat[0], ptsz) || TOO_LARGE(dmat[1], ptsz) ||
>> 536:         TOO_LARGE(dmat[2], ptsz) || TOO_LARGE(dmat[3], ptsz))
>> 537:     {
> 
> Suggestion:
> 
>      TOO_LARGE(dmat[2], ptsz) || TOO_LARGE(dmat[3], ptsz)) {

No, that was deliberate. We generally prefer the { on a new line if the conditional is multi-line.

Still looking for an actual "reviewer" review here.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/18703#discussion_r1571126135