[OpenJDK 2D-Dev] RFR: 8221305: java/awt/FontMetrics/MaxAdvanceIsMax.java fails on MacOS + Solaris

[Philip Race]

On 4/28/20, 2:38 PM, Sergey Bylokhov wrote:
> On 4/28/20 1:28 pm, Philip Race wrote:
>>
>> You mean make sure maxAdvance is always at least as large as the
>> largest returned by getWidths() ?
>>
>> getWidths() is almost as useless as getMaxAdvance().
>> I grimace every time I look at it and can only imagine it was added 
>> as a sort
>> of performance cache.
>> It only returns the advances of the first 256 characters, and it does 
>> not
>> even care whether those characters are supported by the font.
>> Very often they are not as there's a bunch of unprintable code points 
>> in there.
>>
>>
>> And max advance is NOT only about the advance of the first 256 
>> characters anyway
> Yes but it makes it a little bit more useful, it least it will always 
> return correct max
> advance for the first characters.

Actually I'll argue it can be the opposite.
Doing this you ask it to go check in fallback fonts and in the failure I
analysed on macos we got a fallback from a CJK font for an unprintable 
character.
Now whilst that may make it "more likely" if we include that in the 
maxadvance
and care only about chars 0-255 it will pass, you've now got a very 
different max
advance returned than is the design centre for the primary font and REAL 
uses
of chars 0-255 which would never try to display char 132 etc.

>
>> So that would be completely artificial and you'd be potentially 
>> introducing
>> a change that would cause apps to layout differently to keep an 
>> useless test
>> working.
>
> Isn't such applications is not broken already if the getMaxAdvance() 
> returns
> the size smaller than the any of getWidths()? My suggestion is to 
> increase max
> maxAdvance if it is positive but smaller than getWidths.

They are already dealing with what they get today.
If you change it how can that not be more risky ?
And they aren't "broken". Realistically max advance is going to be
good enough for a lot of people and your proposal could mean
they now have a wider advance that affects layout without a benefit.
Yes it could also benefit someone else, but I don't see it as worth it
for the reasons out lined above.

So let's not try to randomly pick chars 0-255 as special and lets
not not try to artificially make a test pass that proves nothing useful.

-phil.
>
>>
>> -phil.
>>
>> On 4/28/20, 1:07 PM, Sergey Bylokhov wrote:
>>> Hi, Phil.
>>>
>>> Why we cannot change the implementation of getMaxAdvance() and
>>> make a quick check of the FontMetrics.getWidths() the same as in the
>>> test? I think it does not break the new spec and the test will pass.
>>>
>>> On 4/28/20 12:38 pm, Philip Race wrote:
>>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8221305
>>>> Webrev: http://cr.openjdk.java.net/~prr/8221305/
>>>>
>>>> Now that we have resolved
>>>> JDK-8230672 : Specification for 
>>>> java.awt.FontMetrics.getMaxAdvance() is too prescriptive.
>>>> with an approved CSR, the assertion being tested by 
>>>> java/awt/FontMetrics/MaxAdvanceIsMax.java is not enforceable.
>>>>
>>>> The test should just be retired.
>>>>
>>>>
>>>> -phil.
>>>
>>>
>
>