Re: RFR: 8314147: Updated the PhongMaterial documentation [v7]
On Wed, 28 Feb 2024 16:01:15 GMT, Nir Lisker wrote: > > One other thing I noticed is that the file names have spaces in them, which > > is not a best practice and causes problems for scripts. Can you change all > > spaces to _ in the names of the newly-added files? > > Yes. What about folders? Also, do you prefer camelCase or the `_` naming? Generally we use all lower case for directory names, but as long as it isn't used as a package name, it isn't as important. - PR Comment: https://git.openjdk.org/jfx/pull/1378#issuecomment-1969357960
Re: RFR: 8314147: Updated the PhongMaterial documentation [v7]
On Wed, 28 Feb 2024 14:21:55 GMT, Kevin Rushforth wrote: > One other thing I noticed is that the file names have spaces in them, which > is not a best practice and causes problems for scripts. Can you change all > spaces to _ in the names of the newly-added files? Yes. What about folders? Also, do you prefer camelCase or the `_` naming? - PR Comment: https://git.openjdk.org/jfx/pull/1378#issuecomment-1969301404
Re: RFR: 8314147: Updated the PhongMaterial documentation [v7]
On Wed, 28 Feb 2024 13:50:38 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 images Btw, if this ends up missing the deadline for JavaFX 22 (about 24 hours from now) it can go into 22u for JavaFX 22.0.1. The release date for 22.0.1 is only a few weeks after 22, and the deadline to get fixes into 22.0.1 is almost 2 weeks away. - PR Comment: https://git.openjdk.org/jfx/pull/1378#issuecomment-1969223783
Re: RFR: 8314147: Updated the PhongMaterial documentation [v7]
On Wed, 28 Feb 2024 13:50:38 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 images I looked at the images in the javadoc-generated web page, and the rendered image size looks good. I recommend scaling most of the images down by at least a factor of 4 (at least a factor of 2 in both width and height), since they are much higher resolution than needed for the size they are rendered at in the page. I see that the animated gif for the writeable image is 6 Mbytes, and could probably be scaled down even more without losing quality. - PR Comment: https://git.openjdk.org/jfx/pull/1378#issuecomment-1969143245
Re: RFR: 8314147: Updated the PhongMaterial documentation [v7]
On Wed, 28 Feb 2024 13:50:38 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 images Thank you for generating new images. Some of the new files are quite large, and the total added storage is 35 Mbytes, which is a lot for binary image files stored in the repo. I haven't generated the docs and looked at them yet (doing that now), but it might be better to use lower resolution images. One other thing I noticed is that the file names have spaces in them, which is not a best practice and causes problems for scripts. Can you change all spaces to `_` in the names of the newly-added files? - PR Comment: https://git.openjdk.org/jfx/pull/1378#issuecomment-1969088841
Re: RFR: 8314147: Updated the PhongMaterial documentation [v7]
On Wed, 28 Feb 2024 13:50:38 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 images I generated the new background is an engine. I also enlarged the images a bit and added another one in the transparency section with not highlight for comparison. - PR Comment: https://git.openjdk.org/jfx/pull/1378#issuecomment-1969032540
Re: RFR: 8314147: Updated the PhongMaterial documentation [v7]
> 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 images - Changes: - all: https://git.openjdk.org/jfx/pull/1378/files - new: https://git.openjdk.org/jfx/pull/1378/files/60d45bc8..2989624d Webrevs: - full: https://webrevs.openjdk.org/?repo=jfx&pr=1378&range=06 - incr: https://webrevs.openjdk.org/?repo=jfx&pr=1378&range=05-06 Stats: 28 lines in 20 files changed: 4 ins; 1 del; 23 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
