Update pymvr to 1.0.2, update MVR export/import to match the new library style#3
Conversation
|
@vanous Thanks for the contribution, I just refactored the complete exporter code, maybe it is better to resync your branch, because it would be easier to merge. |
|
@nrgsille76 yes, I have seen that and it's quite unfortunate. You should think about existing PRs before you make big changes to files in PRs. Your changes are big and they cannot be resynced automatically, so it will be quite an effort for me. |
I have now opened a branch for my changes. The big change was nessecary because the exporter did not worked on all scenarios like when there were many subgroups in groups. Now it works stable. |
|
What branch should I merge my PR into? And, when should I start? I want to make sure to prevent this same situation again, so ideally, the main remains untouched, so code can be merged without new conflicts, thank you. |
You can merge into this branch, I already tested the changes in that branch, the importer works flawlessly but I got an exception with the exporter, here the report: @vanous I also have not found out how to append a In the former version of class ChildList(BaseNode):
def __init__(
self,
scene_objects: Optional[List["SceneObject"]] = None,
group_objects: Optional[List["GroupObject"]] = None,
focus_points: Optional[List["FocusPoint"]] = None,
fixtures: Optional[List["Fixture"]] = None,
supports: Optional[List["Support"]] = None,
trusses: Optional[List["Truss"]] = None,
video_screens: Optional[List["VideoScreen"]] = None,
projectors: Optional[List["Projector"]] = None,
xml_node: Optional["Element"] = None,
*args,
**kwargs,
):
self.scene_objects = scene_objects if scene_objects is not None else []
self.group_objects = group_objects if group_objects is not None else []
self.focus_points = focus_points if focus_points is not None else []
self.fixtures = fixtures if fixtures is not None else []
self.supports = supports if supports is not None else []
self.video_screens = video_screens if video_screens is not None else []
self.trusses = trusses if trusses is not None else []
self.projectors = projectors if projectors is not None else []
super().__init__(xml_node, *args, **kwargs)In <AUXData>
<Symdef name="Symbol 1" uuid="2381627C-5E19-11EA-9908-E44F29009047">
<ChildList>
<Geometry3D fileName="geometry.3ds"/>
</ChildList>
</Symdef>
</AUXData> |
|
I am looking at the list of changes and it is huge. It is much safer for me to re-do the transition from old pymvr to new pymvr again from scratch. Which branch do you want me to use? I can see that the "master" branch is developed further, so please confirm that this is the branch to go to. Also, please do not do changes to it in the meanwhile, this takes several hours. Please confirm so i can start with that. |
In the previous version of the library, the library class structure did not exactly mimic the MVR XML structure. In the new version, the python classes are structured the same as the XML, the conversion from/to XML then doesn't utilize extra logic, making it simpler. During conversion of your code, i added the list, but then it did not get processed to XML as pymvr needs changes, i will do them at the same time as rewriting the export. Do you have an MVR which was able to trigger this, so i can test with it? |
I will try to provide a MVR wich triggers that, meanwhile I managed to get an exported mvr file with the new pymvr version, but every object has now fixture attributes, here is a screenshot of the I opened a fork in pymvr and can show whith a PR what needs to be changed. The master is the branch to go, it is ready for the update to pymvr 1.0.+ I will do one push to it in a few minutes and then I will do nothing for the whole weekend ;) |
This is correct, for example the SceneObject you show has those same properties, see spec here.
Good. I have the pymvr's symdefs already fixed in open-stage/python-mvr#13 , and will update the master to latest pymvr during the weekend 👍 |
I have never seen fixture IDs in a <FixtureID xsi:nil="true" />
@vanous I am now finished with my edits in master. It works with my modifications in |
351c5a2 to
ec6889a
Compare
acf37f7 to
497f591
Compare
497f591 to
ededa7a
Compare
|
@nrgsille76 uh, i am actually surprised that you kept my modifications in, that is good, thank you. So i only had to look at the changes related to the Symdef childs_list. I also found some small details and fixed them too, in both library and the import. If you find some time to test, it would be great. |
@vanous I merged this into master and have tested exporting and importing in blender, importing in MA3 on PC and importing in Depence and have it compared to the formally exported mvr's. The common GeneralSceneDescription-common.xml The new GeneralSceneDescription-pymvr.xml I got problems with Symbols in the new pymvr version. Symbols are now always at zero position when importing, I guess it has to do with the matrix in symbol and the matrix in the In MA3 on PC now every object is a fixture in fixture sheet with no attributes. I do not kow exactly if this was intended by MA Lighting, usually the environment is excluded in the fixture sheet and was only editable in patch. I made a screenshot: the same scenerario in Depence, every object is a fixture now. The symbols loaded correctly though, but they are now also fixtures. It would be nice to at least implement this function to pymvr to check, if a matrix is nessecary: check_mtx = any(isinstance(i, float) for i in set().union(sum(self.matrix.matrix[:-1], [])))It is the function from my proposal in the PR14 |
|
@vanous I have exported the Capture test file with my former version (before pymvr changes) and got exact the same result as the original file. Here is the file exported with the latest changes and here also the symbols are not imported correctly anymore, they seem to be completely gone. |
|
Found this only now, and have unfortunately already pushed 1.0.3 out in the mean while.
This is very strange, not sure why would they parse it this way, a Fixture is well defined in the structure. I will remove these FixtureID and so on during export.
OK, will add this, to eliminate the explicit defaults for matrix. Do i understand correctly, that this is an export issue, right? |
|
I have pushed to this the same PR branch https://github.com/vanous/io_scene_mvr/tree/update_pymvr the latest changes from open-stage/python-mvr#16 , this now ensures that Fixture* properties are only exported if not None, also the Matrix check is now in place. |
It was not an issue when there was no matrix in the Symbols and the Symdef Geometry. I do not know if the problem occurs from the matrix in the symbols in this part...: <Scene>
<Layers>
<Layer name="Stands" uuid="d83a7371-e449-4485-8c5c-c99dcd2b2f3e">
<Matrix>{1,0,0}{0,1,0}{0,0,1}{0.0,0.0,0.0}</Matrix>
<ChildList>
<SceneObject name="Stand1" uuid="d36775a3-765b-4d4a-8a36-b762eebcb4c3">
<Matrix>{1.0,0.0,0.0}{0.0,1.0,0.0}{0.0,0.0,1.0}{-3570.6100463867188,-2881.0393810272217,0.0}</Matrix>
<FixtureID />
<FixtureIDNumeric>0</FixtureIDNumeric>
<UnitNumber>0</UnitNumber>
<CustomIdType>0</CustomIdType>
<CustomId>0</CustomId>
<Geometries>
<Symbol uuid="336a00d5-4589-4016-a3ce-f7d86ee808de" symdef="0956c84b-954e-44f7-abd1-f70986ec769a">
<Matrix>{1,0,0}{0,1,0}{0,0,1}{0.0,0.0,0.0}</Matrix>
</Symbol>
</Geometries>
</SceneObject>or the matrix in the Symdef geometry: <AUXData>
<Symdef name="LightStand" uuid="0956c84b-954e-44f7-abd1-f70986ec769a">
<ChildList>
<Geometry3D fileName="LightStand.3ds">
<Matrix>{1,0,0}{0,1,0}{0,0,1}{0.0,0.0,0.0}</Matrix>
</Geometry3D>
</ChildList>
</Symdef>I think it is the matrix in the symdef wich causes the issue, because I've never seen a matrix in a symdef in other files. A few questions: |
|
I can make release 1.0.4 right away. Does it now work as intended? I cannot test with depence, even testing with ma is not simple for me. |
|
pymvr 1.0.4 released. I also added a round-trip test (mimicking the readme documentation) which adds a very quick possibility to read, parse and then export an MVR. It only does the XML file, allowing to observe if there is any issue with the structure or serialization to XML. Here is how to run it from the pymvr git cloned directory: Either run all tests and the round-tripping will be part of them: Or one can run just the round-trip test: It will produce file_exported.mvr in the same directory as the original file, this exported mvr can then be observed, diffed and so on. |
I tested it and now everything works as intended. I am in contact with MA lighting and will try to get information about how they intended to parse mvr, the objects are recognized as Environment as they should, but they are also somehow recognized as gdtf. This has nothing to do with the changes to pymvr, MA3 has also done this before the changes, I just have not noticed that before. |





Hi!
This updates your pymvr implementation to latest version, it greatly simplifies the export and allows you to work with the data natively (no more of the xml dance, as the 1.x version of pymvr does not leak the serialization for each element). It also eliminates the need for .clear() in many places due to the bad class initialization and so on, which was the case in pre 1.x exporter. I tested a few MVRs, please try some other just in case. I have seen an issue with a fixture_id == None, i will handle this in pymvr. If you find other places with a similar issue, feel free to open an issue in pymvr, it is probably better to catch this in the library. I did not apply any formatting, to match your code.