On Wed, 29 Jul 2020 23:11:20 GMT, Nir Lisker <nlis...@openjdk.org> wrote:
>> High-level comments: >> >> 1. The API docs look good. If you change the public API to `@since 16` then >> you can also update the CSR and move it to >> the "Submitted" state. 2. I think it would be good to cleanup the >> performance test and make it part of this PR, maybe >> in `tests/performance` (which currently only has the not-very-useful >> `VMPerformance` subdir) or `tests/manual`. 3. We >> need some functional tests, ideally automated ones in `tests/system` >> I also left a few inline comments. I haven't reviewed the shaders yet, so I >> do that, in addition to further testing, >> next week. > >> 1. The API docs look good. If you change the public API to `@since 16` then >> you can also update the CSR and move it to >> the "Submitted" state. > > I moved it to the PROPOSED state. > >> 2. I think it would be good to cleanup the performance test and make it part >> of this PR, maybe in `tests/performance` >> (which currently only has the not-very-useful `VMPerformance` subdir) or >> `tests/manual`. > > I added it under `tests/performance`. > >> 3. We need some functional tests, ideally automated ones in `tests/system` > > What should this test do? Which example should I follow? > > By the way, The `tests` project is broken in Eclipse because several source > folders, like `tests\system\src\testapp6`, > define their own `module-info.java`, and it's illegal in Eclipse to have > several of these in a project. To have the > tests there compile, the Eclipse files will need to be updated. Given that we don't have any automated tests for lighting (we have a couple that verify that we can take a snapshot and get the same result, but that isn't testing the lighting itself), probably the most we can expect is a simple test of a large quad with a light fairly close to the object, such that the difference with / without attenuation is noticeable. Then you could sample the center and near the corners of the object for both the attenuated and unattenuated cases. ------------- PR: https://git.openjdk.java.net/jfx/pull/43
