RFR: 8255015: Inconsistent illumination of 3D shape by PointLight [v11]

[Kevin Rushforth]

On Sun, 24 Oct 2021 17:23:40 GMT, Andreas Heger <duke at openjdk.java.net> wrote:

>> The inconsistent illumination happens on Macs with retina displays only if the 3D shape is placed in a SubScene. The light sources are located with wrong coordinates in sub scenes and this causes a different illumination. The wrong coordinates for the light sources come from the fact that the retina pixel scale factors are not used in a SubScene.
>> 
>> With this pull request, the retina pixel scale factors will be also used in SubScenes and this should resolve the bug [https://bugs.openjdk.java.net/browse/JDK-8255015](url)
>
> Andreas Heger has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains 10 additional commits since the last revision:
> 
>  - Merge branch 'openjdk:master' into fix-8255015
>  - 8255015: Comments corrected
>  - 8255015: Comment about copying pixel scale factors corrected
>  - 8255015: Tabs removed from PointLightIllumination.java
>  - Merge branch 'openjdk:master' into fix-8255015
>  - 8255015: JUnit Test class added.
>  - Merge branch 'openjdk:master' into fix-8255015
>  - Merge branch 'openjdk:master' into fix-8255015
>  - Merge branch 'openjdk:master' into fix-8255015
>  - 8255015: Copy pixel scale factors from graphics object to subscene graphics so that the position of lights will be scaled correctly on retina displays

The fix and the test looks good. I left a couple minor comments on the test.

I ran the test on macOS with retina. I'll double-check on the other platforms, but I think this is about ready to go.

tests/system/src/test/java/test/robot/test3d/PointLightIlluminationTest.java line 67:

> 65:     private static final int    LOWER_CORNER_Y     = (int) (SCENE_WIDTH_HEIGHT * (1 - CORNER_FACTOR));
> 66:     private static final double COLOR_TOLERANCE    = 0.07;
> 67:     private static Scene testScene;

This is created on one thread and tested on another (to see whether it's already been created), so I recommend making it `volatile` (i.e., `private static volatile ...`). Also, you might want to explicitly set it to `null` since you rely on it (yes, I know `null` is the default).

tests/system/src/test/java/test/robot/test3d/PointLightIlluminationTest.java line 180:

> 178:         return new Scene(new Group(subScene), SCENE_WIDTH_HEIGHT, SCENE_WIDTH_HEIGHT);
> 179:     }
> 180: }

Minor: there should be a new line at the end of the file.

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

PR: https://git.openjdk.java.net/jfx/pull/531