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

[Kevin Rushforth]

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/d77c8532/attachment.htm>