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

Kevin Rushforth kevin.rushforth at oracle.com
Thu Dec 19 21:16:54 UTC 2019


Looks good.

+1

-- Kevin


On 12/19/2019 1:06 PM, Phil Race wrote:
> 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/2b8f8f0c/attachment.htm>


More information about the 2d-dev mailing list