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