Re: RFR: 8314147: Updated the PhongMaterial documentation [v2]
On Thu, 22 Feb 2024 22:18:38 GMT, Andy Goryachev wrote: >> Nir Lisker has updated the pull request incrementally with one additional >> commit since the last revision: >> >> Update copyright year > > modules/javafx.graphics/src/main/java/javafx/scene/paint/PhongMaterial.java > line 848: > >> 846: + ", specularPower=" + getSpecularPower() + ", >> diffuseMap=" + getDiffuseMap() + ", specularMap=" >> 847: + getSpecularMap() + ", bumpMap=" + getBumpMap() + ", >> selfIlluminationMap=" + getSelfIlluminationMap() >> 848: + "]"; > > could we restore the formatting please? > also, the first `+` is unnecessary Weird, I specifically reverted the auto-formatting, not sure how it got in. Will revert. - PR Review Comment: https://git.openjdk.org/jfx/pull/1378#discussion_r157027
Re: RFR: 8314147: Updated the PhongMaterial documentation [v2]
On Thu, 22 Feb 2024 22:19:27 GMT, Nir Lisker wrote: >> But it's still third-party content and cannot go in without approval. > > What would an approval require? To add to this, such approval would be unlikely, and I am not in favor of making the request. Perhaps you can find suitable images already in the repo? Here is one that might work: https://github.com/openjdk/jfx/tree/e0b88bc7cfede46afe28cbb4a2e333df933b5100/apps/toys/FX8-3DFeatures/src/resources - PR Review Comment: https://git.openjdk.org/jfx/pull/1378#discussion_r159761
Re: RFR: 8314147: Updated the PhongMaterial documentation [v2]
On Thu, 22 Feb 2024 22:17:27 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: > > Update copyright year modules/javafx.graphics/src/main/java/javafx/scene/paint/PhongMaterial.java line 848: > 846: + ", specularPower=" + getSpecularPower() + ", > diffuseMap=" + getDiffuseMap() + ", specularMap=" > 847: + getSpecularMap() + ", bumpMap=" + getBumpMap() + ", > selfIlluminationMap=" + getSelfIlluminationMap() > 848: + "]"; could we restore the formatting please? also, the first `+` is unnecessary - PR Review Comment: https://git.openjdk.org/jfx/pull/1378#discussion_r150870
Re: RFR: 8314147: Updated the PhongMaterial documentation [v2]
On Thu, 22 Feb 2024 22:13:33 GMT, Kevin Rushforth wrote: >> No, it's a standard in image processing: https://en.wikipedia.org/wiki/Lenna. > > But it's still third-party content and cannot go in without approval. What would an approval require? - PR Review Comment: https://git.openjdk.org/jfx/pull/1378#discussion_r152472
Re: RFR: 8314147: Updated the PhongMaterial documentation [v2]
On Thu, 22 Feb 2024 22:10:35 GMT, Nir Lisker wrote: >> modules/javafx.graphics/src/main/docs/javafx/scene/paint/doc-files/color and >> map/map.png line 1: >> >>> (failed to retrieve contents of file, check the PR for context) >> Is this a licensed image? > > No, it's a standard in image processing: https://en.wikipedia.org/wiki/Lenna. But it's still third-party content and cannot go in without approval. - PR Review Comment: https://git.openjdk.org/jfx/pull/1378#discussion_r146926
Re: RFR: 8314147: Updated the PhongMaterial documentation [v2]
On Thu, 22 Feb 2024 20:49:20 GMT, Andy Goryachev wrote: >> Nir Lisker has updated the pull request incrementally with one additional >> commit since the last revision: >> >> Update copyright year > > modules/javafx.graphics/src/main/docs/javafx/scene/paint/doc-files/color and > map/map.png line 1: > >> (failed to retrieve contents of file, check the PR for context) > Is this a licensed image? No, it's a standard in image processing: https://en.wikipedia.org/wiki/Lenna. - PR Review Comment: https://git.openjdk.org/jfx/pull/1378#discussion_r144625
Re: RFR: 8314147: Updated the PhongMaterial documentation [v2]
> 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: Update copyright year - Changes: - all: https://git.openjdk.org/jfx/pull/1378/files - new: https://git.openjdk.org/jfx/pull/1378/files/b744540c..34bbc561 Webrevs: - full: https://webrevs.openjdk.org/?repo=jfx=1378=01 - incr: https://webrevs.openjdk.org/?repo=jfx=1378=00-01 Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 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
