From 134541ee6e7111600c973a6b91aff9e0c3689ff9 Mon Sep 17 00:00:00 2001 From: Kostas Karmas Date: Tue, 31 Mar 2020 17:57:23 +0200 Subject: [PATCH 1/7] Fix reloading multiple objects from 3mf file CURA-7333 --- cura/CuraApplication.py | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/cura/CuraApplication.py b/cura/CuraApplication.py index e893e3d8f5..491ddc69c0 100755 --- a/cura/CuraApplication.py +++ b/cura/CuraApplication.py @@ -1382,13 +1382,14 @@ class CuraApplication(QtApplication): if not nodes: return - for node in nodes: + for obj, node in enumerate(nodes): mesh_data = node.getMeshData() if mesh_data: file_name = mesh_data.getFileName() if file_name: job = ReadMeshJob(file_name) + job.object_to_be_reloaded = obj # The object index to be loaded by this specific ReadMeshJob job._node = node # type: ignore job.finished.connect(self._reloadMeshFinished) if has_merged_nodes: @@ -1575,10 +1576,15 @@ class CuraApplication(QtApplication): def _reloadMeshFinished(self, job): # TODO; This needs to be fixed properly. We now make the assumption that we only load a single mesh! job_result = job.getResult() + object_to_be_reloaded = job.object_to_be_reloaded if len(job_result) == 0: Logger.log("e", "Reloading the mesh failed.") return - mesh_data = job_result[0].getMeshData() + try: # In case the object has disappeared after reloading, log a warning and keep the old mesh in the scene + mesh_data = job_result[object_to_be_reloaded].getMeshData() + except IndexError: + Logger.warning("Object at index {} no longer exists! Keeping the old version in the scene.".format(object_to_be_reloaded)) + return if not mesh_data: Logger.log("w", "Could not find a mesh in reloaded node.") return From b118a46630f85162cdba77d4eb08e9e40a5c61f7 Mon Sep 17 00:00:00 2001 From: Kostas Karmas Date: Tue, 31 Mar 2020 18:02:13 +0200 Subject: [PATCH 2/7] Remove TODO message from 3mf reload function CURA-7333 --- cura/CuraApplication.py | 1 - 1 file changed, 1 deletion(-) diff --git a/cura/CuraApplication.py b/cura/CuraApplication.py index 491ddc69c0..7bd9788d17 100755 --- a/cura/CuraApplication.py +++ b/cura/CuraApplication.py @@ -1574,7 +1574,6 @@ class CuraApplication(QtApplication): fileCompleted = pyqtSignal(str) def _reloadMeshFinished(self, job): - # TODO; This needs to be fixed properly. We now make the assumption that we only load a single mesh! job_result = job.getResult() object_to_be_reloaded = job.object_to_be_reloaded if len(job_result) == 0: From 97199d72ad41747295f523a51bac1bed4e4fab46 Mon Sep 17 00:00:00 2001 From: Kostas Karmas Date: Wed, 1 Apr 2020 17:08:30 +0200 Subject: [PATCH 3/7] Reload nodes based on object index Refactored the code in order to extract the index of the node inside the file and then use that index to reload the correct object from the file. CURA-7333 --- cura/CuraApplication.py | 61 ++++++++++++++++++++++++++++++++++------- 1 file changed, 51 insertions(+), 10 deletions(-) diff --git a/cura/CuraApplication.py b/cura/CuraApplication.py index 7bd9788d17..eafd86389e 100755 --- a/cura/CuraApplication.py +++ b/cura/CuraApplication.py @@ -2,6 +2,7 @@ # Cura is released under the terms of the LGPLv3 or higher. import os +import re import sys import time from typing import cast, TYPE_CHECKING, Optional, Callable, List, Any @@ -1382,23 +1383,63 @@ class CuraApplication(QtApplication): if not nodes: return - for obj, node in enumerate(nodes): + objects_in_filename = {} + for node in nodes: mesh_data = node.getMeshData() - if mesh_data: file_name = mesh_data.getFileName() if file_name: - job = ReadMeshJob(file_name) - job.object_to_be_reloaded = obj # The object index to be loaded by this specific ReadMeshJob - job._node = node # type: ignore - job.finished.connect(self._reloadMeshFinished) - if has_merged_nodes: - job.finished.connect(self.updateOriginOfMergedMeshes) - - job.start() + if file_name not in objects_in_filename: + objects_in_filename[file_name] = [] + if file_name in objects_in_filename: + objects_in_filename[file_name].append(node) else: Logger.log("w", "Unable to reload data because we don't have a filename.") + for file_name, nodes in objects_in_filename.items(): + for object_index, node in enumerate(nodes): + job = ReadMeshJob(file_name) + job.object_to_be_reloaded = self._getObjectIndexInFile(file_name, node.getName()) # The object index in file to be loaded by this specific job + job._node = node # type: ignore + job.finished.connect(self._reloadMeshFinished) + if has_merged_nodes: + job.finished.connect(self.updateOriginOfMergedMeshes) + + job.start() + + @staticmethod + def _getObjectIndexInFile(file_name: str, node_name: str) -> int: + """ + This function extracts the index of the object inside a file. This is achieved by looking into the name + of the node. There are two possibilities: + * The node is named as filename.ext, filename.ext(1), filename.ext(2), etc, which maps to indices 0, 1, 2, ... + * The node is named as Object 1, Object 2, Object 3 etc, which maps to indices 0, 1, 2 ... + + :param file_name: The name of the file where the node has been retrieved from + :param node_name: The name of the node as presented in the Scene + :return: The index of the node inside the file_name + """ + file_name = file_name.split("/")[-1] # Keep only the filename, without the path + node_int_index = 0 + if file_name in node_name: + # if the file_name exists inside the node_name, remove it along with all parenthesis and spaces + node_str_index = re.sub(r'[() ]', '', node_name.replace(file_name, "")) + try: + node_int_index = int(node_str_index) + except ValueError: + Logger.warning("Object '{}' has an incorrect index '{}'.".format(node_name, node_str_index)) + return 0 + elif "Object " in node_name: + # if the nodes are named as Object 1, Object 2, etc, remove 'Object ' and keep only the number + node_str_index = node_name.replace("Object ", "") + try: + node_int_index = int(node_str_index) - 1 + except ValueError: + Logger.warning("Object '{}' has an incorrect index '{}'.".format(node_name, node_str_index)) + return 0 + return node_int_index + + @pyqtSlot("QStringList") def setExpandedCategories(self, categories: List[str]) -> None: categories = list(set(categories)) From 99a762bcb9e8d93b0fd0917ac119ee2de2609e06 Mon Sep 17 00:00:00 2001 From: Kostas Karmas Date: Wed, 1 Apr 2020 17:42:26 +0200 Subject: [PATCH 4/7] Avoid warning for object in position 0 CURA-7333 --- cura/CuraApplication.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/cura/CuraApplication.py b/cura/CuraApplication.py index eafd86389e..05f94dffca 100755 --- a/cura/CuraApplication.py +++ b/cura/CuraApplication.py @@ -1424,6 +1424,8 @@ class CuraApplication(QtApplication): if file_name in node_name: # if the file_name exists inside the node_name, remove it along with all parenthesis and spaces node_str_index = re.sub(r'[() ]', '', node_name.replace(file_name, "")) + if node_str_index == "": + node_str_index = "0" try: node_int_index = int(node_str_index) except ValueError: From 5bdcc2e5ba5727403bed9a0fe2b4020807915150 Mon Sep 17 00:00:00 2001 From: Kostas Karmas Date: Wed, 1 Apr 2020 18:41:04 +0200 Subject: [PATCH 5/7] Add type to objects_in_filenames CURA-7333 --- cura/CuraApplication.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/cura/CuraApplication.py b/cura/CuraApplication.py index 05f94dffca..2660c36fe9 100755 --- a/cura/CuraApplication.py +++ b/cura/CuraApplication.py @@ -5,7 +5,7 @@ import os import re import sys import time -from typing import cast, TYPE_CHECKING, Optional, Callable, List, Any +from typing import cast, TYPE_CHECKING, Optional, Callable, List, Any, Dict import numpy from PyQt5.QtCore import QObject, QTimer, QUrl, pyqtSignal, pyqtProperty, QEvent, Q_ENUMS @@ -1383,7 +1383,7 @@ class CuraApplication(QtApplication): if not nodes: return - objects_in_filename = {} + objects_in_filename = {} # type: Dict[str, List[CuraSceneNode]] for node in nodes: mesh_data = node.getMeshData() if mesh_data: From 8eb48672e17fa969236c66716430e28adfc44185 Mon Sep 17 00:00:00 2001 From: Kostas Karmas Date: Fri, 3 Apr 2020 11:05:38 +0200 Subject: [PATCH 6/7] Use the node id as identifier Now that libSavitar allows us to read the object id from the 3mf file, this id will be propagated as an id inside CuraSceneNodes and it will be used as an identifier to find the object that has to be refreshed. CURA-7333 --- cura/CuraApplication.py | 65 ++++++++++-------------------- plugins/3MFReader/ThreeMFReader.py | 11 ++--- 2 files changed, 27 insertions(+), 49 deletions(-) diff --git a/cura/CuraApplication.py b/cura/CuraApplication.py index 2660c36fe9..cc92b1d914 100755 --- a/cura/CuraApplication.py +++ b/cura/CuraApplication.py @@ -1397,9 +1397,8 @@ class CuraApplication(QtApplication): Logger.log("w", "Unable to reload data because we don't have a filename.") for file_name, nodes in objects_in_filename.items(): - for object_index, node in enumerate(nodes): + for node in nodes: job = ReadMeshJob(file_name) - job.object_to_be_reloaded = self._getObjectIndexInFile(file_name, node.getName()) # The object index in file to be loaded by this specific job job._node = node # type: ignore job.finished.connect(self._reloadMeshFinished) if has_merged_nodes: @@ -1407,41 +1406,6 @@ class CuraApplication(QtApplication): job.start() - @staticmethod - def _getObjectIndexInFile(file_name: str, node_name: str) -> int: - """ - This function extracts the index of the object inside a file. This is achieved by looking into the name - of the node. There are two possibilities: - * The node is named as filename.ext, filename.ext(1), filename.ext(2), etc, which maps to indices 0, 1, 2, ... - * The node is named as Object 1, Object 2, Object 3 etc, which maps to indices 0, 1, 2 ... - - :param file_name: The name of the file where the node has been retrieved from - :param node_name: The name of the node as presented in the Scene - :return: The index of the node inside the file_name - """ - file_name = file_name.split("/")[-1] # Keep only the filename, without the path - node_int_index = 0 - if file_name in node_name: - # if the file_name exists inside the node_name, remove it along with all parenthesis and spaces - node_str_index = re.sub(r'[() ]', '', node_name.replace(file_name, "")) - if node_str_index == "": - node_str_index = "0" - try: - node_int_index = int(node_str_index) - except ValueError: - Logger.warning("Object '{}' has an incorrect index '{}'.".format(node_name, node_str_index)) - return 0 - elif "Object " in node_name: - # if the nodes are named as Object 1, Object 2, etc, remove 'Object ' and keep only the number - node_str_index = node_name.replace("Object ", "") - try: - node_int_index = int(node_str_index) - 1 - except ValueError: - Logger.warning("Object '{}' has an incorrect index '{}'.".format(node_name, node_str_index)) - return 0 - return node_int_index - - @pyqtSlot("QStringList") def setExpandedCategories(self, categories: List[str]) -> None: categories = list(set(categories)) @@ -1616,16 +1580,29 @@ class CuraApplication(QtApplication): fileLoaded = pyqtSignal(str) fileCompleted = pyqtSignal(str) - def _reloadMeshFinished(self, job): - job_result = job.getResult() - object_to_be_reloaded = job.object_to_be_reloaded + def _reloadMeshFinished(self, job: ReadMeshJob) -> None: + """ + Function called whenever a ReadMeshJob finishes in the background. It reloads a specific node object in the + scene from its source file. The function gets all the nodes that exist in the file through the job result, and + then finds the scene node that it wants to refresh by its object id. Each job refreshes only one node. + + :param job: The ReadMeshJob running in the background that reads all the meshes in a file + :return: None + """ + job_result = job.getResult() # nodes that exist inside the file read by this job if len(job_result) == 0: Logger.log("e", "Reloading the mesh failed.") return - try: # In case the object has disappeared after reloading, log a warning and keep the old mesh in the scene - mesh_data = job_result[object_to_be_reloaded].getMeshData() - except IndexError: - Logger.warning("Object at index {} no longer exists! Keeping the old version in the scene.".format(object_to_be_reloaded)) + object_found = False + mesh_data = None + # Find the node to be refreshed based on its id + for job_result_node in job_result: + if job_result_node.getId() == job._node.getId(): + mesh_data = job_result_node.getMeshData() + object_found = True + break + if not object_found: + Logger.warning("The object with id {} no longer exists! Keeping the old version in the scene.".format(job_result_node.getId())) return if not mesh_data: Logger.log("w", "Could not find a mesh in reloaded node.") diff --git a/plugins/3MFReader/ThreeMFReader.py b/plugins/3MFReader/ThreeMFReader.py index 670049802d..a368cedd75 100755 --- a/plugins/3MFReader/ThreeMFReader.py +++ b/plugins/3MFReader/ThreeMFReader.py @@ -52,7 +52,6 @@ class ThreeMFReader(MeshReader): self._root = None self._base_name = "" self._unit = None - self._object_count = 0 # Used to name objects as there is no node name yet. def _createMatrixFromTransformationString(self, transformation: str) -> Matrix: if transformation == "": @@ -87,17 +86,20 @@ class ThreeMFReader(MeshReader): ## Convenience function that converts a SceneNode object (as obtained from libSavitar) to a scene node. # \returns Scene node. def _convertSavitarNodeToUMNode(self, savitar_node: Savitar.SceneNode, file_name: str = "") -> Optional[SceneNode]: - self._object_count += 1 - node_name = savitar_node.getName() + node_id = savitar_node.getId() if node_name == "": - node_name = "Object %s" % self._object_count + if file_name != "": + node_name = os.path.basename(file_name) + else: + node_name = "Object {}".format(node_id) active_build_plate = CuraApplication.getInstance().getMultiBuildPlateModel().activeBuildPlate um_node = CuraSceneNode() # This adds a SettingOverrideDecorator um_node.addDecorator(BuildPlateDecorator(active_build_plate)) um_node.setName(node_name) + um_node.setId(node_id) transformation = self._createMatrixFromTransformationString(savitar_node.getTransformation()) um_node.setTransformation(transformation) mesh_builder = MeshBuilder() @@ -169,7 +171,6 @@ class ThreeMFReader(MeshReader): def _read(self, file_name: str) -> Union[SceneNode, List[SceneNode]]: result = [] - self._object_count = 0 # Used to name objects as there is no node name yet. # The base object of 3mf is a zipped archive. try: archive = zipfile.ZipFile(file_name, "r") From 4495cea8a4a29695af595c14eb5e5f2a2a2829f4 Mon Sep 17 00:00:00 2001 From: Kostas Karmas Date: Fri, 3 Apr 2020 11:20:09 +0200 Subject: [PATCH 7/7] Remove type of job to avoid type error If the type of the job is mentioned, the CI/CD system complains about reading the "private" variable _node of the job. CURA-7333 --- cura/CuraApplication.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cura/CuraApplication.py b/cura/CuraApplication.py index cc92b1d914..e6eb999e7a 100755 --- a/cura/CuraApplication.py +++ b/cura/CuraApplication.py @@ -1580,7 +1580,7 @@ class CuraApplication(QtApplication): fileLoaded = pyqtSignal(str) fileCompleted = pyqtSignal(str) - def _reloadMeshFinished(self, job: ReadMeshJob) -> None: + def _reloadMeshFinished(self, job) -> None: """ Function called whenever a ReadMeshJob finishes in the background. It reloads a specific node object in the scene from its source file. The function gets all the nodes that exist in the file through the job result, and