3MFConsortium / spec_core

3MF's Core specification
BSD 2-Clause "Simplified" License
56 stars 16 forks source link

Moving Id="rel0 for root model from Production to Core #1

Closed KrisIverson closed 6 years ago

KrisIverson commented 6 years ago

Adding ID=”rel0” to the Core spec appears to be a breaking change. It is also inconsistently specified in the Production spec, as Martin pointed out.

According to the OPC specification, the relationship ID value can be any unique string. The samples I have from MS apps generating this ID, they are random ID values. For example, I used the latest 3D Builder and saved a model. This is what the relationship is:

<?xml version="1.0" encoding="UTF-8"?>

Additionally, there are a couple of samples in the Core spec that do not follow the rel0 requirement making it clear this was not considered before.

What is most important in the Core spec is that there is a /3D/.model named part of type http://schemas.microsoft.com/3dmanufacturing/2013/01/3dmodel pointing to a valid part.

The difference between the Production spec and Core spec is that Production allows for multiple model parts in the same file and the relationship is used to identify and enumerate them. The Id=”1” requirement (why was it not 0 indexed?) was important to identify the root-level 3D model part.

However, this clause further confused me: “Non-root model files must not be referenced from the root .rels file.” Given that, the simplest thing to do is just remove this sentence entirely: “The root model file MUST always be referenced in the root .rels file with Id=”0”; as there can only be the root model in the root rels. The ID value of the relationship isn’t relevant.

Recommendation:

• Remove “The root model file MUST always be referenced in the root .rels file with Id=”0” from the Production spec • Remove the Quality Logic test for this case • Add a new test to ensure that no other model parts are included in the root .rels file to ensure conformance.

jordig100 commented 6 years ago

New production draft removing 'with id="0"', so the root relationship index has no restrictions. QL already updated their test suite to remove the negative test case. QL added to their TODO list to add a new negative test case to test that a second model is rejected.