On Fri, 4 Nov 2022 19:02:05 GMT, Alexey Ivanov <aiva...@openjdk.org> wrote:

>> The methods like Rigion.dimAdd/clipScale should be used everywhere the 
>> overflow is possible.
>
>> > Thank you, @mrserb, for your suggestion. I didn't know about those methods.
>> > We already have [JDK-8294680](https://bugs.openjdk.org/browse/JDK-8294680) 
>> > to refactor this code. If you don't object, I'd rather postpone updating 
>> > the calculations to a later time. We should absolutely re-use the existing 
>> > code if it does the same thing.
>> 
>> @aivanov-jdk I think we decided to retain roundHalfDown and make the changes 
>> when refactoring the code.
> 
> The comment above is about `Rigion.dimAdd` and `Rigion.clipScale`.
> 
> As for `roundHalfDown`, you had replaced it with `Region.clipRound`. When I 
> was reading the code, I got the impression that you used `Region.clipRound` 
> everywhere but one place. Eventually, I see you've reverted that change, and 
> you're using `roundHalfDown` with no trace of `clipRound` left to be found.
> 
> What I saw must have been coming from code comments rather than the real code.
> 
> [The comment 
> above](https://github.com/openjdk/jdk/pull/10274#discussion_r990278757) 
> confirms that using `Region.clipRound` gives the same result:
> 
>> > The sun.java2d.pipe.Region#clipRound() seems have the same purpose, but it 
>> > also clip the double to int.
>> 
>> … It's better to re-use the existing implementation. I tested with it, it 
>> gives the same result.
> 
> It went into #10681.
> 
> I just missed that you had reverted it back.
> 
> I guess it's not worth re-doing it once again. We'll handle it in 
> [JDK-8294680](https://bugs.openjdk.org/browse/JDK-8294680) too.

Sounds good. I'll have it updated during 
[JDK-8294680](https://bugs.openjdk.org/browse/JDK-8294680) changes.

-------------

PR: https://git.openjdk.org/jdk/pull/10274

Reply via email to