[x3d-public] X3D4 specification review, lighting component

[Michalis Kamburelis]

Hi!
Answers inline below.

wt., 17 lis 2020 o 19:21 Don Brutzman <brutzman at nps.edu> napisał(a):
>
> 1. Intensity range
>
> *Wondering* shouldn't we describe what lighting intensity greater than 1 means?
>
> I had thought that values greater than 1 might be related to High Dynamic Range (HDR) lighting but apparently that is only bit depth, not value range:
>

There is no special meaning for values > 1, actually.

So I don't think we can give here any explanation :) Values > 1 are
just normal values, just like <= 1, they go through lighting equations
in the same way.

While of course now we can get larger resulting colors, but it's not a
special change. Lighting equations in X3D 3 (with intensity limited to
1.0) already allowed the resulting colors to have RGB components
arbitrarily larger than 1.0 (by having multiple lights).

So there is nothing special about intensity <= 1.0 compared to
intensity > 1.0. It works the same way, it means the same thing in
physics.

And 1.0 limit never had any good reason, as I see it. The value of
intensity = 1.0 was in no way special / extreme, in X3D 3 or 4.

It is only loosely related to HDR. "Allowing light intensity > 1"
implies we can be more correct (with respect to physics). Which is
important for HDR, but also without HDR. HDR means that you need to
store the resulting color with greater range than just 0...255, and
then (in case of computer graphics) convert it back to the range that
monitors can display. Indeed you can say HDR is about "larger bit
depth". Having light intensity > 1.0 doesn't mean that we "have HDR",
but it means that "we can have proper physics values in output", which
means that "HDR makes sense".

>
> 2. Terms of reference
>
[snip]

My prose around luminance etc. was adjusted from
https://github.com/KhronosGroup/glTF/tree/master/extensions/2.0/Khronos/KHR_lights_punctual
.

The explanation from Wikipedia for these terms seems sensible.

>
> ---
>
> 3. Missing figure?
>
> 17.2.2.3 Common definitions for all lighting models
>
> includes following definitions, shouldn't there be a corresponding figure:
>
> =================
> - N = normalized normal vector at this point on geometry. This vector is interpolated from vertex normals specified in a node derived from X3DNormalNode or calculated by the browser. It is modified by the normalTexture providing normals in the tangent space (see X3DOneSidedMaterialNode definition).
>
> - V = normalized vector from point on geometry to viewer's position.
> =================
>

I agree it would be nice to have here some figure to illustrate.
Actually, the whole section "17.2.2 Lighting model" could use a lot of
figures :)

That said, note that it was like that in X3D 3 already. These terms
(N, V) were already present in X3D 3, without a figure.

glTF spec also doesn't need such figure. These are standard terms in
computer graphics.

So.. I agree it would be nice to have a figure. But I don't think it's
strictly necessary :) As for me, right now I do not have the capacity
to prepare such graphics.

> ---
>
> 4. Prose review.
>
> In section 17.2.2.3 Common definitions for all lighting models
> we plan to make several refinements:
>
> - Remove underscore styling for types,

Up to you. I found that undescoring them improved readability,
otherwise type names / function names / parameters are all written
with the same style (just italics) and some pieces are harder to read.

> - Use regular X3D types instead of generic, e.g. SFFloat instead of float, SFVec3F instead of vector3, etc.

Up to you. Indeed, the types SFFloat / SFVec2f / SFVec3f / SFVec4f may
be more instantly recognizable for X3D authors, I agree.

Note that these functions are in practice not done using X3D fields or
types. They are implemented in shaders, with whatever precision the
default shaders have. E.g. "vector3" is neither exactly SFVec3f nor is
it SFVec3d. That said, I agree that writing "SFVec3f" may be more
obvious in context of X3D.

> - Any asterisks should be multiplication symbols

Should be like that already :) Unless I made a mistake somewhere :)
Yes, any * should be multiplication.

> - Consistently referring to "lighting model" or "shading model"

This should already be right.

Warning: Please don't mix these terms. That part of the specification
*does* talk about lighting model, and it also talks about shading
model, and they are *different* things. We had a talk about it, please
read my https://github.com/michaliskambi/x3d-tests/wiki/Do-not-confuse-Phong-shading-with-Phong-lighting-model
.

The current prose should already use these terms correctly (unless I
made a stupid mistake somewhere, of course :) ).

> - Avoid "we" and stay in 3rd person

Sure.

>
> For consistency within spec, change occurrences such as "Material.diffuseTexture" to "Material node /diffuseTexture/ field"

Up to you. I think it will result in some overly-verbose prose
sometimes, but if you feel it is necessary, OK.

Regards,
Michalis