On Fri, 16 Jan 2026 21:58:49 GMT, Andy Goryachev <[email protected]> wrote:
>> Christopher Schnick has updated the pull request incrementally with three
>> additional commits since the last revision:
>>
>> - Remove space
>> - Use for loops again
>> - Remove unused method
>
> modules/javafx.graphics/src/main/java/com/sun/javafx/sg/prism/NGGroup.java
> line 231:
>
>> 229: return;
>> 230: }
>> 231: for (int i = 0; i < orderedChildren.size(); i++) {
>
> I don't know why we need to change for forEach loop, at the expense of extra
> memory allocation. the old code is perfectly fine.
Reverted
> modules/javafx.graphics/src/main/java/com/sun/javafx/sg/prism/NGGroup.java
> line 287:
>
>> 285: } while (bot == null || !idValid);
>> 286:
>> 287: bot.unref();
>
> I am not sure of this, I'd suggest to revert.
>
> This is a kind of change that we should not be making:
> - it's very time consuming to code review
> - if developer of reviewers collectively made a mistake and missed a case, it
> causes regression.
I agree, but it's not that complicated. There is a while loop that precedes
this which will not exit until bot != null and it does not contain break
statements.
> modules/javafx.graphics/src/main/java/com/sun/javafx/sg/prism/NGGroup.java
> line 297:
>
>> 295: }
>> 296: List<NGNode> orderedChildren = getOrderedChildren();
>> 297: int n = orderedChildren.size();
>
> please remove extra space
Fixed
> modules/javafx.graphics/src/main/java/com/sun/javafx/sg/prism/NGGroup.java
> line 452:
>
>> 450: List<NGNode> orderedChildren = getOrderedChildren();
>> 451: for (NGNode orderedChild : orderedChildren) {
>> 452: child = orderedChild;
>
> completely unnecessary: you've added unnecessary memory allocation (and
> iterator).
>
> please revert.
Reverted
> modules/javafx.graphics/src/main/java/com/sun/javafx/sg/prism/NGGroup.java
> line 473:
>
>> 471: clone = clone.deriveWithConcatenation(getTransform());
>> 472: List<NGNode> orderedChildren = getOrderedChildren();
>> 473: for (final NGNode child : orderedChildren) {
>
> please revert, for the same reason (unnecessary, extra work for computer and
> people alike)
Reverted
-------------
PR Review Comment: https://git.openjdk.org/jfx/pull/2043#discussion_r2700166541
PR Review Comment: https://git.openjdk.org/jfx/pull/2043#discussion_r2700169240
PR Review Comment: https://git.openjdk.org/jfx/pull/2043#discussion_r2700165377
PR Review Comment: https://git.openjdk.org/jfx/pull/2043#discussion_r2700164594
PR Review Comment: https://git.openjdk.org/jfx/pull/2043#discussion_r2700164268
