allow EXTERNPROTO, PROTO inside MFNode in classic encoding

[brutzman]

The following scene

• https://www.web3d.org/x3d/content/examples/X3dForAdvancedModeling/SanCarlosCathedral/SanCarlosCathedralInex.html
• https://www.web3d.org/x3d/content/examples/X3dForAdvancedModeling/SanCarlosCathedral/SanCarlosCathedral.x3d
• https://www.web3d.org/x3d/content/examples/X3dForAdvancedModeling/SanCarlosCathedral/SanCarlosCathedral.x3dv

has <Group DEF='PrototypeDeclarations'> containing ExternProtoDeclare and ProtoDeclare statements. ClassicVRML validation yields errors like

C:\x3d-code\www.web3d.org\x3d\content\examples\X3dForAdvancedModeling/SanCarlosCathedral//SanCarlosCathedral.x3d processing with X3dToX3dvClassicVrmlEncoding stylesheet
ClassicVRML validation: tovrmlx3d.exe SanCarlosCathedral//SanCarlosCathedral.x3dv --validate --enable-downloads
tovrmlx3d: Warning: VRML/X3D: Error when reading, will skip the rest of X3D file: Error at line 352 column 18: Expected node type or DEF or USE, got keyword "EXTERNPROTO"
Exception "Exception":
Validation failed (consult the warnings above), exiting with non-zero status
Result: 1

Interestingly, view3dscene did not provide these warnings for either .x3d or .x3dv versions of the scene.

Deleting the ExternProtoDeclare yielded a similar error for the following ProtoDeclare statement.

These statements are allowed within an MFNode children { ] list and should not raise error flags.

[michaliskambi]

These statements are allowed within an MFNode children { ] list and should not raise error flags.

We talked about it on x3d-public this year, and the resolution was opposite. You cannot place such things (EXTERNPROTO, PROTO, ROUTE) inside the MFNode list (inside [ ... ]). The X3D classic encoding specification clearly states this.

I recall our thread on x3d-public had links to X3D specifications confirming this, and a discussion with consensus (as far as I understand) it should stay like it, don't change the spec.

You have to place things like EXTERNPROTO, PROTO, ROUTE directly inside the node contents, i.e. it cannot be mixed within the children [ ... ] field contents.

Testing, both view3dscene and tovrmlx3d report error about it for the classic encoding (.x3dv) version. The XML encoding (.x3d) doesn't have any issue.

[brutzman]

The email discussion and specification implications were carefully considered by the X3D Working Group and X3D specification editors. You will likely find further detail inside Mantis Issue Tracker noting key points. Thanks again for those earlier inputs.

Once again X3D 4.0 Architecture abstract specification, which is now finalized, takes precedence for current work. Note also this content model for ExternProtoDeclare and ProtoDeclare statements inside a children list, is explicitly allowed in X3D Unified Object Model (X3DUOM), X3D XML Schema and XML DOCTYPE. Of course the primary design imperative remains throughout is for 19775-1 X3D Architecture to describe functionality, with corresponding specifications for each separate 19776-* file encoding and 19777-* programming-language binding describing specific implementation rules for that context.

• https://www.web3d.org/standards

Therefore we need to craft prose and grammar modifications for revising X3D 19776-2 ClassicVRML Encoding for an update to match X3D Architecture 4.0. If you want to suggest them here, great.

• https://www.web3d.org/documents/specifications/19776-2/V3.3/index.html

Please reopen the issue. The current approach is incorrect.

We can then follow our regular standards-development process and post all such recommendations on x3d-public mailing list, for X3D Working Group consideration, with distilled rationales in Mantis Issue Tracker, and changes going into the X3D Specifications that are maintained by the Web3D Consortium on github.

• https://www.web3d.org/standards/adoption-process

This due diligence using a proven process is how we have built all of our X3D improvements so successfully. Again thanks for careful attention to detail and working together to continue these success.

[michaliskambi]

Please reopen the issue. The current approach is incorrect.

I really think the current approach is correct.

1. What we have implemented in CGE/view3dscene now is valid with regards to the current specification

2. and I don't see a pressing need to change the specification.

The input testcase you show should just be fixed, IMHO. That's not a ticket for view3dscene, it's a ticket for that testcase author.

The AD 1 (what we do follows current spec) is I think a "fact" -- that is, reading the current spec, https://www.web3d.org/documents/specifications/19776-2/V3.3/Part02/grammar.html , the mfnodeValue element contains a list of nodeStatement . And nodeStatement cannot be ROUTE, EXTERNPROTO or such. If you believe otherwise, please submit a testcase -- an input X3D in classic encoding, that you think is valid with respect to the spec, and that we don't handle

The AD 2 (whether we should change the spec) -- I realize this is more like "opinion". For me, the current spec (with nodeStatement that cannot be ROUTE, EXTERNPROTO ) was evidently done deliberately and that's a good rule. Just like in other programming languages, you cannot "intersperse" the declaration of a list with other statements.

We did talk about it in June this year, x3d-public mailing list "[x3d-public] Problem in x3dviewscene: ROUTE placement". Vincent confirmed my opinion, that changing spec was not desirable. Doug did tests on browsers, and some of them, just like CGE/view3dscene, follow the grammar strictly and don't permit ROUTE, EXTERNPROTO in MFString.

If you want to pursue changing the spec, OK. Let's first get consensus on x3d-public among X3D browser developers.

[brutzman]

The upcoming ClassicVRML Encoding 4.0 specification will be changed to match the X3D 4.0 Architecture, so consensus is already achieved. This issue is mistakenly closed.

[michaliskambi]

The upcoming ClassicVRML Encoding 4.0 specification will be changed to match the X3D 4.0 Architecture, so consensus is already achieved. This issue is mistakenly closed.

I don't see how the resolution can "match the X3D 4.0 Architecture". The X3D 4.0 Architecture doesn't say one can place EXTERNPROTO etc. in the middle of MFNode lists. The issue is independent of X3D 4.0 Architecture spec IMHO, it is just about classic encoding formal grammar.

That said, I have implemented what I believe is the change you intended. I hope --- as I don't have access to the updated classic 4.0 specification you mention (it is not public as far as I see) and I don't have access to Mantis also anymore.

I still think this is a bad spec change, but I have implemented it.

The implementation and tests confirmed my fears, to be honest: This spec change introduces a weird set of situations, when EXTERNPROTO/PROTO/ROUTE/IMPORT/EXPORT are inside MFNode as far as syntax is concerned, but for browsers they are more logically inside the parent nodes of these MFNode fields. This introduces edge-cases when trying to save the resulting graph back. To be really "on the safe side" one could change browser definition of MFNode from a "list of nodes" but a "list of anything", but that's adding a significant complexity, so instead we place the EXTERNPROTO/PROTO/ROUTE/IMPORT/EXPORT inside parent node, and hope that the order on save will be correct. Which is tricky, as named nodes and protos may depend on each other. The testcase https://github.com/castle-engine/demo-models/blob/master/x3d/routes_protos_within_mfnode.x3dv explores these challenging cases.

I'll posted about it to the x3d-public, so that other browser vendors may be prepared for it and may reuse my testcase https://github.com/castle-engine/demo-models/blob/master/x3d/routes_protos_within_mfnode.x3dv .