On Sun, 16 Jan 2022 22:54:22 GMT, Nir Lisker <nlis...@openjdk.org> wrote:
> Now that the standard concrete light types were added, there is an
> opportunity to rearrange and rewrite some of the class docs. Here is a
> summary of the changes:
>
> * Moved the explanations of attenuation and direction up to `LightBase` since
> different light types share characteristics. `LightBase` now contains a
> summary of its subtypes and all the explanations of the
> properties/characteristics of the lights divided into sections: Color, Scope,
> Direction, Attenuation.
> * Each light type links to the relevant section in `LightBase` when it
> mentioned the properties it has.
> * Added examples of real-world applications for each light type.
> * Consolidated the writing style for all the subclasses.
I'll need a second pass over this to look at the generated docs, but what I did
read looks good.
I left a few inline comments.
modules/javafx.graphics/src/main/java/javafx/scene/AmbientLight.java line 37:
> 35: * <p>
> 36: * {@code AmbientLight}s can represent strong light sources in an
> enclosed area where the lights bounces from many
> 37: * objects, causing them to be illuminated from many directions. A strong
> light in a room and moonlight are common light
> A strong light in a room ...
I think this is OK, as long as a reader doesn't think of an overhead light in a
room, which also has the properties of a point light.
modules/javafx.graphics/src/main/java/javafx/scene/DirectionalLight.java line 2:
> 1: /*
> 2: * Copyright (c) 2022, Oracle and/or its affiliates. All rights reserved.
Normally we do not change the start year, however it's OK in this case, since
it is correcting an earlier mistake.
modules/javafx.graphics/src/main/java/javafx/scene/LightBase.java line 69:
> 67: * In addition to these, each light type supports a different set of
> properties, summarized in the following table:
> 68: *
> 69: * <div style="display:flex; flex-direction: column; justify-content:
> center; align-items: center">
I think the table itself would be better if it were not centered (the columns
are fine centered).
modules/javafx.graphics/src/main/java/javafx/scene/LightBase.java line 78:
> 76: * </tr>
> 77: * <tr>
> 78: * <td>{@link AmbientLight}</td>
In order to avoid introducing accessibility issues, tables need to have column
and row headers with `scope="col"` or `scope="row"` as appropriate. See
[JDK-8184223](https://bugs.openjdk.java.net/browse/JDK-8184223) for more
details.
modules/javafx.graphics/src/main/java/javafx/scene/LightBase.java line 119:
> 117: * <li> The transparency (alpha) component of a light is ignored.
> 118: * </ol>
> 119: * There are no guarantee that these behaviors will not change.
This is a new specification of behavior rather than a doc reorganization or
clarification of an existing note. As such, I think it should be done
separately and with a CSR.
modules/javafx.graphics/src/main/java/javafx/scene/LightBase.java line 401:
> 399: if (node instanceof Shape3D) {
> 400: // Dirty using a lightweight DirtyBits.NODE_DRAWMODE bit
> 401: NodeHelper.markDirty(node, DirtyBits.NODE_DRAWMODE);
I realize that this is an unnecessary cast, but since this is a doc-only change
and we are in rampdown for jfx18, it would be best to revert this change.
modules/javafx.graphics/src/main/java/javafx/scene/SpotLight.java line 2:
> 1: /*
> 2: * Copyright (c) 2021, 2022, Oracle and/or its affiliates. All rights
> reserved.
Normally we do not change the start year, however it's OK in this case, since
it is correcting an earlier mistake.
-------------
PR: https://git.openjdk.java.net/jfx/pull/717
