RFR: 8377427: Reduce substring allocations in Color.web(String, double) [v4]
Michael Strauß
mstrauss at openjdk.org
Sat Feb 14 12:11:42 UTC 2026
On Sat, 14 Feb 2026 00:33:13 GMT, John Hendrikx <jhendrikx at openjdk.org> wrote:
>> Michael Strauß has updated the pull request incrementally with one additional commit since the last revision:
>>
>> Review comments
>
> modules/javafx.graphics/src/main/java/javafx/scene/paint/Color.java line 404:
>
>> 402: int offset = 0;
>> 403:
>> 404: if (colorString.regionMatches(true, 0, "#", 0, 1)) {
>
> You sure that's better than `colorString.charAt(0) == '#'` ? Same for the other ones that don't need case insensitive matching.
Good point. I've replaced it with `charAt()`.
> modules/javafx.graphics/src/main/java/javafx/scene/paint/Color.java line 447:
>
>> 445: int b = Integer.parseInt(colorString, offset + 2, offset + 3, 16);
>> 446: int a = Integer.parseInt(colorString, offset + 3, offset + 4, 16);
>> 447: yield Color.color(r / 15.0, g / 15.0, b / 15.0, opacity * a / 15.0);
>
> I suppose that works, could also consider `(r << 4) + r` and call `Color.rgb`; it may not matter if things are stored internally as doubles.
Doesn't really matter as the values are stored as doubles anyway.
> modules/javafx.graphics/src/test/java/test/com/sun/javafx/css/parser/CssNumberParserTest.java line 52:
>
>> 50: assertEquals(-0.5, parseDouble("-.5"), 0.0);
>> 51: assertEquals(12.34, parseDouble("12.34"), 0.0);
>> 52: assertEquals(0.00123, parseDouble("0.00123"), 1e-18);
>
> It needs the 1e-18 here?
Not anymore.
-------------
PR Review Comment: https://git.openjdk.org/jfx/pull/2069#discussion_r2807430138
PR Review Comment: https://git.openjdk.org/jfx/pull/2069#discussion_r2807430376
PR Review Comment: https://git.openjdk.org/jfx/pull/2069#discussion_r2807430475
More information about the openjfx-dev
mailing list