RFR: 8377427: Reduce substring allocations in Color.web(String, double) [v2]

[Andy Goryachev]

On Tue, 10 Feb 2026 15:09:55 GMT, Michael Strauß <mstrauss at openjdk.org> wrote:

>> Color.web(string, double) parses a color string by creating substrings of the input. These string allocations can be removed.
>> 
>> There are no new tests for the `Color` class, since the existing tests already cover all relevant code paths.
>
> Michael Strauß has updated the pull request incrementally with one additional commit since the last revision:
> 
>   CSS-compliant double parser

The JavaFX CSS spec states (under <number>):


Real numbers and integers are specified in decimal notation only. ... A <number> can either be an <integer>, or it can be zero or more digits followed by a dot (.) followed by one or more digits. Both integers and real numbers may be preceded by a "-" or "+" to indicate the sign. -0 is equivalent to 0 and is not a negative number.

[+|-]? [[0-9]+|[0-9]*"."[0-9]+]


it looks to me like exponential notation should not be supported.

Mandatory disclaimer: W3C is not a real standard.

I am not sure if we want to change the CSS spec to allow scientific notation in all the <number>s, I don't even sure we want to add this support in `Color.web()`.  If we do though, I think we would need to update the javadoc and possibly the CSS Reference, right?

I must admit I don't know how much changes in `Color.web()` impact CSS parsing.
For example, it looks like `Color.web()` is being called from `CssParser.colorValueOfString(String)` but a simple css like `-fx-background-color: rgb(1);` won't trigger that code.

it's not a real standard in a sense that it is an attempt to write a spec for something that every browser manufacturer want to change to make their browser "better".  maybe the situation changed recently, I don't know.

But I do agree with you - the majority of developers are familiar with it, so it might make sense to adapt in some cases (while keeping in mind possible regressions).

> JavaFX contradicts its own specification, but the specification is (and has always been) wrong anyway.

This is hilarious and concerning at the same time.  I wonder whether we need to update the javadoc / create a CSR since we are changing the behavior.  What do you think?

modules/javafx.graphics/src/main/java/javafx/scene/paint/Color.java line 437:

> 435: 
> 436:             if (len == 3) {
> 437:                 r = Integer.parseInt(colorString, offset, offset + 1, 16);

would a switch statement be better in this case (e.g. case when len == 1)?

modules/javafx.graphics/src/test/java/test/com/sun/javafx/css/parser/CssNumberParserTest.java line 40:

> 38:     @Test
> 39:     public void parseIntegerAndSignedInteger() {
> 40:         assertEquals(0.0, parseDouble("0"), 0.0);

The JavaFX CSS spec also states


-0 is equivalent to 0 and is not a negative number.


How should we test this?

modules/javafx.graphics/src/test/java/test/com/sun/javafx/css/parser/CssNumberParserTest.java line 79:

> 77:         assertNumberFormatException("1 ");
> 78:         assertNumberFormatException("xx1e2yy", 2, 6); // slice is "1e2y"
> 79:     }

maybe add another test for malformed numbers such as `++0`, `1e--2`, etc.?

modules/javafx.graphics/src/test/java/test/com/sun/javafx/css/parser/CssNumberParserTest.java line 270:

> 268:         return rawBits >= 0 ? rawBits : (0x8000_0000_0000_0000L - rawBits);
> 269:     }
> 270: }

missing newline

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

PR Review: https://git.openjdk.org/jfx/pull/2069#pullrequestreview-3780071461
PR Comment: https://git.openjdk.org/jfx/pull/2069#issuecomment-3880094916
PR Comment: https://git.openjdk.org/jfx/pull/2069#issuecomment-3880191468
PR Comment: https://git.openjdk.org/jfx/pull/2069#issuecomment-3880669134
PR Review Comment: https://git.openjdk.org/jfx/pull/2069#discussion_r2789907975
PR Review Comment: https://git.openjdk.org/jfx/pull/2069#discussion_r2789129871
PR Review Comment: https://git.openjdk.org/jfx/pull/2069#discussion_r2789098613
PR Review Comment: https://git.openjdk.org/jfx/pull/2069#discussion_r2788929245