RFR: 8087444: CornerRadii with different horizontal and vertical values treated as uniform

John Hendrikx jhendrikx at openjdk.org
Tue May 28 21:19:06 UTC 2024


On Tue, 28 May 2024 16:15:25 GMT, Michael Strauß <mstrauss at openjdk.org> wrote:

> This PR fixes the incorrect computation of the `CornerRadii.uniform` flag for the 16-argument constructor.
> 
> I've also added tests for all other constructors.

LGTM

modules/javafx.graphics/src/main/java/javafx/scene/layout/CornerRadii.java line 315:

> 313:                 bottomRightHorizontalRadiusAsPercent || bottomRightVerticalRadiusAsPercent ||
> 314:                 bottomLeftVerticalRadiusAsPercent || bottomLeftHorizontalRadiusAsPercent;
> 315:         uniform = topLeftHorizontalRadius == topLeftVerticalRadius &&

I found the diff a bit troubling to read, but from what I gather, all the radii need to be equal to qualify for uniformity, and what you did here is add a `topLeftHorizontalRadius == topLeftVerticalRadius` (and percent version) to this check.

You also already improved the readability by using `topLeftHorizontalRadius` everywhere.  It might be even better to extract this one to a new variable, so it is even more clear that you're comparing all the other radii with the same value.

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

Marked as reviewed by jhendrikx (Committer).

PR Review: https://git.openjdk.org/jfx/pull/1465#pullrequestreview-2083861912
PR Review Comment: https://git.openjdk.org/jfx/pull/1465#discussion_r1617900002


More information about the openjfx-dev mailing list