On Sun, 24 Oct 2021 17:23:40 GMT, Andreas Heger <d...@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
