On Wed, 12 Oct 2022 21:16:47 GMT, Sergey Bylokhov <s...@openjdk.org> wrote:
>> @mrserb Thank for the suggestion. Are you suggesting that same >> transformations be applied for width, height and additionally to x&y >> translations? >> https://github.com/openjdk/jdk/blob/ec4fb47b90c9737dfdc285ebe98367a221c90c79/src/java.desktop/share/classes/sun/java2d/SunGraphics2D.java#L373 > > 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. ------------- PR: https://git.openjdk.org/jdk/pull/10274
