On Thu, 1 Aug 2024 23:21:12 GMT, Andy Goryachev <ango...@openjdk.org> wrote:
>> Michael Strauß has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> fixed a bug
>
> modules/javafx.graphics/src/main/java/javafx/scene/layout/BackgroundPosition.java
> line 266:
>
>> 264: Side verticalSide, double verticalPosition,
>> boolean verticalAsPercentage) {
>> 265: return this.horizontalSide == horizontalSide
>> 266: && this.horizontalPosition == horizontalPosition
>
> so here `==` seems to be ok since we are comparing enums and primitive
> values, but...
> should we account for floating point errors that might appear during
> interpolation?
> i.e.
>
> isClose(this.horizontalPosition, horizontalPosition) &&
>
>
> where
>
> private static boolean isClose(double a, double b) {
> return Math.abs(a - b) < EPSILON; // 0.00001 or something like that should
> be ok for screen coordinates
> }
I don't think that this is necessary. It would only save an allocation in a
exceedingly rare case where the interpolation factor was microscopically
different from either 0 or 1. This shouldn't happen in practice.
> modules/javafx.graphics/src/main/java/javafx/scene/layout/Border.java line
> 331:
>
>> 329:
>> 330: for (int i = 0, max = strokes.size(); i < max; i++) {
>> 331: final BorderStroke stroke = strokes.get(i);
>
> just curious: do we really need `final` keyword for local variables after
> java8?
I don't use it, but I didn't change the existing code here.
-------------
PR Review Comment: https://git.openjdk.org/jfx/pull/1522#discussion_r1701071650
PR Review Comment: https://git.openjdk.org/jfx/pull/1522#discussion_r1701072138
