RFR: 8312603: ArrayIndexOutOfBoundsException in Marlin when scaleX is 0 [v2]

[Karthik P K]

On Thu, 25 Jan 2024 17:16:51 GMT, Laurent Bourgès <lbourges at openjdk.org> wrote:

>> Fixed scale=0 in DMarlinPrismUtils + added new test Scale0Test.java
>
> Laurent Bourgès has updated the pull request incrementally with one additional commit since the last revision:
> 
>   fixed test package

Tested the changes and it fixes the issue as expected.
Left some minor comments inline. I will finish the review soon.

modules/javafx.graphics/src/main/java/com/sun/marlin/ArrayCacheDouble.java line 168:

> 166:                 if (DO_CLEAN_DIRTY && (toIndex != 0)) {
> 167:                     // clean-up array of dirty part[fromIndex; toIndex[
> 168:                     fill(array, fromIndex, toIndex, /*(double)*/ 0.0);

Do you think /*(double)*/ will be helpful? In my opinion, it is not required.

modules/javafx.graphics/src/main/java/com/sun/marlin/ArrayCacheInt.java line 168:

> 166:                 if (DO_CLEAN_DIRTY && (toIndex != 0)) {
> 167:                     // clean-up array of dirty part[fromIndex; toIndex[
> 168:                     fill(array, fromIndex, toIndex, /*(int)*/ 0);

Same as previous comment,  /*(int)*/ comment is not required here.

modules/javafx.graphics/src/main/java/com/sun/marlin/ArrayCacheIntClean.java line 171:

> 169:                 if (toIndex != 0) {
> 170:                     // clean-up array of dirty part[fromIndex; toIndex[
> 171:                     fill(array, fromIndex, toIndex, /*(int)*/ 0);

Same as previous comment,  /*(int)*/ comment is not required here.

modules/javafx.graphics/src/main/java/com/sun/marlin/DPathConsumer2D.java line 2:

> 1: /*
> 2:  * Copyright (c) 2007, 2022, Oracle and/or its affiliates. All rights reserved.

Shouldn't it be 2007, 2024?

modules/javafx.graphics/src/main/java/com/sun/prism/impl/shape/DMarlinPrismUtils.java line 111:

> 109: 
> 110:             if (Math.abs(det) <= (2.0d * Double.MIN_VALUE)) {
> 111:                 // this rendering engine takes one dimensional curves and turns

Minor: this -> This

modules/javafx.graphics/src/main/java/com/sun/prism/impl/shape/DMarlinPrismUtils.java line 118:

> 116: 
> 117:                 // Every path needs an initial moveTo and a pathDone. If these
> 118:                 // are not there this causes a SIGSEGV in libawt.so (at the time

Minor: Add space before so and S can be made capital.
Also wanted to check if libawt usage is correct here?

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

PR Review: https://git.openjdk.org/jfx/pull/1348#pullrequestreview-1848155818
PR Review Comment: https://git.openjdk.org/jfx/pull/1348#discussion_r1469276629
PR Review Comment: https://git.openjdk.org/jfx/pull/1348#discussion_r1469277966
PR Review Comment: https://git.openjdk.org/jfx/pull/1348#discussion_r1469279357
PR Review Comment: https://git.openjdk.org/jfx/pull/1348#discussion_r1469322152
PR Review Comment: https://git.openjdk.org/jfx/pull/1348#discussion_r1469352123
PR Review Comment: https://git.openjdk.org/jfx/pull/1348#discussion_r1469351817