[OpenJDK 2D-Dev] RFR: 8231556: Wrong font ligatures used when 2 versions of same font used

[Phil Race]

You are right that is not what I intended. In practice it shifted 
quality from the
fullName to the platName which would work well for everything except ttc 
files
but clearly that isn't enough :

Updated webrev :
http://cr.openjdk.java.net/~prr/8231556.2/

-phil.

On 12/19/19 11:50 AM, Kevin Rushforth wrote:
> Are the parentheses in the right place here?
> + return
> + (this.fullName.equals(other.fullName)) &&
> + (this.platName == null && other.platName == null) ||
> + (this.platName != null && this.platName.equals(other.platName));
>
> This is equivalent to:
>
> + return
> + ( (this.fullName.equals(other.fullName)) &&
> + (this.platName == null && other.platName == null) ) ||
> + (this.platName != null && this.platName.equals(other.platName));
>
> which will return true if platName == other.platName even if the 
> fullName is not equal. Maybe I'm missing something.
>
> -- Kevin
>
>
> On 12/19/2019 11:35 AM, Phil Race wrote:
>> 2nd reviewer please.
>>
>> -phil.
>>
>> On 12/11/19 4:17 PM, Sergey Bylokhov wrote:
>>> Looks fine.
>>>
>>> On 12/11/19 4:11 pm, Phil Race wrote:
>>>> Apparently you were waiting for an answer on the hashCode question 
>>>> which I had not seen.
>>>> I don't think it matters much in practice. A clash would only newly 
>>>> occur in this rare case
>>>> but I can include it .. http://cr.openjdk.java.net/~prr/8231556.1/
>>>>
>>>> -phil.
>>>>
>>>> On 12/7/19 6:24 PM, Philip Race wrote:
>>>>> > probably this check can be added to the current method
>>>>> > in PhysicalFont and removed from the CFont?
>>>>>
>>>>> Addressing this point, it *could* but if that was the best solution
>>>>> I would have done it back then. It is a macos fix in macos code which
>>>>> seems best to me.
>>>>>
>>>>> -phil.
>>>>>
>>>>> On 12/7/19, 4:39 PM, Philip Race wrote:
>>>>>>
>>>>>> I am not sure. I don't remember doing the follow on fix mentioned 
>>>>>> there
>>>>>> to make the full name of the components of logical fonts on macos 
>>>>>> have
>>>>>> different names. And since at least two of the logical fonts on 
>>>>>> macos are
>>>>>> in .ttc files then they'd have the same file name .. so I think 
>>>>>> that over-ride
>>>>>> is still needed.
>>>>>>
>>>>>> -phil.
>>>>>>
>>>>>> On 12/7/19, 4:00 PM, Sergey Bylokhov wrote:
>>>>>>> Hi, Phil.
>>>>>>>
>>>>>>> A few years ago in JDK-8139176 we added a check for the "style" 
>>>>>>> to the
>>>>>>> CFont.equals(), probably this check can be added to the current 
>>>>>>> method
>>>>>>> in PhysicalFont and removed from the CFont?
>>>>>>>
>>>>>>> On 12/7/19 11:59 am, Philip Race wrote:
>>>>>>>> The equals() method was refactored a bit to make it easier to add
>>>>>>>> the extra tests, and some of the complexity is due to paranoia 
>>>>>>>> that
>>>>>>>> platName *might* be null .. although I don't think that should 
>>>>>>>> ever be the case.
>>>>>>>
>>>>>>> I think code can be simplified if Objects.equals() will be used
>>>>>>> Don't we need to update the hashcode as well?:
>>>>>>>     return Objects.hash(fullName, platName);
>>>>>>>
>>>>>>>
>>>>
>>>
>>>
>>
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.java.net/pipermail/2d-dev/attachments/20191219/3efe82d9/attachment.htm>