From 23904e1dcacddb2675b63d1123466a337bb710bf Mon Sep 17 00:00:00 2001 From: Ghostkeeper Date: Tue, 8 Oct 2019 14:16:19 +0200 Subject: [PATCH 01/27] Don't cache machine manager and application This makes testing harder. Also fixed the typing of the application this way. Contributes to issue CURA-6793. --- cura/Machines/Models/NozzleModel.py | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/cura/Machines/Models/NozzleModel.py b/cura/Machines/Models/NozzleModel.py index 6b6503d187..5f7f8b02f5 100644 --- a/cura/Machines/Models/NozzleModel.py +++ b/cura/Machines/Models/NozzleModel.py @@ -3,9 +3,9 @@ from PyQt5.QtCore import Qt -from UM.Application import Application from UM.Logger import Logger from UM.Qt.ListModel import ListModel +import cura.CuraApplication # Imported like this to prevent circular dependencies. from cura.Machines.ContainerTree import ContainerTree @@ -21,16 +21,13 @@ class NozzleModel(ListModel): self.addRoleName(self.HotendNameRole, "hotend_name") self.addRoleName(self.ContainerNodeRole, "container_node") - self._application = Application.getInstance() - self._machine_manager = self._application.getMachineManager() - - self._machine_manager.globalContainerChanged.connect(self._update) + cura.CuraApplication.CuraApplication.getInstance().getMachineManager().globalContainerChanged.connect(self._update) self._update() def _update(self): Logger.log("d", "Updating {model_class_name}.".format(model_class_name = self.__class__.__name__)) - global_stack = self._machine_manager.activeMachine + global_stack = cura.CuraApplication.CuraApplication.getInstance().getGlobalContainerStack() if global_stack is None: self.setItems([]) return From b137e6a36d9038cf80a2a4932d490c89dc7a366e Mon Sep 17 00:00:00 2001 From: Ghostkeeper Date: Tue, 8 Oct 2019 15:21:19 +0200 Subject: [PATCH 02/27] Don't add extruders twice upon start-up The extruders are added when changing printers anyway, and we call this change signal upon start-up. So one of them is going to do unnecessary work. Contributes to issue CURA-6793. --- cura/Settings/ExtruderManager.py | 2 -- cura/Settings/MachineManager.py | 2 +- 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/cura/Settings/ExtruderManager.py b/cura/Settings/ExtruderManager.py index 417f7b01ff..121b465432 100755 --- a/cura/Settings/ExtruderManager.py +++ b/cura/Settings/ExtruderManager.py @@ -42,8 +42,6 @@ class ExtruderManager(QObject): # TODO; I have no idea why this is a union of ID's and extruder stacks. This needs to be fixed at some point. self._selected_object_extruders = [] # type: List[Union[str, "ExtruderStack"]] - self._addCurrentMachineExtruders() - Selection.selectionChanged.connect(self.resetSelectedObjectExtruders) ## Signal to notify other components when the list of extruders for a machine definition changes. diff --git a/cura/Settings/MachineManager.py b/cura/Settings/MachineManager.py index 13b2c76bc6..2454d5a67c 100755 --- a/cura/Settings/MachineManager.py +++ b/cura/Settings/MachineManager.py @@ -140,7 +140,7 @@ class MachineManager(QObject): activeVariantChanged = pyqtSignal() activeQualityChanged = pyqtSignal() activeIntentChanged = pyqtSignal() - activeStackChanged = pyqtSignal() # Emitted whenever the active stack is changed (ie: when changing between extruders, changing a profile, but not when changing a value) + activeStackChanged = pyqtSignal() # Emitted whenever the active extruder stack is changed (ie: when changing between extruders, changing a profile, but not when changing a value) extruderChanged = pyqtSignal() globalValueChanged = pyqtSignal() # Emitted whenever a value inside global container is changed. From dd8ee2e3d8b94148eaabb5f7e9816b3bbe92cee1 Mon Sep 17 00:00:00 2001 From: Ghostkeeper Date: Tue, 8 Oct 2019 15:22:12 +0200 Subject: [PATCH 03/27] Switch early fail around to reduce indentation Makes this more readable. Contributes to issue CURA-6793. --- cura/Settings/ExtruderManager.py | 47 ++++++++++++++++---------------- 1 file changed, 24 insertions(+), 23 deletions(-) diff --git a/cura/Settings/ExtruderManager.py b/cura/Settings/ExtruderManager.py index 121b465432..8f38eda519 100755 --- a/cura/Settings/ExtruderManager.py +++ b/cura/Settings/ExtruderManager.py @@ -323,35 +323,36 @@ class ExtruderManager(QObject): ## Adds the extruders of the currently active machine. def _addCurrentMachineExtruders(self) -> None: global_stack = self._application.getGlobalContainerStack() + if not global_stack: + return + extruders_changed = False + container_registry = ContainerRegistry.getInstance() + global_stack_id = global_stack.getId() - if global_stack: - container_registry = ContainerRegistry.getInstance() - global_stack_id = global_stack.getId() + # Gets the extruder trains that we just created as well as any that still existed. + extruder_trains = container_registry.findContainerStacks(type = "extruder_train", machine = global_stack_id) - # Gets the extruder trains that we just created as well as any that still existed. - extruder_trains = container_registry.findContainerStacks(type = "extruder_train", machine = global_stack_id) + # Make sure the extruder trains for the new machine can be placed in the set of sets + if global_stack_id not in self._extruder_trains: + self._extruder_trains[global_stack_id] = {} + extruders_changed = True - # Make sure the extruder trains for the new machine can be placed in the set of sets - if global_stack_id not in self._extruder_trains: - self._extruder_trains[global_stack_id] = {} - extruders_changed = True + # Register the extruder trains by position + for extruder_train in extruder_trains: + extruder_position = extruder_train.getMetaDataEntry("position") + self._extruder_trains[global_stack_id][extruder_position] = extruder_train - # Register the extruder trains by position - for extruder_train in extruder_trains: - extruder_position = extruder_train.getMetaDataEntry("position") - self._extruder_trains[global_stack_id][extruder_position] = extruder_train + # regardless of what the next stack is, we have to set it again, because of signal routing. ??? + extruder_train.setParent(global_stack) + extruder_train.setNextStack(global_stack) + extruders_changed = True - # regardless of what the next stack is, we have to set it again, because of signal routing. ??? - extruder_train.setParent(global_stack) - extruder_train.setNextStack(global_stack) - extruders_changed = True - - self.fixSingleExtrusionMachineExtruderDefinition(global_stack) - if extruders_changed: - self.extrudersChanged.emit(global_stack_id) - self.setActiveExtruderIndex(0) - self.activeExtruderChanged.emit() + self.fixSingleExtrusionMachineExtruderDefinition(global_stack) + if extruders_changed: + self.extrudersChanged.emit(global_stack_id) + self.setActiveExtruderIndex(0) + self.activeExtruderChanged.emit() # After 3.4, all single-extrusion machines have their own extruder definition files instead of reusing # "fdmextruder". We need to check a machine here so its extruder definition is correct according to this. From 0238f65e6ab615888f24774df6ee0b363434576f Mon Sep 17 00:00:00 2001 From: Ghostkeeper Date: Tue, 8 Oct 2019 16:32:20 +0200 Subject: [PATCH 04/27] Remove double call of _onGlobalContainerChanged and activeExtruderChanged on switching When switching printers, it would first emit the global container changed signal which connects to _onGlobalContainerChanged, then update stuff in the extruder manager, then manually call _onGlobalContainerChanged again to update some other stuff with the new data from the extruder manager. This was prohibitively expensive, so this prevents that. Another double or triple emit of the activeExtruderChanged was removed in the extruder manager when creating the extruders for a printer: It would first set the extruder number to 0, possibly emitting the signal, then emit the signal just to be sure since the extruder itself changed (rather than just the number), and then change the extruder number to the preferred extruder, possibly again emitting a signal. Now it just sets the extruder number to the preferred extruder and always emits the signal once (either through setting the extruder number or manually afterwards). Contributes to issue CURA-6793. --- cura/Settings/ExtruderManager.py | 12 +++--------- cura/Settings/MachineManager.py | 16 +++++++++++----- 2 files changed, 14 insertions(+), 14 deletions(-) diff --git a/cura/Settings/ExtruderManager.py b/cura/Settings/ExtruderManager.py index 8f38eda519..b7fca7e6f9 100755 --- a/cura/Settings/ExtruderManager.py +++ b/cura/Settings/ExtruderManager.py @@ -1,4 +1,4 @@ -# Copyright (c) 2018 Ultimaker B.V. +# Copyright (c) 2019 Ultimaker B.V. # Cura is released under the terms of the LGPLv3 or higher. from PyQt5.QtCore import pyqtSignal, pyqtProperty, QObject, QVariant # For communicating data and events to Qt. @@ -320,12 +320,8 @@ class ExtruderManager(QObject): self.resetSelectedObjectExtruders() - ## Adds the extruders of the currently active machine. - def _addCurrentMachineExtruders(self) -> None: - global_stack = self._application.getGlobalContainerStack() - if not global_stack: - return - + ## Adds the extruders to the selected machine. + def addMachineExtruders(self, global_stack: GlobalStack) -> None: extruders_changed = False container_registry = ContainerRegistry.getInstance() global_stack_id = global_stack.getId() @@ -351,8 +347,6 @@ class ExtruderManager(QObject): self.fixSingleExtrusionMachineExtruderDefinition(global_stack) if extruders_changed: self.extrudersChanged.emit(global_stack_id) - self.setActiveExtruderIndex(0) - self.activeExtruderChanged.emit() # After 3.4, all single-extrusion machines have their own extruder definition files instead of reusing # "fdmextruder". We need to check a machine here so its extruder definition is correct according to this. diff --git a/cura/Settings/MachineManager.py b/cura/Settings/MachineManager.py index 2454d5a67c..4dc8e672aa 100755 --- a/cura/Settings/MachineManager.py +++ b/cura/Settings/MachineManager.py @@ -219,7 +219,10 @@ class MachineManager(QObject): return 0 return len(general_definition_containers[0].getAllKeys()) + ## Triggered when the global container stack is changed in CuraApplication. def _onGlobalContainerChanged(self) -> None: + import traceback + traceback.print_stack() if self._global_container_stack: try: self._global_container_stack.containersChanged.disconnect(self._onContainersChanged) @@ -298,7 +301,6 @@ class MachineManager(QObject): self.blurSettings.emit() # Ensure no-one has focus. container_registry = CuraContainerRegistry.getInstance() - containers = container_registry.findContainerStacks(id = stack_id) if not containers: return @@ -308,21 +310,25 @@ class MachineManager(QObject): # Make sure that the default machine actions for this machine have been added self._application.getMachineActionManager().addDefaultMachineActions(global_stack) - ExtruderManager.getInstance().fixSingleExtrusionMachineExtruderDefinition(global_stack) + extruder_manager = ExtruderManager.getInstance() + extruder_manager.fixSingleExtrusionMachineExtruderDefinition(global_stack) if not global_stack.isValid(): # Mark global stack as invalid ConfigurationErrorMessage.getInstance().addFaultyContainers(global_stack.getId()) return # We're done here self._global_container_stack = global_stack + extruder_manager.addMachineExtruders(global_stack) self._application.setGlobalContainerStack(global_stack) - ExtruderManager.getInstance()._globalContainerStackChanged() - self._onGlobalContainerChanged() # Switch to the first enabled extruder self.updateDefaultExtruder() default_extruder_position = int(self.defaultExtruderPosition) - ExtruderManager.getInstance().setActiveExtruderIndex(default_extruder_position) + old_active_extruder_index = extruder_manager.activeExtruderIndex + extruder_manager.setActiveExtruderIndex(default_extruder_position) + if old_active_extruder_index == default_extruder_position: + # This signal might not have been emitted yet (if it didn't change) but we still want the models to update that depend on it because we changed the contents of the containers too. + extruder_manager.activeExtruderChanged.emit() self.__emitChangedSignals() From 4ffda015db295feeff336c5eedc92d923289f32e Mon Sep 17 00:00:00 2001 From: Ghostkeeper Date: Wed, 9 Oct 2019 10:53:58 +0200 Subject: [PATCH 05/27] Implement lazy loading for machine nodes Should be completely transparent. It'll fail the unit tests though because it now pretends that all printers have machine nodes. Contributes to issue CURA-6793. --- cura/Machines/ContainerTree.py | 65 +++++++++++++++---------------- cura/Settings/CuraStackBuilder.py | 4 -- 2 files changed, 31 insertions(+), 38 deletions(-) diff --git a/cura/Machines/ContainerTree.py b/cura/Machines/ContainerTree.py index 50ccc893a9..0c39f0d391 100644 --- a/cura/Machines/ContainerTree.py +++ b/cura/Machines/ContainerTree.py @@ -3,7 +3,6 @@ from UM.Logger import Logger from UM.Settings.ContainerRegistry import ContainerRegistry # To listen to containers being added. -from UM.Settings.Interfaces import ContainerInterface from UM.Signal import Signal import cura.CuraApplication # Imported like this to prevent circular dependencies. from cura.Machines.MachineNode import MachineNode @@ -11,7 +10,6 @@ from cura.Settings.GlobalStack import GlobalStack # To listen only to global st from typing import Dict, List, TYPE_CHECKING import time -import UM.FlameProfiler if TYPE_CHECKING: from cura.Machines.QualityGroup import QualityGroup @@ -38,13 +36,9 @@ class ContainerTree: return cls.__instance def __init__(self) -> None: - self.machines = {} # type: Dict[str, MachineNode] # Mapping from definition ID to machine nodes. + self.machines = self.MachineNodeMap() # Mapping from definition ID to machine nodes with lazy loading. self.materialsChanged = Signal() # Emitted when any of the material nodes in the tree got changed. - container_registry = ContainerRegistry.getInstance() - container_registry.containerAdded.connect(self._machineAdded) - self._loadAll() - ## Get the quality groups available for the currently activated printer. # # This contains all quality groups, enabled or disabled. To check whether @@ -76,19 +70,6 @@ class ContainerTree: extruder_enabled = [extruder.isEnabled for extruder in global_stack.extruderList] return self.machines[global_stack.definition.getId()].getQualityChangesGroups(variant_names, material_bases, extruder_enabled) - # Add a machine node by the id of it's definition. - # This is automatically called by the _machineAdded function, but it's sometimes needed to add a machine node - # faster than would have been done when waiting on any signals (for instance; when creating an entirely new machine) - @UM.FlameProfiler.profile - def addMachineNodeByDefinitionId(self, definition_id: str) -> None: - if definition_id in self.machines: - return # Already have this definition ID. - - start_time = time.time() - self.machines[definition_id] = MachineNode(definition_id) - self.machines[definition_id].materialsChanged.connect(self.materialsChanged) - Logger.log("d", "Adding container tree for {definition_id} took {duration} seconds.".format(definition_id = definition_id, duration = time.time() - start_time)) - ## Builds the initial container tree. def _loadAll(self): Logger.log("i", "Building container tree.") @@ -97,26 +78,15 @@ class ContainerTree: for stack in all_stacks: if not isinstance(stack, GlobalStack): continue # Only want to load global stacks. We don't need to create a tree for extruder definitions. - definition_id = stack.definition.getId() - if definition_id not in self.machines: - definition_start_time = time.time() - self.machines[definition_id] = MachineNode(definition_id) - self.machines[definition_id].materialsChanged.connect(self.materialsChanged) - Logger.log("d", "Adding container tree for {definition_id} took {duration} seconds.".format(definition_id = definition_id, duration = time.time() - definition_start_time)) + _ = self.machines[stack.definition.getId()] # TODO: Load this lazily. Logger.log("d", "Building the container tree took %s seconds", time.time() - start_time) - ## When a printer gets added, we need to build up the tree for that container. - def _machineAdded(self, container_stack: ContainerInterface) -> None: - if not isinstance(container_stack, GlobalStack): - return # Not our concern. - self.addMachineNodeByDefinitionId(container_stack.definition.getId()) - ## For debugging purposes, visualise the entire container tree as it stands # now. def _visualise_tree(self) -> str: lines = ["% CONTAINER TREE"] # Start with array and then combine into string, for performance. - for machine in self.machines.values(): + for machine in self.machines.machines.values(): lines.append(" # " + machine.container_id) for variant in machine.variants.values(): lines.append(" * " + variant.container_id) @@ -126,4 +96,31 @@ class ContainerTree: lines.append(" - " + quality.container_id) for intent in quality.intents.values(): lines.append(" . " + intent.container_id) - return "\n".join(lines) \ No newline at end of file + return "\n".join(lines) + + ## Dictionary-like object that contains the machines. + # + # This handles the lazy loading of MachineNodes. + class MachineNodeMap: + def __init__(self): + self.machines = {} + + ## Returns whether a printer with a certain definition ID exists. This + # is regardless of whether or not the printer is loaded yet. + # \param definition_id The definition to look for. + # \return Whether or not a printer definition exists with that name. + def __contains__(self, definition_id: str) -> bool: + return len(ContainerRegistry.getInstance().findInstanceContainersMetadata(id = definition_id)) == 0 + + ## Returns a machine node for the specified definition ID. + # + # If the machine node wasn't loaded yet, this will load it lazily. + # \param definition_id The definition to look for. + # \return A machine node for that definition. + def __getitem__(self, definition_id: str) -> MachineNode: + if definition_id not in self.machines: + start_time = time.time() + self.machines[definition_id] = MachineNode(definition_id) + self.machines[definition_id].materialsChanged.connect(ContainerTree.getInstance().materialsChanged) + Logger.log("d", "Adding container tree for {definition_id} took {duration} seconds.".format(definition_id = definition_id, duration = time.time() - start_time)) + return self.machines[definition_id] \ No newline at end of file diff --git a/cura/Settings/CuraStackBuilder.py b/cura/Settings/CuraStackBuilder.py index b82c5141fb..61a04e1be6 100644 --- a/cura/Settings/CuraStackBuilder.py +++ b/cura/Settings/CuraStackBuilder.py @@ -37,10 +37,6 @@ class CuraStackBuilder: return None machine_definition = definitions[0] - # The container tree listens to the containerAdded signal to add the definition and build the tree, - # but that signal is emitted with a delay which might not have passed yet. - # Therefore we must make sure that it's manually added here. - container_tree.addMachineNodeByDefinitionId(machine_definition.getId()) machine_node = container_tree.machines[machine_definition.getId()] generated_name = registry.createUniqueName("machine", "", name, machine_definition.getName()) From 5199e3e6db070c2ac45d3aa2d9fbec5e9e2d0a7f Mon Sep 17 00:00:00 2001 From: Ghostkeeper Date: Wed, 9 Oct 2019 16:44:00 +0200 Subject: [PATCH 06/27] Add background job to pre-load other added printers Not just the active one. Contributes to issue CURA-6793. --- cura/Machines/ContainerTree.py | 40 +++++++++++++++++++++++++++++++++- 1 file changed, 39 insertions(+), 1 deletion(-) diff --git a/cura/Machines/ContainerTree.py b/cura/Machines/ContainerTree.py index 0c39f0d391..a1adae7fe4 100644 --- a/cura/Machines/ContainerTree.py +++ b/cura/Machines/ContainerTree.py @@ -1,6 +1,8 @@ # Copyright (c) 2019 Ultimaker B.V. # Cura is released under the terms of the LGPLv3 or higher. +from UM.Job import Job # For our background task of loading MachineNodes lazily. +from UM.JobQueue import JobQueue # For our background task of loading MachineNodes lazily. from UM.Logger import Logger from UM.Settings.ContainerRegistry import ContainerRegistry # To listen to containers being added. from UM.Signal import Signal @@ -38,6 +40,7 @@ class ContainerTree: def __init__(self) -> None: self.machines = self.MachineNodeMap() # Mapping from definition ID to machine nodes with lazy loading. self.materialsChanged = Signal() # Emitted when any of the material nodes in the tree got changed. + cura.CuraApplication.CuraApplication.getInstance().initializationFinished.connect(self._onStartupFinished) # Start the background task to load more machine nodes after start-up is completed. ## Get the quality groups available for the currently activated printer. # @@ -82,6 +85,10 @@ class ContainerTree: Logger.log("d", "Building the container tree took %s seconds", time.time() - start_time) + ## Ran after completely starting up the application. + def _onStartupFinished(self): + JobQueue.getInstance().add(self.MachineNodeLoadJob(self)) + ## For debugging purposes, visualise the entire container tree as it stands # now. def _visualise_tree(self) -> str: @@ -119,8 +126,39 @@ class ContainerTree: # \return A machine node for that definition. def __getitem__(self, definition_id: str) -> MachineNode: if definition_id not in self.machines: + print("-----------------------------------bluuuh", definition_id) start_time = time.time() self.machines[definition_id] = MachineNode(definition_id) self.machines[definition_id].materialsChanged.connect(ContainerTree.getInstance().materialsChanged) Logger.log("d", "Adding container tree for {definition_id} took {duration} seconds.".format(definition_id = definition_id, duration = time.time() - start_time)) - return self.machines[definition_id] \ No newline at end of file + return self.machines[definition_id] + + ## Returns whether we've already cached this definition's node. + # \param definition_id The definition that we may have cached. + # \return ``True`` if it's cached. + def is_loaded(self, definition_id: str) -> bool: + return definition_id in self.machines + + ## Pre-loads all currently added printers as a background task so that + # switching printers in the interface is faster. + class MachineNodeLoadJob(Job): + ## Creates a new background task. + # \param tree_root The container tree instance. This cannot be + # obtained through the singleton static function since the instance + # may not yet be constructed completely. + def __init__(self, tree_root: "ContainerTree"): + self.tree_root = tree_root + super().__init__() + + ## Starts the background task. + # + # The ``JobQueue`` will schedule this on a different thread. + def run(self) -> None: + currently_added = ContainerRegistry.getInstance().findContainerStacks() # Find all currently added global stacks. + for stack in currently_added: + time.sleep(0.5) + if not isinstance(stack, GlobalStack): + continue + definition_id = stack.definition.getId() + if not self.tree_root.machines.is_loaded(definition_id): + _ = self.tree_root.machines[definition_id] \ No newline at end of file From c5b957d0b129b027af86743a6e27b762c1eeec32 Mon Sep 17 00:00:00 2001 From: Ghostkeeper Date: Wed, 9 Oct 2019 16:44:45 +0200 Subject: [PATCH 07/27] Remove debug code Contributes to issue CURA-6793. --- cura/Machines/ContainerTree.py | 17 ----------------- 1 file changed, 17 deletions(-) diff --git a/cura/Machines/ContainerTree.py b/cura/Machines/ContainerTree.py index a1adae7fe4..bb5669ad61 100644 --- a/cura/Machines/ContainerTree.py +++ b/cura/Machines/ContainerTree.py @@ -89,22 +89,6 @@ class ContainerTree: def _onStartupFinished(self): JobQueue.getInstance().add(self.MachineNodeLoadJob(self)) - ## For debugging purposes, visualise the entire container tree as it stands - # now. - def _visualise_tree(self) -> str: - lines = ["% CONTAINER TREE"] # Start with array and then combine into string, for performance. - for machine in self.machines.machines.values(): - lines.append(" # " + machine.container_id) - for variant in machine.variants.values(): - lines.append(" * " + variant.container_id) - for material in variant.materials.values(): - lines.append(" + " + material.container_id) - for quality in material.qualities.values(): - lines.append(" - " + quality.container_id) - for intent in quality.intents.values(): - lines.append(" . " + intent.container_id) - return "\n".join(lines) - ## Dictionary-like object that contains the machines. # # This handles the lazy loading of MachineNodes. @@ -126,7 +110,6 @@ class ContainerTree: # \return A machine node for that definition. def __getitem__(self, definition_id: str) -> MachineNode: if definition_id not in self.machines: - print("-----------------------------------bluuuh", definition_id) start_time = time.time() self.machines[definition_id] = MachineNode(definition_id) self.machines[definition_id].materialsChanged.connect(ContainerTree.getInstance().materialsChanged) From 268da885ee7b006e3b5d3509b783a487643fe127 Mon Sep 17 00:00:00 2001 From: Ghostkeeper Date: Thu, 10 Oct 2019 14:48:46 +0200 Subject: [PATCH 08/27] Remove unused _loadAll function This one is no longer used since we no longer load all stacks upon start-up. Contributes to issue CURA-6793. --- cura/Machines/ContainerTree.py | 12 ------------ 1 file changed, 12 deletions(-) diff --git a/cura/Machines/ContainerTree.py b/cura/Machines/ContainerTree.py index bb5669ad61..4f64b6ad40 100644 --- a/cura/Machines/ContainerTree.py +++ b/cura/Machines/ContainerTree.py @@ -73,18 +73,6 @@ class ContainerTree: extruder_enabled = [extruder.isEnabled for extruder in global_stack.extruderList] return self.machines[global_stack.definition.getId()].getQualityChangesGroups(variant_names, material_bases, extruder_enabled) - ## Builds the initial container tree. - def _loadAll(self): - Logger.log("i", "Building container tree.") - start_time = time.time() - all_stacks = ContainerRegistry.getInstance().findContainerStacks() - for stack in all_stacks: - if not isinstance(stack, GlobalStack): - continue # Only want to load global stacks. We don't need to create a tree for extruder definitions. - _ = self.machines[stack.definition.getId()] # TODO: Load this lazily. - - Logger.log("d", "Building the container tree took %s seconds", time.time() - start_time) - ## Ran after completely starting up the application. def _onStartupFinished(self): JobQueue.getInstance().add(self.MachineNodeLoadJob(self)) From 9323ed5d04a63939985781518f09bdc6c5936f53 Mon Sep 17 00:00:00 2001 From: Ghostkeeper Date: Thu, 10 Oct 2019 14:56:00 +0200 Subject: [PATCH 09/27] Add implementation of get() for the pretend-dict Some places are using this. Contributes to issue CURA-6973. --- cura/Machines/ContainerTree.py | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/cura/Machines/ContainerTree.py b/cura/Machines/ContainerTree.py index 4f64b6ad40..9e31d1ac32 100644 --- a/cura/Machines/ContainerTree.py +++ b/cura/Machines/ContainerTree.py @@ -10,7 +10,7 @@ import cura.CuraApplication # Imported like this to prevent circular dependenci from cura.Machines.MachineNode import MachineNode from cura.Settings.GlobalStack import GlobalStack # To listen only to global stacks being added. -from typing import Dict, List, TYPE_CHECKING +from typing import Dict, List, Optional, TYPE_CHECKING import time if TYPE_CHECKING: @@ -104,6 +104,21 @@ class ContainerTree: Logger.log("d", "Adding container tree for {definition_id} took {duration} seconds.".format(definition_id = definition_id, duration = time.time() - start_time)) return self.machines[definition_id] + ## Gets a machine node for the specified definition ID, with default. + # + # The default is returned if there is no definition with the specified + # ID. If the machine node wasn't loaded yet, this will load it lazily. + # \param definition_id The definition to look for. + # \param default The machine node to return if there is no machine + # with that definition (can be ``None`` optionally or if not + # provided). + # \return A machine node for that definition, or the default if there + # is no definition with the provided definition_id. + def get(self, definition_id: str, default: Optional[MachineNode] = None) -> Optional[MachineNode]: + if definition_id not in self: + return default + return self[definition_id] + ## Returns whether we've already cached this definition's node. # \param definition_id The definition that we may have cached. # \return ``True`` if it's cached. From c698938c60fb95ac064e1b5926425357114097d3 Mon Sep 17 00:00:00 2001 From: Ghostkeeper Date: Thu, 10 Oct 2019 15:22:52 +0200 Subject: [PATCH 10/27] Remove debug print --- cura/Settings/MachineManager.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/cura/Settings/MachineManager.py b/cura/Settings/MachineManager.py index 4dc8e672aa..1bbe2ad72a 100755 --- a/cura/Settings/MachineManager.py +++ b/cura/Settings/MachineManager.py @@ -221,8 +221,6 @@ class MachineManager(QObject): ## Triggered when the global container stack is changed in CuraApplication. def _onGlobalContainerChanged(self) -> None: - import traceback - traceback.print_stack() if self._global_container_stack: try: self._global_container_stack.containersChanged.disconnect(self._onContainersChanged) From 38e723b51cfe6ff0e8fbe3dcd4d43d43fd17d0e6 Mon Sep 17 00:00:00 2001 From: Ghostkeeper Date: Thu, 10 Oct 2019 15:32:14 +0200 Subject: [PATCH 11/27] Fix loading GlobalStacks on different thread The findContainerStacks() will list all container stacks and lazily load them. However this lazy loading is done on the thread that's calling it. The lazy loading will create GlobalStack objects, which are QObjects. The QML code then can't access those QObjects because they are created on different threads. So now instead we'll do the find query on the main thread but all the rest on the background thread. Contributes to issue CURA-6973. --- cura/Machines/ContainerTree.py | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/cura/Machines/ContainerTree.py b/cura/Machines/ContainerTree.py index 9e31d1ac32..4793945863 100644 --- a/cura/Machines/ContainerTree.py +++ b/cura/Machines/ContainerTree.py @@ -16,6 +16,7 @@ import time if TYPE_CHECKING: from cura.Machines.QualityGroup import QualityGroup from cura.Machines.QualityChangesGroup import QualityChangesGroup + from UM.Settings.ContainerStack import ContainerStack ## This class contains a look-up tree for which containers are available at @@ -75,7 +76,8 @@ class ContainerTree: ## Ran after completely starting up the application. def _onStartupFinished(self): - JobQueue.getInstance().add(self.MachineNodeLoadJob(self)) + currently_added = ContainerRegistry.getInstance().findContainerStacks() # Find all currently added global stacks. + JobQueue.getInstance().add(self.MachineNodeLoadJob(self, currently_added)) ## Dictionary-like object that contains the machines. # @@ -132,16 +134,19 @@ class ContainerTree: # \param tree_root The container tree instance. This cannot be # obtained through the singleton static function since the instance # may not yet be constructed completely. - def __init__(self, tree_root: "ContainerTree"): + # \param container_stacks All of the stacks to pre-load the container + # trees for. This needs to be provided from here because the stacks + # need to be constructed on the main thread because they are QObject. + def __init__(self, tree_root: "ContainerTree", container_stacks: List["ContainerStack"]): self.tree_root = tree_root + self.container_stacks = container_stacks super().__init__() ## Starts the background task. # # The ``JobQueue`` will schedule this on a different thread. def run(self) -> None: - currently_added = ContainerRegistry.getInstance().findContainerStacks() # Find all currently added global stacks. - for stack in currently_added: + for stack in self.container_stacks: time.sleep(0.5) if not isinstance(stack, GlobalStack): continue From ab4fade01764660589c86a5b19a0825cb1e8e2b4 Mon Sep 17 00:00:00 2001 From: Ghostkeeper Date: Thu, 10 Oct 2019 15:36:19 +0200 Subject: [PATCH 12/27] Fix check if definition with ID exists It's not an InstanceContainer but a DefinitionContainer. Also, when checking with the ID, it'll short circuit to the dictionary look up by ID. Then it's faster to directly check without specifying what type of container it is to prevent another nested function call. Contributes to issue CURA-6793. --- cura/Machines/ContainerTree.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cura/Machines/ContainerTree.py b/cura/Machines/ContainerTree.py index 4793945863..2901ee29a4 100644 --- a/cura/Machines/ContainerTree.py +++ b/cura/Machines/ContainerTree.py @@ -91,7 +91,7 @@ class ContainerTree: # \param definition_id The definition to look for. # \return Whether or not a printer definition exists with that name. def __contains__(self, definition_id: str) -> bool: - return len(ContainerRegistry.getInstance().findInstanceContainersMetadata(id = definition_id)) == 0 + return len(ContainerRegistry.getInstance().findContainersMetadata(id = definition_id)) == 0 ## Returns a machine node for the specified definition ID. # From d3a3be832fe5e0279b39520b801be6c22698e8b3 Mon Sep 17 00:00:00 2001 From: Ghostkeeper Date: Thu, 10 Oct 2019 16:02:13 +0200 Subject: [PATCH 13/27] Fix QML errors when extruder list is temporarily 0 Found during work on CURA-6793. --- resources/qml/Menus/MaterialMenu.qml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/resources/qml/Menus/MaterialMenu.qml b/resources/qml/Menus/MaterialMenu.qml index a574e240d3..94a5262b72 100644 --- a/resources/qml/Menus/MaterialMenu.qml +++ b/resources/qml/Menus/MaterialMenu.qml @@ -13,8 +13,8 @@ Menu title: catalog.i18nc("@label:category menu label", "Material") property int extruderIndex: 0 - property string currentRootMaterialId: Cura.MachineManager.currentRootMaterialId[extruderIndex] - property string activeMaterialId: Cura.MachineManager.activeMachine.extruderList[extruderIndex].material.id + property var currentRootMaterialId: Cura.MachineManager.currentRootMaterialId[extruderIndex] + property string activeMaterialId: Cura.MachineManager.activeMachine.extruderList[extruderIndex] ? Cura.MachineManager.activeMachine.extruderList[extruderIndex].material.id : "" property bool updateModels: true Cura.FavoriteMaterialsModel { From 22d874d932e4fb9ee39ff7fea04347bfa9fedf82 Mon Sep 17 00:00:00 2001 From: Ghostkeeper Date: Thu, 10 Oct 2019 16:02:47 +0200 Subject: [PATCH 14/27] Code style Found during work on CURA-6973. --- tests/Machines/TestContainerTree.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/Machines/TestContainerTree.py b/tests/Machines/TestContainerTree.py index 6458f0f429..9a088cc80c 100644 --- a/tests/Machines/TestContainerTree.py +++ b/tests/Machines/TestContainerTree.py @@ -37,7 +37,7 @@ def application(): def test_containerTreeInit(container_registry): - with patch("UM.Settings.ContainerRegistry.ContainerRegistry.getInstance", MagicMock(return_value=container_registry)): + with patch("UM.Settings.ContainerRegistry.ContainerRegistry.getInstance", MagicMock(return_value = container_registry)): container_tree = ContainerTree() assert "machine_1" in container_tree.machines @@ -45,7 +45,7 @@ def test_containerTreeInit(container_registry): def test_newMachineAdded(container_registry): - mocked_definition_container = MagicMock(spec=DefinitionContainer) + mocked_definition_container = MagicMock(spec = DefinitionContainer) mocked_definition_container.getId = MagicMock(return_value = "machine_3") with patch("UM.Settings.ContainerRegistry.ContainerRegistry.getInstance", MagicMock(return_value = container_registry)): From f6d83d7a6bcb6f76fbc96a687918fa5cf0668f3f Mon Sep 17 00:00:00 2001 From: Ghostkeeper Date: Thu, 10 Oct 2019 16:10:19 +0200 Subject: [PATCH 15/27] Fix sign of __contains__ Should've been the other way around! Contributes to issue CURA-6793. --- cura/Machines/ContainerTree.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cura/Machines/ContainerTree.py b/cura/Machines/ContainerTree.py index 2901ee29a4..788f30e9e1 100644 --- a/cura/Machines/ContainerTree.py +++ b/cura/Machines/ContainerTree.py @@ -91,7 +91,7 @@ class ContainerTree: # \param definition_id The definition to look for. # \return Whether or not a printer definition exists with that name. def __contains__(self, definition_id: str) -> bool: - return len(ContainerRegistry.getInstance().findContainersMetadata(id = definition_id)) == 0 + return len(ContainerRegistry.getInstance().findContainersMetadata(id = definition_id)) > 0 ## Returns a machine node for the specified definition ID. # From ce4c5a1c936451758251defdcb910834484d5081 Mon Sep 17 00:00:00 2001 From: Ghostkeeper Date: Tue, 15 Oct 2019 15:01:45 +0200 Subject: [PATCH 16/27] Remove unnecessary listening to switching extruder tabs Also fix unnecessary emitting of switching extruder tabs. This should improve performance a lot. I tested a lot of things and am convinced that it didn't break anything. But the automated GUI tests and QA team should be the final arbiters of that... Contributes to issue CURA-6793. --- cura/BuildVolume.py | 4 ---- cura/Machines/Models/BaseMaterialsModel.py | 2 +- cura/Machines/Models/IntentCategoryModel.py | 5 ++++- cura/Machines/Models/QualityProfilesDropDownMenuModel.py | 3 ++- cura/Settings/MachineManager.py | 8 ++------ 5 files changed, 9 insertions(+), 13 deletions(-) diff --git a/cura/BuildVolume.py b/cura/BuildVolume.py index a640760471..bc8c806699 100755 --- a/cura/BuildVolume.py +++ b/cura/BuildVolume.py @@ -126,10 +126,6 @@ class BuildVolume(SceneNode): # Therefore this works. self._machine_manager.activeQualityChanged.connect(self._onStackChanged) - # This should also ways work, and it is semantically more correct, - # but it does not update the disallowed areas after material change - self._machine_manager.activeStackChanged.connect(self._onStackChanged) - # Enable and disable extruder self._machine_manager.extruderChanged.connect(self.updateNodeBoundaryCheck) diff --git a/cura/Machines/Models/BaseMaterialsModel.py b/cura/Machines/Models/BaseMaterialsModel.py index fc2f90f1e4..c7dbadf54a 100644 --- a/cura/Machines/Models/BaseMaterialsModel.py +++ b/cura/Machines/Models/BaseMaterialsModel.py @@ -42,7 +42,7 @@ class BaseMaterialsModel(ListModel): self._machine_manager.globalContainerChanged.connect(self._updateExtruderStack) self._updateExtruderStack() - # Update this model when switching machines, when adding materials or changing their metadata. + # Update this model when switching machines or tabs, when adding materials or changing their metadata. self._machine_manager.activeStackChanged.connect(self._update) ContainerTree.getInstance().materialsChanged.connect(self._materialsListChanged) self._application.getMaterialManagementModel().favoritesChanged.connect(self._update) diff --git a/cura/Machines/Models/IntentCategoryModel.py b/cura/Machines/Models/IntentCategoryModel.py index c436f94421..da8044addd 100644 --- a/cura/Machines/Models/IntentCategoryModel.py +++ b/cura/Machines/Models/IntentCategoryModel.py @@ -48,7 +48,10 @@ class IntentCategoryModel(ListModel): ContainerRegistry.getInstance().containerAdded.connect(self._onContainerChange) ContainerRegistry.getInstance().containerRemoved.connect(self._onContainerChange) - cura.CuraApplication.CuraApplication.getInstance().getMachineManager().activeStackChanged.connect(self.update) + machine_manager = cura.CuraApplication.CuraApplication.getInstance().getMachineManager() + machine_manager.activeMaterialChanged.connect(self.update) + machine_manager.activeVariantChanged.connect(self.update) + machine_manager.extruderChanged.connect(self.update) self.update() diff --git a/cura/Machines/Models/QualityProfilesDropDownMenuModel.py b/cura/Machines/Models/QualityProfilesDropDownMenuModel.py index 9bf1cc08a8..9e1dfc4949 100644 --- a/cura/Machines/Models/QualityProfilesDropDownMenuModel.py +++ b/cura/Machines/Models/QualityProfilesDropDownMenuModel.py @@ -40,7 +40,8 @@ class QualityProfilesDropDownMenuModel(ListModel): application.globalContainerStackChanged.connect(self._onChange) machine_manager.activeQualityGroupChanged.connect(self._onChange) - machine_manager.activeStackChanged.connect(self._onChange) + machine_manager.activeMaterialChanged.connect(self._onChange) + machine_manager.activeVariantChanged.connect(self._onChange) machine_manager.extruderChanged.connect(self._onChange) self._layer_height_unit = "" # This is cached diff --git a/cura/Settings/MachineManager.py b/cura/Settings/MachineManager.py index 5985234442..84e5495e7f 100755 --- a/cura/Settings/MachineManager.py +++ b/cura/Settings/MachineManager.py @@ -139,8 +139,8 @@ class MachineManager(QObject): activeVariantChanged = pyqtSignal() activeQualityChanged = pyqtSignal() activeIntentChanged = pyqtSignal() - activeStackChanged = pyqtSignal() # Emitted whenever the active extruder stack is changed (ie: when changing between extruders, changing a profile, but not when changing a value) - extruderChanged = pyqtSignal() + activeStackChanged = pyqtSignal() # Emitted whenever the active extruder stack is changed (ie: when switching the active extruder tab or changing between printers) + extruderChanged = pyqtSignal() # Emitted whenever an extruder is activated or deactivated or the default extruder changes. activeStackValueChanged = pyqtSignal() # Emitted whenever a value inside the active stack is changed. activeStackValidationChanged = pyqtSignal() # Emitted whenever a validation inside active container is changed @@ -269,11 +269,7 @@ class MachineManager(QObject): def _onActiveExtruderStackChanged(self) -> None: self.blurSettings.emit() # Ensure no-one has focus. - if self._active_container_stack is not None: - self._active_container_stack.pyqtContainersChanged.disconnect(self.activeStackChanged) # Unplug from the old one. self._active_container_stack = ExtruderManager.getInstance().getActiveExtruderStack() - if self._active_container_stack is not None: - self._active_container_stack.pyqtContainersChanged.connect(self.activeStackChanged) # Plug into the new one. def __emitChangedSignals(self) -> None: self.activeQualityChanged.emit() From 401390209f93b77a67245e64ba0e5787230b5ec7 Mon Sep 17 00:00:00 2001 From: Ghostkeeper Date: Wed, 16 Oct 2019 10:59:58 +0200 Subject: [PATCH 17/27] Robustness against missing approximate diameter Pretend like that material doesn't exist then. Contributes to issue CURA-6793. --- cura/Machines/Models/BaseMaterialsModel.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cura/Machines/Models/BaseMaterialsModel.py b/cura/Machines/Models/BaseMaterialsModel.py index c7dbadf54a..edcdacb8a1 100644 --- a/cura/Machines/Models/BaseMaterialsModel.py +++ b/cura/Machines/Models/BaseMaterialsModel.py @@ -141,7 +141,7 @@ class BaseMaterialsModel(ListModel): nozzle_name = extruder_stack.variant.getName() materials = ContainerTree.getInstance().machines[global_stack.definition.getId()].variants[nozzle_name].materials approximate_material_diameter = extruder_stack.getApproximateMaterialDiameter() - self._available_materials = {key: material for key, material in materials.items() if float(material.container.getMetaDataEntry("approximate_diameter")) == approximate_material_diameter} + self._available_materials = {key: material for key, material in materials.items() if float(material.container.getMetaDataEntry("approximate_diameter", -1)) == approximate_material_diameter} ## This method is used by all material models in the beginning of the # _update() method in order to prevent errors. It's the same in all models From 9f84304829f88d5932031712de5c92eea684f777 Mon Sep 17 00:00:00 2001 From: Ghostkeeper Date: Wed, 16 Oct 2019 12:56:05 +0200 Subject: [PATCH 18/27] Defensive against missing global qualities This is just something I encountered. Could be that some profiles got corrupted. In my case, I had a bug in Uranium that I fixed later. Contributes to issue CURA-6793. --- cura/Machines/MachineNode.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/cura/Machines/MachineNode.py b/cura/Machines/MachineNode.py index 29968512ce..72652f2987 100644 --- a/cura/Machines/MachineNode.py +++ b/cura/Machines/MachineNode.py @@ -177,5 +177,7 @@ class MachineNode(ContainerNode): global_qualities = container_registry.findInstanceContainersMetadata(type = "quality", definition = self.quality_definition, global_quality = "True") # First try specific to this printer. if len(global_qualities) == 0: # This printer doesn't override the global qualities. global_qualities = container_registry.findInstanceContainersMetadata(type = "quality", definition = "fdmprinter", global_quality = "True") # Otherwise pick the global global qualities. + if len(global_qualities) == 0: # There are no global qualities either?! Something went very wrong, but we'll not crash and properly fill the tree. + global_qualities = [cura.CuraApplication.CuraApplication.getInstance().empty_quality_container.getMetaData()] for global_quality in global_qualities: self.global_qualities[global_quality["quality_type"]] = QualityNode(global_quality["id"], parent = self) From 7348c70af6e7453b7a2f69bbccde65e47d7370c5 Mon Sep 17 00:00:00 2001 From: Ghostkeeper Date: Wed, 16 Oct 2019 13:56:23 +0200 Subject: [PATCH 19/27] Don't get whole container to just get metadata A performance improvement, especially in the material models. Contributes to issue CURA-6793. --- cura/Machines/MaterialManager.py | 2 +- cura/Machines/Models/BaseMaterialsModel.py | 6 +++--- cura/Machines/Models/FavoriteMaterialsModel.py | 2 +- cura/Machines/Models/GenericMaterialsModel.py | 4 ++-- cura/Machines/Models/MaterialBrandsModel.py | 6 +++--- cura/Machines/QualityGroup.py | 4 ++-- cura/Settings/ContainerManager.py | 5 ++--- 7 files changed, 14 insertions(+), 15 deletions(-) diff --git a/cura/Machines/MaterialManager.py b/cura/Machines/MaterialManager.py index 83be6941ea..549d7bf747 100644 --- a/cura/Machines/MaterialManager.py +++ b/cura/Machines/MaterialManager.py @@ -132,7 +132,7 @@ class MaterialManager(QObject): # Fetch the available materials (ContainerNode) for the current active machine and extruder setup. materials = self.getAvailableMaterials(machine.definition.getId(), nozzle_name) compatible_material_diameter = extruder_stack.getApproximateMaterialDiameter() - result = {key: material for key, material in materials.items() if material.container and float(material.container.getMetaDataEntry("approximate_diameter")) == compatible_material_diameter} + result = {key: material for key, material in materials.items() if material.container and float(material.getMetaDataEntry("approximate_diameter")) == compatible_material_diameter} return result # diff --git a/cura/Machines/Models/BaseMaterialsModel.py b/cura/Machines/Models/BaseMaterialsModel.py index edcdacb8a1..368d30ae25 100644 --- a/cura/Machines/Models/BaseMaterialsModel.py +++ b/cura/Machines/Models/BaseMaterialsModel.py @@ -126,8 +126,8 @@ class BaseMaterialsModel(ListModel): if material_base_file in self._available_materials: self._update() - ## This is an abstract method that needs to be implemented by the specific - # models themselves. + ## This is an abstract method that needs to be implemented by the specific + # models themselves. def _update(self): self._favorite_ids = set(cura.CuraApplication.CuraApplication.getInstance().getPreferences().getValue("cura/favorite_materials").split(";")) @@ -141,7 +141,7 @@ class BaseMaterialsModel(ListModel): nozzle_name = extruder_stack.variant.getName() materials = ContainerTree.getInstance().machines[global_stack.definition.getId()].variants[nozzle_name].materials approximate_material_diameter = extruder_stack.getApproximateMaterialDiameter() - self._available_materials = {key: material for key, material in materials.items() if float(material.container.getMetaDataEntry("approximate_diameter", -1)) == approximate_material_diameter} + self._available_materials = {key: material for key, material in materials.items() if float(material.getMetaDataEntry("approximate_diameter", -1)) == approximate_material_diameter} ## This method is used by all material models in the beginning of the # _update() method in order to prevent errors. It's the same in all models diff --git a/cura/Machines/Models/FavoriteMaterialsModel.py b/cura/Machines/Models/FavoriteMaterialsModel.py index 255ef1dc0a..77ae4da115 100644 --- a/cura/Machines/Models/FavoriteMaterialsModel.py +++ b/cura/Machines/Models/FavoriteMaterialsModel.py @@ -27,7 +27,7 @@ class FavoriteMaterialsModel(BaseMaterialsModel): for root_material_id, container_node in self._available_materials.items(): # Do not include the materials from a to-be-removed package - if bool(container_node.container.getMetaDataEntry("removed", False)): + if bool(container_node.getMetaDataEntry("removed", False)): continue # Only add results for favorite materials diff --git a/cura/Machines/Models/GenericMaterialsModel.py b/cura/Machines/Models/GenericMaterialsModel.py index 2542a6412a..42aff40d0d 100644 --- a/cura/Machines/Models/GenericMaterialsModel.py +++ b/cura/Machines/Models/GenericMaterialsModel.py @@ -18,11 +18,11 @@ class GenericMaterialsModel(BaseMaterialsModel): for root_material_id, container_node in self._available_materials.items(): # Do not include the materials from a to-be-removed package - if bool(container_node.container.getMetaDataEntry("removed", False)): + if bool(container_node.getMetaDataEntry("removed", False)): continue # Only add results for generic materials - if container_node.container.getMetaDataEntry("brand", "unknown").lower() != "generic": + if container_node.getMetaDataEntry("brand", "unknown").lower() != "generic": continue item = self._createMaterialItem(root_material_id, container_node) diff --git a/cura/Machines/Models/MaterialBrandsModel.py b/cura/Machines/Models/MaterialBrandsModel.py index dea6982f03..184d27f390 100644 --- a/cura/Machines/Models/MaterialBrandsModel.py +++ b/cura/Machines/Models/MaterialBrandsModel.py @@ -37,18 +37,18 @@ class MaterialBrandsModel(BaseMaterialsModel): # Part 1: Generate the entire tree of brands -> material types -> spcific materials for root_material_id, container_node in self._available_materials.items(): # Do not include the materials from a to-be-removed package - if bool(container_node.container.getMetaDataEntry("removed", False)): + if bool(container_node.getMetaDataEntry("removed", False)): continue # Add brands we haven't seen yet to the dict, skipping generics - brand = container_node.container.getMetaDataEntry("brand", "") + brand = container_node.getMetaDataEntry("brand", "") if brand.lower() == "generic": continue if brand not in brand_group_dict: brand_group_dict[brand] = {} # Add material types we haven't seen yet to the dict - material_type = container_node.container.getMetaDataEntry("material", "") + material_type = container_node.getMetaDataEntry("material", "") if material_type not in brand_group_dict[brand]: brand_group_dict[brand][material_type] = [] diff --git a/cura/Machines/QualityGroup.py b/cura/Machines/QualityGroup.py index 48047974a9..58ba3acc63 100644 --- a/cura/Machines/QualityGroup.py +++ b/cura/Machines/QualityGroup.py @@ -68,7 +68,7 @@ class QualityGroup: if not node.container: Logger.log("w", "Node {0} doesn't have a container.".format(node.container_id)) return - is_experimental = parseBool(node.container.getMetaDataEntry("is_experimental", False)) + is_experimental = parseBool(node.getMetaDataEntry("is_experimental", False)) self.is_experimental |= is_experimental def setExtruderNode(self, position: int, node: "ContainerNode") -> None: @@ -78,5 +78,5 @@ class QualityGroup: if not node.container: Logger.log("w", "Node {0} doesn't have a container.".format(node.container_id)) return - is_experimental = parseBool(node.container.getMetaDataEntry("is_experimental", False)) + is_experimental = parseBool(node.getMetaDataEntry("is_experimental", False)) self.is_experimental |= is_experimental diff --git a/cura/Settings/ContainerManager.py b/cura/Settings/ContainerManager.py index af360ec235..4a4a7b64dd 100644 --- a/cura/Settings/ContainerManager.py +++ b/cura/Settings/ContainerManager.py @@ -85,7 +85,7 @@ class ContainerManager(QObject): if container_node.container is None: Logger.log("w", "Container node {0} doesn't have a container.".format(container_node.container_id)) return False - root_material_id = container_node.container.getMetaDataEntry("base_file", "") + root_material_id = container_node.getMetaDataEntry("base_file", "") container_registry = cura.CuraApplication.CuraApplication.getInstance().getContainerRegistry() if container_registry.isReadOnly(root_material_id): Logger.log("w", "Cannot set metadata of read-only container %s.", root_material_id) @@ -350,8 +350,7 @@ class ContainerManager(QObject): @pyqtSlot("QVariant") def unlinkMaterial(self, material_node: "MaterialNode") -> None: # Get the material group - if material_node.container is None: - Logger.log("w", "Material node {0} doesn't have a container.".format(material_node.container_id)) + if material_node.container is None: # Failed to lazy-load this container. return root_material_query = cura.CuraApplication.CuraApplication.getInstance().getContainerRegistry().findInstanceContainers(id = material_node.getMetaDataEntry("base_file", "")) if not root_material_query: From 5624f0a8aeb398f3bc8ac9e3a68239e925fadcc4 Mon Sep 17 00:00:00 2001 From: Ghostkeeper Date: Wed, 16 Oct 2019 14:59:46 +0200 Subject: [PATCH 20/27] Fix tests with new lazy-loading technique The new lazy loading requests different functions from the registry so we need to mock that out. Also fix the manual insertion of things into the lazy-loaded mapping. Contributes to issue CURA-6793. --- tests/Machines/TestContainerTree.py | 39 ++++------------------------- 1 file changed, 5 insertions(+), 34 deletions(-) diff --git a/tests/Machines/TestContainerTree.py b/tests/Machines/TestContainerTree.py index 9a088cc80c..c07eff340b 100644 --- a/tests/Machines/TestContainerTree.py +++ b/tests/Machines/TestContainerTree.py @@ -29,6 +29,7 @@ def createMockedStack(definition_id: str): def container_registry(): result = MagicMock() result.findContainerStacks = MagicMock(return_value = [createMockedStack("machine_1"), createMockedStack("machine_2")]) + result.findContainersMetadata = lambda id: [{"id": id}] if id in {"machine_1", "machine_2"} else [] return result @pytest.fixture @@ -40,38 +41,8 @@ def test_containerTreeInit(container_registry): with patch("UM.Settings.ContainerRegistry.ContainerRegistry.getInstance", MagicMock(return_value = container_registry)): container_tree = ContainerTree() - assert "machine_1" in container_tree.machines - assert "machine_2" in container_tree.machines - - -def test_newMachineAdded(container_registry): - mocked_definition_container = MagicMock(spec = DefinitionContainer) - mocked_definition_container.getId = MagicMock(return_value = "machine_3") - - with patch("UM.Settings.ContainerRegistry.ContainerRegistry.getInstance", MagicMock(return_value = container_registry)): - container_tree = ContainerTree() - # machine_3 shouldn't be there, as on init it wasn't in the registry - assert "machine_3" not in container_tree.machines - - # It should only react when a globalStack is added. - container_tree._machineAdded(mocked_definition_container) - assert "machine_3" not in container_tree.machines - - container_tree._machineAdded(createMockedStack("machine_3")) - assert "machine_3" in container_tree.machines - - -def test_alreadyKnownMachineAdded(container_registry): - mocked_definition_container = MagicMock(spec = DefinitionContainer) - mocked_definition_container.getId = MagicMock(return_value = "machine_2") - - with patch("UM.Settings.ContainerRegistry.ContainerRegistry.getInstance", MagicMock(return_value = container_registry)): - container_tree = ContainerTree() - assert len(container_tree.machines) == 2 - - # The ID is already there, so no machine should be added. - container_tree._machineAdded(mocked_definition_container) - assert len(container_tree.machines) == 2 + assert "machine_1" in container_tree.machines + assert "machine_2" in container_tree.machines def test_getCurrentQualityGroupsNoGlobalStack(container_registry): with patch("UM.Settings.ContainerRegistry.ContainerRegistry.getInstance", MagicMock(return_value = container_registry)): @@ -84,7 +55,7 @@ def test_getCurrentQualityGroupsNoGlobalStack(container_registry): def test_getCurrentQualityGroups(container_registry, application): with patch("UM.Settings.ContainerRegistry.ContainerRegistry.getInstance", MagicMock(return_value = container_registry)): container_tree = ContainerTree() - container_tree.machines["current_global_stack"] = MagicMock() # Mock so that we can track whether the getQualityGroups function gets called with correct parameters. + container_tree.machines.machines["current_global_stack"] = MagicMock() # Mock so that we can track whether the getQualityGroups function gets called with correct parameters. with patch("cura.CuraApplication.CuraApplication.getInstance", MagicMock(return_value = application)): result = container_tree.getCurrentQualityGroups() @@ -108,7 +79,7 @@ def test_getCurrentQualityChangesGroupsNoGlobalStack(container_registry): def test_getCurrentQualityChangesGroups(container_registry, application): with patch("UM.Settings.ContainerRegistry.ContainerRegistry.getInstance", MagicMock(return_value = container_registry)): container_tree = ContainerTree() - container_tree.machines["current_global_stack"] = MagicMock() # Mock so that we can track whether the getQualityGroups function gets called with correct parameters. + container_tree.machines.machines["current_global_stack"] = MagicMock() # Mock so that we can track whether the getQualityGroups function gets called with correct parameters. with patch("cura.CuraApplication.CuraApplication.getInstance", MagicMock(return_value = application)): result = container_tree.getCurrentQualityChangesGroups() From 8179dfc4125f51cc1897cc10d64963b3316ebcb5 Mon Sep 17 00:00:00 2001 From: Ghostkeeper Date: Wed, 16 Oct 2019 15:04:07 +0200 Subject: [PATCH 21/27] Make cache of machines a protected field We don't want to use it outside of the mapping. This mapping should be transparent. We are still using it from our tests though but that's fine. Tests are allowed to touch private fields. Contributes to issue CURA-6793. --- cura/Machines/ContainerTree.py | 12 ++++++------ tests/Machines/TestContainerTree.py | 4 ++-- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/cura/Machines/ContainerTree.py b/cura/Machines/ContainerTree.py index 788f30e9e1..bfede3d977 100644 --- a/cura/Machines/ContainerTree.py +++ b/cura/Machines/ContainerTree.py @@ -84,7 +84,7 @@ class ContainerTree: # This handles the lazy loading of MachineNodes. class MachineNodeMap: def __init__(self): - self.machines = {} + self._machines = {} ## Returns whether a printer with a certain definition ID exists. This # is regardless of whether or not the printer is loaded yet. @@ -99,12 +99,12 @@ class ContainerTree: # \param definition_id The definition to look for. # \return A machine node for that definition. def __getitem__(self, definition_id: str) -> MachineNode: - if definition_id not in self.machines: + if definition_id not in self._machines: start_time = time.time() - self.machines[definition_id] = MachineNode(definition_id) - self.machines[definition_id].materialsChanged.connect(ContainerTree.getInstance().materialsChanged) + self._machines[definition_id] = MachineNode(definition_id) + self._machines[definition_id].materialsChanged.connect(ContainerTree.getInstance().materialsChanged) Logger.log("d", "Adding container tree for {definition_id} took {duration} seconds.".format(definition_id = definition_id, duration = time.time() - start_time)) - return self.machines[definition_id] + return self._machines[definition_id] ## Gets a machine node for the specified definition ID, with default. # @@ -125,7 +125,7 @@ class ContainerTree: # \param definition_id The definition that we may have cached. # \return ``True`` if it's cached. def is_loaded(self, definition_id: str) -> bool: - return definition_id in self.machines + return definition_id in self._machines ## Pre-loads all currently added printers as a background task so that # switching printers in the interface is faster. diff --git a/tests/Machines/TestContainerTree.py b/tests/Machines/TestContainerTree.py index c07eff340b..ef11d9acc0 100644 --- a/tests/Machines/TestContainerTree.py +++ b/tests/Machines/TestContainerTree.py @@ -55,7 +55,7 @@ def test_getCurrentQualityGroupsNoGlobalStack(container_registry): def test_getCurrentQualityGroups(container_registry, application): with patch("UM.Settings.ContainerRegistry.ContainerRegistry.getInstance", MagicMock(return_value = container_registry)): container_tree = ContainerTree() - container_tree.machines.machines["current_global_stack"] = MagicMock() # Mock so that we can track whether the getQualityGroups function gets called with correct parameters. + container_tree.machines._machines["current_global_stack"] = MagicMock() # Mock so that we can track whether the getQualityGroups function gets called with correct parameters. with patch("cura.CuraApplication.CuraApplication.getInstance", MagicMock(return_value = application)): result = container_tree.getCurrentQualityGroups() @@ -79,7 +79,7 @@ def test_getCurrentQualityChangesGroupsNoGlobalStack(container_registry): def test_getCurrentQualityChangesGroups(container_registry, application): with patch("UM.Settings.ContainerRegistry.ContainerRegistry.getInstance", MagicMock(return_value = container_registry)): container_tree = ContainerTree() - container_tree.machines.machines["current_global_stack"] = MagicMock() # Mock so that we can track whether the getQualityGroups function gets called with correct parameters. + container_tree.machines._machines["current_global_stack"] = MagicMock() # Mock so that we can track whether the getQualityGroups function gets called with correct parameters. with patch("cura.CuraApplication.CuraApplication.getInstance", MagicMock(return_value = application)): result = container_tree.getCurrentQualityChangesGroups() From 87f9c6d7ccfa6d8e7daaff2d9b610a02e384c842 Mon Sep 17 00:00:00 2001 From: Ghostkeeper Date: Wed, 16 Oct 2019 14:11:29 +0000 Subject: [PATCH 22/27] Add typing for MachineNodeMap's constructor Contributes to issue CURA-6793. Co-Authored-By: Jaime van Kessel --- cura/Machines/ContainerTree.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/cura/Machines/ContainerTree.py b/cura/Machines/ContainerTree.py index bfede3d977..699c621fbe 100644 --- a/cura/Machines/ContainerTree.py +++ b/cura/Machines/ContainerTree.py @@ -83,7 +83,7 @@ class ContainerTree: # # This handles the lazy loading of MachineNodes. class MachineNodeMap: - def __init__(self): + def __init__(self) -> None: self._machines = {} ## Returns whether a printer with a certain definition ID exists. This @@ -152,4 +152,4 @@ class ContainerTree: continue definition_id = stack.definition.getId() if not self.tree_root.machines.is_loaded(definition_id): - _ = self.tree_root.machines[definition_id] \ No newline at end of file + _ = self.tree_root.machines[definition_id] From f3736f0576085ca22c22942a28774d41f75229d8 Mon Sep 17 00:00:00 2001 From: Ghostkeeper Date: Wed, 23 Oct 2019 07:40:12 +0000 Subject: [PATCH 23/27] Add typing for _machines Contributes to issue CURA-6793. Co-Authored-By: Jaime van Kessel --- cura/Machines/ContainerTree.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cura/Machines/ContainerTree.py b/cura/Machines/ContainerTree.py index 699c621fbe..a058eef72e 100644 --- a/cura/Machines/ContainerTree.py +++ b/cura/Machines/ContainerTree.py @@ -84,7 +84,7 @@ class ContainerTree: # This handles the lazy loading of MachineNodes. class MachineNodeMap: def __init__(self) -> None: - self._machines = {} + self._machines = {} # type: Dict[str, MachineNode] ## Returns whether a printer with a certain definition ID exists. This # is regardless of whether or not the printer is loaded yet. From f1e35907d255dbc2a148df144982f83a3eeae786 Mon Sep 17 00:00:00 2001 From: Ghostkeeper Date: Tue, 22 Oct 2019 17:57:30 +0200 Subject: [PATCH 24/27] Make MachineNodeLoadJob protected inner class It should not be used outside of this class. Contributes to issue CURA-6793. --- cura/Machines/ContainerTree.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/cura/Machines/ContainerTree.py b/cura/Machines/ContainerTree.py index a058eef72e..b4e7980734 100644 --- a/cura/Machines/ContainerTree.py +++ b/cura/Machines/ContainerTree.py @@ -77,7 +77,7 @@ class ContainerTree: ## Ran after completely starting up the application. def _onStartupFinished(self): currently_added = ContainerRegistry.getInstance().findContainerStacks() # Find all currently added global stacks. - JobQueue.getInstance().add(self.MachineNodeLoadJob(self, currently_added)) + JobQueue.getInstance().add(self._MachineNodeLoadJob(self, currently_added)) ## Dictionary-like object that contains the machines. # @@ -129,7 +129,7 @@ class ContainerTree: ## Pre-loads all currently added printers as a background task so that # switching printers in the interface is faster. - class MachineNodeLoadJob(Job): + class _MachineNodeLoadJob(Job): ## Creates a new background task. # \param tree_root The container tree instance. This cannot be # obtained through the singleton static function since the instance From f69360fb145bef3ea15f2e9d8e84439c39007d30 Mon Sep 17 00:00:00 2001 From: Ghostkeeper Date: Wed, 23 Oct 2019 09:42:43 +0200 Subject: [PATCH 25/27] Make lazy-load container a protected inner class Contributes to issue CURA-6793. --- cura/Machines/ContainerTree.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/cura/Machines/ContainerTree.py b/cura/Machines/ContainerTree.py index b4e7980734..5daae32826 100644 --- a/cura/Machines/ContainerTree.py +++ b/cura/Machines/ContainerTree.py @@ -39,7 +39,7 @@ class ContainerTree: return cls.__instance def __init__(self) -> None: - self.machines = self.MachineNodeMap() # Mapping from definition ID to machine nodes with lazy loading. + self.machines = self._MachineNodeMap() # Mapping from definition ID to machine nodes with lazy loading. self.materialsChanged = Signal() # Emitted when any of the material nodes in the tree got changed. cura.CuraApplication.CuraApplication.getInstance().initializationFinished.connect(self._onStartupFinished) # Start the background task to load more machine nodes after start-up is completed. @@ -82,7 +82,7 @@ class ContainerTree: ## Dictionary-like object that contains the machines. # # This handles the lazy loading of MachineNodes. - class MachineNodeMap: + class _MachineNodeMap: def __init__(self) -> None: self._machines = {} # type: Dict[str, MachineNode] From 5b70d409eda079700485c10b4e32bb4224ed45de Mon Sep 17 00:00:00 2001 From: Ghostkeeper Date: Wed, 23 Oct 2019 09:46:44 +0200 Subject: [PATCH 26/27] Improve documentation for lazy-loading background task In particular the thread switching sleep was a bit mysterious. Contributes to issue CURA_6793. --- cura/Machines/ContainerTree.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/cura/Machines/ContainerTree.py b/cura/Machines/ContainerTree.py index 5daae32826..6c4a0c3450 100644 --- a/cura/Machines/ContainerTree.py +++ b/cura/Machines/ContainerTree.py @@ -146,7 +146,10 @@ class ContainerTree: # # The ``JobQueue`` will schedule this on a different thread. def run(self) -> None: - for stack in self.container_stacks: + for stack in self.container_stacks: # Load all currently-added containers. + # Allow a thread switch after every container. + # Experimentally, sleep(0) didn't allow switching. sleep(0.1) or sleep(0.2) neither. + # We're in no hurry though. Half a second is fine. time.sleep(0.5) if not isinstance(stack, GlobalStack): continue From aed2346465d0ea8c6005871c9cd9bb7330ef15ae Mon Sep 17 00:00:00 2001 From: Ghostkeeper Date: Wed, 23 Oct 2019 09:53:22 +0200 Subject: [PATCH 27/27] Don't sleep for extruder stacks Contributes to issue CURA-6793. --- cura/Machines/ContainerTree.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/cura/Machines/ContainerTree.py b/cura/Machines/ContainerTree.py index 6c4a0c3450..c2bfabea2c 100644 --- a/cura/Machines/ContainerTree.py +++ b/cura/Machines/ContainerTree.py @@ -147,12 +147,12 @@ class ContainerTree: # The ``JobQueue`` will schedule this on a different thread. def run(self) -> None: for stack in self.container_stacks: # Load all currently-added containers. + if not isinstance(stack, GlobalStack): + continue # Allow a thread switch after every container. # Experimentally, sleep(0) didn't allow switching. sleep(0.1) or sleep(0.2) neither. # We're in no hurry though. Half a second is fine. time.sleep(0.5) - if not isinstance(stack, GlobalStack): - continue definition_id = stack.definition.getId() if not self.tree_root.machines.is_loaded(definition_id): _ = self.tree_root.machines[definition_id]