Re: RFR: 8314147: Updated the PhongMaterial documentation [v5]

2024-02-26 Thread Lukasz Kostyra
On Sat, 24 Feb 2024 14:40:25 GMT, Nir Lisker  wrote:

>> Overhaul to the `PhongMaterial` documentation (and a bit to its superclass 
>> `Material`). Except for the introduction, I divided the documentation into 3 
>> sections: qualitative explanation, mathematical model (I wouldn't think it 
>> necessary, but the current doc explains it), and examples.
>> 
>> The reason for the verbosity of the doc is that I envisioned 2 target 
>> audiences for this class. One is a Java developer who wants to understand 
>> the terminology and workings of computer graphics or of the artists who are 
>> already familiar with this domain. (How many Java developers know what 
>> diffuse, specular and normal maps are?) The other is an artist who is 
>> already familiar with the domain, but wants to see how this class compares 
>> with other renderers. For this reason, I looked at the terminology used by 
>> engines like Blender, Maya, UE4 and Unity and tried to mention the 
>> comparisons (like bump vs. height vs. normal maps, or specular vs. 
>> roughness/smoothness).
>> 
>> The examples I chose and some of the schematics are not the best, looking at 
>> it retroactively, but I want to give enough time for reviewers and get this 
>> into 22.
>
> Nir Lisker has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Remove redundant image copies

This is a great docs update and exhaustive explanation of how Phong model 
works. I found one typo to iron out, otherwise LGTM (I will do a second pass 
once backgrounds are updated).

modules/javafx.graphics/src/main/java/javafx/scene/paint/PhongMaterial.java 
line 227:

> 225:  * the total specular component contribution is S * 
> CSM.
> 226:  *
> 227:  * Slef-Illumination

Typo - `s/Slef/Self`

-

Changes requested by lkostyra (Committer).

PR Review: https://git.openjdk.org/jfx/pull/1378#pullrequestreview-1901233188
PR Review Comment: https://git.openjdk.org/jfx/pull/1378#discussion_r1502832050


Re: RFR: 8314147: Updated the PhongMaterial documentation [v5]

2024-02-24 Thread Nir Lisker
> Overhaul to the `PhongMaterial` documentation (and a bit to its superclass 
> `Material`). Except for the introduction, I divided the documentation into 3 
> sections: qualitative explanation, mathematical model (I wouldn't think it 
> necessary, but the current doc explains it), and examples.
> 
> The reason for the verbosity of the doc is that I envisioned 2 target 
> audiences for this class. One is a Java developer who wants to understand the 
> terminology and workings of computer graphics or of the artists who are 
> already familiar with this domain. (How many Java developers know what 
> diffuse, specular and normal maps are?) The other is an artist who is already 
> familiar with the domain, but wants to see how this class compares with other 
> renderers. For this reason, I looked at the terminology used by engines like 
> Blender, Maya, UE4 and Unity and tried to mention the comparisons (like bump 
> vs. height vs. normal maps, or specular vs. roughness/smoothness).
> 
> The examples I chose and some of the schematics are not the best, looking at 
> it retroactively, but I want to give enough time for reviewers and get this 
> into 22.

Nir Lisker has updated the pull request incrementally with one additional 
commit since the last revision:

  Remove redundant image copies

-

Changes:
  - all: https://git.openjdk.org/jfx/pull/1378/files
  - new: https://git.openjdk.org/jfx/pull/1378/files/e367c882..83bf2eb1

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jfx&pr=1378&range=04
 - incr: https://webrevs.openjdk.org/?repo=jfx&pr=1378&range=03-04

  Stats: 65 lines in 3 files changed: 0 ins; 65 del; 0 mod
  Patch: https://git.openjdk.org/jfx/pull/1378.diff
  Fetch: git fetch https://git.openjdk.org/jfx.git pull/1378/head:pull/1378

PR: https://git.openjdk.org/jfx/pull/1378