On Mon, 23 Feb 2026 21:33:21 GMT, Michael Strauß <[email protected]> wrote:
>> Color.web(string, double) parses a color string by creating substrings of
>> the input. These string allocations can be removed.
>>
>> CSS numbers are parsed by `CssNumberParser`, which parses CSS-compliant
>> numbers using Lemire's algorithm.
>>
>> 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:
>
> review comment
Marked as reviewed by jhendrikx (Reviewer).
modules/javafx.graphics/src/main/java/com/sun/javafx/css/parser/CssNumberParser.java
line 182:
> 180: }
> 181:
> 182: // Use Lemire's algorithm if possible, otherwise fall back to
> Double.parseDouble()
FWIW, I think that this issue has become a bit ridiculous, but kudo's for
sticking with it. I personally think the fallback is completely unnecessary,
since it won't matter when the difference is in the 20th decimal place orso.
But I'm in favor of not reopening this further.
modules/javafx.graphics/src/main/java/com/sun/javafx/css/parser/CssNumberParser.java
line 236:
> 234: // Subnormals (lines 11-15)
> 235: if (p <= -1022) {
> 236: m >>>= -1022 - p;
Did a `p + 1` get dropped here?
modules/javafx.graphics/src/test/java/test/com/sun/javafx/css/parser/CssNumberParserTest.java
line 215:
> 213: public void roundingIsCorrectForSubnormals() {
> 214: long seed = new Random().nextLong();
> 215: System.out.println("Testing
> CssNumberParserTest.roundingIsCorrectForSubnormals with seed " + seed);
I'll object to this and the entire reasoning that led to this. JUnit tests
should be reproducable, not potentially fail arbitrarily in the run two days
after a release was done. If we're not convinced that the tests are sufficient
as is, then this certainly won't convince me either (quite the opposite in
fact). In other words, this adds zero value.
-------------
PR Review: https://git.openjdk.org/jfx/pull/2069#pullrequestreview-3850898435
PR Review Comment: https://git.openjdk.org/jfx/pull/2069#discussion_r2849890536
PR Review Comment: https://git.openjdk.org/jfx/pull/2069#discussion_r2849911136
PR Review Comment: https://git.openjdk.org/jfx/pull/2069#discussion_r2849956701
