[prev in list] [next in list] [prev in thread] [next in thread] 

List:       openjdk-2d-dev
Subject:    Re: [OpenJDK 2D-Dev] RFR: 4497648: TextLayout equals method is not implemented
From:       Prasanta Sadhukhan <prasanta.sadhukhan () oracle ! com>
Date:       2016-06-20 12:23:18
Message-ID: 5767DD66.2030200 () oracle ! com
[Download RAW message or body]

Looks ok to me.

Regards
Prasanta
On 6/19/2016 4:46 AM, Philip Race wrote:
> Bug: https://bugs.openjdk.java.net/browse/JDK-4497648
> Webrev : http://cr.openjdk.java.net/~prr/4497648/
>
> TextLayout is a final class that derives directly from Object.
> It defines
> boolean equals(TextLayout)
> and over-rides
> boolean equals(Object)
> int hashCode();
>
> The hashCode() delegates to the implementation class TextLine().
> TextLine is also a final class that directly derives from Object
> It does over-ride Object.hashCode() but *does not* over-ride
> Object.equals(Object).
>
> Aside from this breaking the rule that you should over-ride neither or 
> both -
> migitated only by the fact that this is a non-API class -
> the equals(..) methods on TextLayout() delegate to this class.
>
> The result of all this is that TextLayout.equals(..) is really 
> Object.equals(),
> whilst TextLayout.hashCode() is not Object.hashCode().
>
> Further the javadoc on TextLayout.equals(TextLayout) says that the
> compared instances are equal if they contain the same glyph vectors.
> Note that whilst there is this and other commentary on the class
> mentioning glyph vectors, the GlyphVector class is not exposed
> anywhere in the API of TextLayout and we can take this only to
> infer that it is implying that the laid out text is equivalent.
> But as already noted there is no implementation to support
> comparing the internal state.
>
> Since this state of affairs goes back to 1.2 and it is neither easy,
> nor obviously valuable to be able to make such comparisons, we
> should make the spec. match the long standing implementation.
>
> So this fix removes the over-rides that add no value, and removes
> the wording about equal glyph vectors in favour of text that cannot
> be used to interpret that there is anything other than the minimal
> equivalence of equals. It would have been better if equals(TextLayout)
> had not been added at all but that ship has sailed.
>
> FYI .. I have already discussed this proposal off-line with the JCK team
> and they are happy so long as the spec. matches the implementation.
> CCC will be filed once this is reviewed - and I also had an off-line
> conversation with Joe Darcy.
>
> -phil.
>

[prev in list] [next in list] [prev in thread] [next in thread]