[jfx18] RFR: 8279345: Realign class docs of LightBase and subclasses

Kevin Rushforth kcr at openjdk.java.net
Tue Jan 25 01:20:44 UTC 2022


On Sun, 16 Jan 2022 22:54:22 GMT, Nir Lisker <nlisker at 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


More information about the openjfx-dev mailing list