[x3d-public] Various comments reading X3Dv4 current working draft (WD3 in GitHub repository)
Michalis Kamburelis
michalis.kambi at gmail.com
Sat Dec 5 16:33:00 PST 2020
Done!
I made the modifications listed in this thread (see points 3,6,8,9 above).
I also fixed a new error I found in "17.5 Support levels". Someone
wrote there "Support for Physical and Unlit shading." -- which is not
correct. As we discussed in an earlier thread, and I outlined in
https://github.com/michaliskambi/x3d-tests/wiki/Do-not-confuse-Phong-shading-with-Phong-lighting-model
, there's no such thing as "physical shading" or "unlit shading". This
is confusing "shading" with "lighting model". So I simply changed it
to "Support for Physical and Unlit lighting models.".
Checked with "ant" that I didn't introduce any new errors/warnings.
My commits are on
https://github.com/Web3DConsortium/X3D/commit/4021f559e6498cc7807700dc1aef416bea01000f
so you can see exactly what I changed.
Regards,
Michalis
niedz., 6 gru 2020 o 00:29 Michalis Kamburelis
<michalis.kambi at gmail.com> napisał(a):
>
> I'm sitting down to do the changes outlined in this thread now :) So I
> will make a commit+push soon, will let you know (this should not take
> me more than ~1 hour).
>
> Regards,
> Michalis
>
> pt., 4 gru 2020 o 02:12 Don Brutzman <brutzman at nps.edu> napisał(a):
> >
> > All good. Have corrected subject line to WD3.
> >
> > Please make the changes you think are appropriate for all of these points.
> >
> > Recommend "simple is good" regarding glTF level of support. If it is not changing component/level requirements, if it avoids need for minor changes over the next year, that is fine. Active implementations always have gaps (and steady increases) in capabilities.
> >
> > Lighting shading textures rendering - all yours, again. 8)
> >
> > On 12/3/2020 1:20 PM, Michalis Kamburelis wrote:
> > >
> > > Thank you Don! Comments (only to 3,6,8,9) below:
> > >
> > >>> 3. In "17.2.2.6 Physical lighting model"
> > >>>
> > >>> I see you removed the last paragraph, that refers to glTF lighting
> > >>> equations and was saying "Future revisions of this draft will contain
> > >>> the final recommended equations." Of course that is OK, we don't want
> > >>> to leave a text saying "Future revisions of this draft...." in a final
> > >>> specification :)
> > >>>
> > >>> And it is my fault, I promised but completely failed to create a
> > >>> correct prose for this text with equations. But we need to say
> > >>> *something* about what should be done. I propose this text:
> > >>>
> > >>> """Using the <i>baseParameter</i>, <i>metallicParameter</i> and
> > >>> <i>roughnessParameter</i>, the physical lighting model calculates the
> > >>> <i>physicalLightContribution<sub>i</sub></i> value. The calculation
> > >>> should follow the standard Physically Based Rendering equations, and
> > >>> be consistent with the glTF 2.0 lighting model.
> > >>> """
> > >>
> > >> Rephrased:
> > >>
> > >> "The physical lighting model uses the baseParameter, metallicParameter and roughnessParameter to calculate the physicalLightContribution_i value in accordance with the physically based rendering equations as specified in 2.[GLTF]. The ambientIntensity value is unused."
> > >>
> > >> Edited second sentence, placed it later under physical lighting mode, as
> > >>
> > >> "The physical lighting equations are specified in 2.[GLTF]."
> > >
> > > Your rephrasing is perfectly OK, thank you.
> > >
> > > However, you added it to "17.2.1.1 Overview" instead of at the end of
> > > "17.2.2.6 Physical lighting model". This makes the paragraph
> > > confusing, as it talks about baseParameter, metallicParameter
> > > roughnessParameter, physicalLightContribution_i --- all of them are
> > > only defined in "17.2.2.6 Physical lighting model".
> > >
> > > I would suggest to move this text to "17.2.2.6 Physical lighting
> > > model", at the end.
> > >
> > >>> 6. The lighting equations have not been updated to reflect shadows. I
> > >>> provided a prose for it, on 2020-10-20, repasting it below:
> > >>>
> > >>> """
> > >>> shadowTest_i scales down the light contribution depending on shadow:
> > >>>
> > >>> - shadowTest_i is 1.0 if the light source i has "shadow" = FALSE
> > >>>
> > >>> - otherwise, shadowTest_i is 1.0 if the light source i has
> > >>> "shadow" = TRUE but nothing obscures the light, i.e. there is no Shape
> > >>> (that has visible and castShadow = TRUE) between the light source
> > >>> position and the given point. The DirectionalLight is treated like a
> > >>> "point in infinity", which means that shadow rays are all parallel to
> > >>> the light direction.
> > >>>
> > >>> - otherwise, shadowTest_i is "1.0 - shadowIntensity". This occurs
> > >>> when "shadow" = TRUE and the light is obscured. For example,
> > >>> "shadowIntensity" equal 1.0 (the default) means that light
> > >>> contribution drops to zero when the light is obscured by the shadow
> > >>> caster.
> > >>> """
> > >>>
> > >>> And replace the "on_i" with "on_i * shadowTest_i" in above
> > >>> equations, to use shadowTest_i to scale down the light contribution.
> > >>
> > >> Our changes above are checked in. Please make these detailed equation-related changes yourself and commit.
> > >
> > > OK, I'll do it (late Saturday, most likely, is when I'll find the
> > > earliest possible time).
> > >
> > > Please don't wait for me -- whatever you do in the meantime, I'll
> > > manage to resolve and commit my changes.
> > >
> > >>> 8. "9.5 Support levels" (in Networking component)
> > >>>
> > >>> About the text mentioning glTF at the end, "Support for .gltf
> > >>> (model/gltf+json) and .bin (application/octet-stream). Requires
> > >>> support for Shape component level 2 and Lighting component level 4."
> > >>>
> > >>> I see you didn't apply there my notes from 2020-08-09 (thread "Re:
> > >>> X3D minutes16 July 2020: Inline refresh, https, description;
> > >>> X3DUrlObject: glTF support") :) The text most likely doesn't say what
> > >>> you want, because X3D specification isn't really concerned about .bin
> > >>> files (they are not referenced from X3D, they are only referenced from
> > >>> glTF), and the text doesn't mention .glb (and it probably should). I
> > >>> propose to change it to this:
> > >>>
> > >>> """
> > >>> Support for glTF models in Inline nodes, with .gltf
> > >>> (model/gltf+json) and .glb (model/gltf-binary) formats.
> > >>>
> > >>> Minimum required glTF support:
> > >>> - glTF transformation hierarchy,
> > >>> - glTF meshes,
> > >>> - glTF standard physical materials (correspond to X3D
> > >>> PhysicalMaterial nodes, requires "Shape" component level 2 support)
> > >>> - glTF unlit (KHR_materials_unlit) materials (correspond to X3D
> > >>> UnlitMaterial nodes)
> > >>> - loading of external binary data referenced from .gltf files
> > >>> (e.g. for vertex coordinates)
> > >>> """
> > >>
> > >> Wondering, what is left out from your list?
> > >>
> > >> We don't have to exclude glTF extensions, and we don't have to omit things that implementers might optionally include.
> > >>
> > >> Am thinking that, if subsetting is actually needed, this goes better in the appropriate support table. Seems unnecessary.
> > >
> > > This lists the "Minimum required glTF support:", so it's not about
> > > excluding, but including :)
> > >
> > > That is, I want to specify a *required subset of glTF* for an X3D
> > > browser to claim "I support X3D Networking component at level 4"
> > > (which means, glTF support).
> > >
> > > The big things missing (that is, *not* required from X3D browser, with
> > > my proposed text) are
> > >
> > > - skinned animation in glTF
> > > - morph target animation in glTF
> > >
> > > I can go in details, but basically we're still figuring out (at
> > > implementations, like CGE or X3DOM) how to express these things in X3D
> > > without losing any information or efficiency. While CGE supports glTF
> > > skinned animation, my solution is not final, I'd like to use some
> > > standard X3D nodes for that in a standard way -- but no implementation
> > > has yet figured out how to do it perfectly (see my
> > > https://github.com/michaliskambi/x3d-tests/wiki/Converting-glTF-to-X3D
> > > ).
> > >
> > > There is also lots of small things, for which CGE and/or X3DOM have
> > > extensions, but they are not yet in X3D standard (alphaMode,
> > > alphaCutoff, per-vertex colors with multiplication, inverting vertical
> > > texture coords orientation, details about punctual lights etc.).
> > >
> > > So we're not yet at level "you can express 100% of glTF using standard
> > > X3D 4.0 nodes". We got closer to it with PBR, but there's still work
> > > ahead :)
> > >
> > >>
> > >>> 9. " 12.2.4 Texture mapping specified in material nodes"
> > >>>
> > >>> I promised to provide a prose making the soft-fail (I believe
> > >>> both Richard and Andreas said it's a good idea), to specify what
> > >>> happens if given "xxxTextureMapping" is not existing or existing
> > >>> multiple times in the relevant list. Here's are 2 corrections, please
> > >>> apply:
> > >>>
> > >>> In "12.2.4.1 Texture coordinates" :
> > >>>
> > >>> """If the xxxTextureMapping field is not empty, it must refer to
> > >>> a corresponding X3DSingleTextureCoordinateNode node on a list of
> > >>> texture coordinates."""
> > >>>
> > >>> ->
> > >>>
> > >>> """If the xxxTextureMapping field is not empty, it refers to a
> > >>> corresponding X3DSingleTextureCoordinateNode node on a list of texture
> > >>> coordinates. If no corresponding X3DSingleTextureCoordinateNode is
> > >>> found, the browser should determine texture coordinates as if
> > >>> xxxTextureMapping was empty (see below). If multiple
> > >>> X3DSingleTextureCoordinateNode nodes with the same "mapping" value are
> > >>> present, the first one is chosen."""
> > >>>
> > >>> In " 12.2.4.2 Texture coordinates transformation" :
> > >>>
> > >>> """If the xxxTextureMapping field is not empty, it must refer to
> > >>> a corresponding X3DSingleTextureTransformNode node within the list of
> > >>> texture transformations. The X3DSingleTextureTransformNode node must
> > >>> have equal mapping value."""
> > >>>
> > >>> ->
> > >>>
> > >>> """If the xxxTextureMapping field is not empty, it refers to a
> > >>> corresponding X3DSingleTextureTransformNode node within the list of
> > >>> texture transformations. The X3DSingleTextureTransformNode node must
> > >>> have equal mapping value. If no corresponding
> > >>> X3DSingleTextureTransformNode is found, the browser should determine
> > >>> texture transformation as if xxxTextureMapping was empty (see below).
> > >>> If multiple X3DSingleTextureTransformNode nodes with the same
> > >>> "mapping" value are present, the first one is chosen."""
> > >>
> > >> Please make the changes you think best and then we will review.
> > >>
> > >> We are still thinking about a better way to express xxxTexture and xxxTextureMapping as a way to refer to similarly named fields. Seems cheesy to me...
> > >>
> > >
> > > OK. I admit I don't like the"xxxTextureMapping" / "xxxTexture" phrases
> > > myself, but it was the best I could come up with that is simple and
> > > unambiguous to the reader :)
> > >
> > > In the meantime I would suggest to apply my functional change above,
> > > that introduces the "soft-fail" mechanism that Richard and Andreas
> > > requested. Let's have at least functionality perfect :)
> > >
> > > Regards,
> > > Michalis
> > >
> >
> > all the best, Don
> > --
> > Don Brutzman Naval Postgraduate School, Code USW/Br brutzman at nps.edu
> > Watkins 270, MOVES Institute, Monterey CA 93943-5000 USA +1.831.656.2149
> > X3D graphics, virtual worlds, navy robotics http://faculty.nps.edu/brutzman
More information about the x3d-public
mailing list