[OpenJDK 2D-Dev] [9] Review Request: Cleanup of sun.java2d.pipe.Region

[Jim Graham]

Looks good +1.

Is there a bug filed for this?

			...jim

On 5/12/16 1:50 PM, Sergey Bylokhov wrote:
>> This looks great!
>>
>> I guess the final was removed from the getters because the class is now
>> final, though it probably doesn't hurt anything to leave them final.  In
>> any case, you only removed it from 3 of them, so it is unbalanced now.
>
> Ouch, it was the last minute change. Yes, the final keywords were removed because I change the class itself to final.
> An updated version:
> http://cr.openjdk.java.net/~serb/2d_cleanup/webrev.01
>
>>
>> I suppose Hotspot is smart enough to inline accessors in a wide variety
>> of conditions, even non-final methods on non-final classes in some
>> cases, but I usually make accessors final to underscore their status as
>> a bare accessor even when it isn't really necessary.  Just a personal
>> preference...
>>
>>             ...jim
>>
>> On 05/12/2016 12:04 PM, Sergey Bylokhov wrote:
>>> Hello.
>>>
>>> Can somebody take a look to the proposed cleanup of
>>> sun.java2d.pipe.Region. When I worked on some bugs I got a situation
>>> when I tried to change the Region object via a different set methods.
>>> But these changes are ignored because the reference was to
>>> ImmutableRegion which replaces all setter to no-op methods.
>>>
>>> In the fix I propose to remove specific ImmutableRegion class, and make
>>> the whole Region class immutable:
>>>   - setXX methods are removed, since most of them are unused.
>>>   - the new getInstance(int box[], SpanIterator) was added, so the
>>> appendSpans() can be changed to the private.
>>>   - small cleanup in equals, toString.
>>>
>>> If the change will be approved I will file a corresponding CR.
>>>
>>> Webrev can be found at:
>>> http://cr.openjdk.java.net/~serb/2d_cleanup/webrev
>>>
>>>
>
>