x3d-serializer: specular highlights and shape names

[andreasplesch]

All Submissions:

• [x] Have you followed the guidelines in our Contributing document?
• [x] Have you checked to ensure there aren't other open Pull Requests for the same update/change?
• [x] Does your submission pass tests?

Rather subtle improvement to rendering by adding some specular highlighting similar to the default renderer. See #1234.

[andreasplesch]

Without highlights (current) With highlights, this PR

[z3dev]

@andreasplesch this seems reasonable but i think colors and other material attributes should be provided at the shape level.

several of the serializers already support other attributes on shapes, for example 'name', 'id', etc.

if the other X3D material attributes are important then how about adding support for those?

https://doc.x3dom.org/developer/x3dom/nodeTypes/Material.html

[andreasplesch]

Yes, the Material fields correspond to the shape level. Only diffuseColor is important for 3d printing, I believe. The other fields are important for screen rendering. The initial idea was to get closer to the rendering of the regl renderer by default which also uses specular highlights. Further refinement could be done in postprocessing outside of jscad. For that it would be useful to support a 'name' attribute which would correspond to the X3D DEF attribute.

It would be possible to look for object properties with the Material field names such as object.ambientIntensity and then use those for the x3d serialization. This way other serializers or the regl renderer could also use these object properties.

Should these properties be grouped in a material (or x3d) subobject: object.material.ambientIntensity ? Should these properties be managed or propagated by jscad in operations which create new objects ? This seems out of scope at this point.

There are some decisions which then would follow:

• Use x3d defaults values, except for specularColor and shininess.
• geom3 uses color as diffuseColor. If both are given use diffuseColor.
• line geom uses color as emissiveColor. If both are given use emissiveColor.
• If line geom has properties other than color, include all in the material node. This is legal although x3d will only use emissive.
• Allow options for all fields to be used if object property does not exist.

So, it may suffice to support only a 'name' attribute and expect postprocessing outside of jscad. Adding support for all material fields as object attributes is also possible, potentially allowing use of those elsewhere although they are somewhat x3d specific.

I am fine with both options.

[andreasplesch]

I added support for a .name attribute. There is no default option. Because X3D requires unique DEF names, I added a function which adds suffixes for Shapes which have identical .names. This could be left out and become a responsibility for the designer.

With regards to Material attributes, this may not be a priority but please let me know and I can add support.

[andreasplesch]

Yes, DEF names in x3d are required to be unique. x3d processors may complain.

---on the phone---

On Mon, May 1, 2023, 2:38 AM Z3 Development @.***> wrote:

@.**** commented on this pull request.

In packages/io/x3d-serializer/src/index.js https://github.com/jscad/OpenJSCAD.org/pull/1239#discussion_r1181419529:

@@ -165,6 +168,24 @@ const convertGeom2 = (object, options) => { return group }

+/*

• • generate attributes for Shape node
• */

• +const shapeAttributes = (object, attributes = {}) => {

• if (object.name) {
• Object.assign(attributes, { DEF: uniqueDefName(object.name) })

that sounds reasonable. by definition, are DEF (names) expected to be unique?

— Reply to this email directly, view it on GitHub https://github.com/jscad/OpenJSCAD.org/pull/1239#discussion_r1181419529, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABPCT266NBVHKWQJJDPVNULXD5K6TANCNFSM6AAAAAAXP6LHNU . You are receiving this because you were mentioned.Message ID: @.***>

[andreasplesch]

Thanks for taking a look. I fixed the lint reports and requested review to convert from draft to PR. If this is ready to be merged, I can address remaining items later in another PR.