On Mon, 15 Jan 2024 21:25:57 GMT, Michael Strauß <mstra...@openjdk.org> wrote:
>> Lukasz Kostyra has updated the pull request incrementally with one >> additional commit since the last revision: >> >> doSnapshot: Replace light accumulation code >> >> Uses suggested simpler implementation using Java's Streams. >> >> Needed an additional check in NGShape3D, otherwise IndexOutOfBounds was >> thrown > > modules/javafx.graphics/src/main/java/javafx/scene/Scene.java line 1364: > >> 1362: // Grab the lights from the scene and/or subscene >> 1363: context.lights = null; >> 1364: int totalLightCount = 0; > > Here's a suggestion: you might be able to replace all of the new code, > including the `accumulateLightsForSnapshot` method, with the following > shorter code: > > > Stream<NGLightBase> lights = Stream.concat( > Optional.ofNullable(scene).stream().flatMap(s -> > s.lights.stream()).map(LightBase::getPeer), > Optional.ofNullable(subScene).stream().flatMap(s -> > s.getLights().stream()).map(LightBase::getPeer)); > > context.lights = lights.toArray(NGLightBase[]::new); I changed the code as suggested. After that Snapshot3DTest code started showing some failures, this was due to NGShape3D code not checking if it gets an empty (length == 0) array and throwing `IndexOutOfBoundsException`. I patched that as well and didn't see any regressions in tests afterwards. ------------- PR Review Comment: https://git.openjdk.org/jfx/pull/1332#discussion_r1457610923
