[x3d-public] Various comments reading X3Dv4 current working draft (WD3 in GitHub repository)

Sat Dec 5 15:29:49 PST 2020

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