From 9a0264644b870b96720e98318f0702ffcc5b31d4 Mon Sep 17 00:00:00 2001 From: Jaime van Kessel Date: Fri, 14 Aug 2020 13:41:20 +0200 Subject: [PATCH 1/7] Simplify the _shouldResolve function This increases the speed of the setting resolvement. --- cura/Settings/GlobalStack.py | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/cura/Settings/GlobalStack.py b/cura/Settings/GlobalStack.py index a9164d0fb9..a254104c5b 100755 --- a/cura/Settings/GlobalStack.py +++ b/cura/Settings/GlobalStack.py @@ -256,8 +256,6 @@ class GlobalStack(CuraContainerStack): raise Exceptions.InvalidOperationError("Global stack cannot have a next stack!") - # protected: - # Determine whether or not we should try to get the "resolve" property instead of the # requested property. def _shouldResolve(self, key: str, property_name: str, context: Optional[PropertyEvaluationContext] = None) -> bool: @@ -273,10 +271,8 @@ class GlobalStack(CuraContainerStack): # track all settings that are being resolved. return False - setting_state = super().getProperty(key, "state", context = context) - if setting_state is not None and setting_state != InstanceState.Default: - # When the user has explicitly set a value, we should ignore any resolve and - # just return that value. + if self.hasUserValue(key): + # When the user has explicitly set a value, we should ignore any resolve and just return that value. return False return True From 4c00a8ff2cb8ea2bae15bd9246ac96870c8478a8 Mon Sep 17 00:00:00 2001 From: Jaime van Kessel Date: Fri, 14 Aug 2020 13:57:09 +0200 Subject: [PATCH 2/7] Add extra short-circuit for resolve --- cura/Settings/GlobalStack.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/cura/Settings/GlobalStack.py b/cura/Settings/GlobalStack.py index a254104c5b..4528fd2302 100755 --- a/cura/Settings/GlobalStack.py +++ b/cura/Settings/GlobalStack.py @@ -263,6 +263,10 @@ class GlobalStack(CuraContainerStack): # Do not try to resolve anything but the "value" property return False + if not self.definition.getProperty(key, "resolve"): + # If there isn't a resolve set for this setting, there isn't anything to do here. + return False + current_thread = threading.current_thread() if key in self._resolving_settings[current_thread.name]: # To prevent infinite recursion, if getProperty is called with the same key as From 51737dccd60ba7b9c21aa3f2f8e0b3af6a490c23 Mon Sep 17 00:00:00 2001 From: Jaime van Kessel Date: Fri, 14 Aug 2020 14:02:59 +0200 Subject: [PATCH 3/7] Don't create a context when it's not provided The rest of the functions already assume that None is an empty context --- cura/Settings/ExtruderStack.py | 14 ++++++++------ cura/Settings/GlobalStack.py | 13 +++++++------ 2 files changed, 15 insertions(+), 12 deletions(-) diff --git a/cura/Settings/ExtruderStack.py b/cura/Settings/ExtruderStack.py index bb35b336c7..2a9838c671 100644 --- a/cura/Settings/ExtruderStack.py +++ b/cura/Settings/ExtruderStack.py @@ -131,13 +131,13 @@ class ExtruderStack(CuraContainerStack): if not self._next_stack: raise Exceptions.NoGlobalStackError("Extruder {id} is missing the next stack!".format(id = self.id)) - if context is None: - context = PropertyEvaluationContext() - context.pushContainer(self) + if context: + context.pushContainer(self) if not super().getProperty(key, "settable_per_extruder", context): result = self.getNextStack().getProperty(key, property_name, context) - context.popContainer() + if context: + context.popContainer() return result limit_to_extruder = super().getProperty(key, "limit_to_extruder", context) @@ -150,13 +150,15 @@ class ExtruderStack(CuraContainerStack): try: result = self.getNextStack().extruderList[int(limit_to_extruder)].getProperty(key, property_name, context) if result is not None: - context.popContainer() + if context: + context.popContainer() return result except IndexError: pass result = super().getProperty(key, property_name, context) - context.popContainer() + if context: + context.popContainer() return result @override(CuraContainerStack) diff --git a/cura/Settings/GlobalStack.py b/cura/Settings/GlobalStack.py index 4528fd2302..da5a8546d3 100755 --- a/cura/Settings/GlobalStack.py +++ b/cura/Settings/GlobalStack.py @@ -192,7 +192,7 @@ class GlobalStack(CuraContainerStack): self._extruders[position] = extruder self.extrudersChanged.emit() Logger.log("i", "Extruder[%s] added to [%s] at position [%s]", extruder.id, self.id, position) - + @override(ContainerStack) def getProperty(self, key: str, property_name: str, context: Optional[PropertyEvaluationContext] = None) -> Any: """Overridden from ContainerStack @@ -211,9 +211,8 @@ class GlobalStack(CuraContainerStack): if not self.definition.findDefinitions(key = key): return None - if context is None: - context = PropertyEvaluationContext() - context.pushContainer(self) + if context: + context.pushContainer(self) # Handle the "resolve" property. #TODO: Why the hell does this involve threading? @@ -238,13 +237,15 @@ class GlobalStack(CuraContainerStack): if super().getProperty(key, "settable_per_extruder", context): result = self._extruders[str(limit_to_extruder)].getProperty(key, property_name, context) if result is not None: - context.popContainer() + if context: + context.popContainer() return result else: Logger.log("e", "Setting {setting} has limit_to_extruder but is not settable per extruder!", setting = key) result = super().getProperty(key, property_name, context) - context.popContainer() + if context: + context.popContainer() return result @override(ContainerStack) From 9c904f95cea9cf6f15e20111972a959326c12f03 Mon Sep 17 00:00:00 2001 From: Jaime van Kessel Date: Fri, 14 Aug 2020 14:51:46 +0200 Subject: [PATCH 4/7] Add a cache for settable_per_extruder property --- cura/Settings/CuraContainerStack.py | 12 ++++++++++++ cura/Settings/GlobalStack.py | 2 +- 2 files changed, 13 insertions(+), 1 deletion(-) diff --git a/cura/Settings/CuraContainerStack.py b/cura/Settings/CuraContainerStack.py index 4595bf3996..68371568fb 100755 --- a/cura/Settings/CuraContainerStack.py +++ b/cura/Settings/CuraContainerStack.py @@ -60,6 +60,8 @@ class CuraContainerStack(ContainerStack): import cura.CuraApplication #Here to prevent circular imports. self.setMetaDataEntry("setting_version", cura.CuraApplication.CuraApplication.SettingVersion) + self._settable_per_extruder_cache = {} + self.setDirty(False) # This is emitted whenever the containersChanged signal from the ContainerStack base class is emitted. @@ -387,6 +389,16 @@ class CuraContainerStack(ContainerStack): value = int(Application.getInstance().getMachineManager().defaultExtruderPosition) return value + def getProperty(self, key: str, property_name: str, context = None) -> Any: + if property_name == "settable_per_extruder": + # Setable per extruder isn't a value that can ever change. So once we requested it once, we can just keep + # that in memory. + if key not in self._settable_per_extruder_cache: + self._settable_per_extruder_cache[key] = super().getProperty(key, property_name, context) + return self._settable_per_extruder_cache[key] + + return super().getProperty(key, property_name, context) + class _ContainerIndexes: """Private helper class to keep track of container positions and their types.""" diff --git a/cura/Settings/GlobalStack.py b/cura/Settings/GlobalStack.py index da5a8546d3..2c7cbf5e25 100755 --- a/cura/Settings/GlobalStack.py +++ b/cura/Settings/GlobalStack.py @@ -192,7 +192,7 @@ class GlobalStack(CuraContainerStack): self._extruders[position] = extruder self.extrudersChanged.emit() Logger.log("i", "Extruder[%s] added to [%s] at position [%s]", extruder.id, self.id, position) - + @override(ContainerStack) def getProperty(self, key: str, property_name: str, context: Optional[PropertyEvaluationContext] = None) -> Any: """Overridden from ContainerStack From eee84a82bfb28babddf043602813d29a0b08ac0f Mon Sep 17 00:00:00 2001 From: Jaime van Kessel Date: Fri, 14 Aug 2020 15:15:33 +0200 Subject: [PATCH 5/7] Use exception instead of check if key is in dict Since the amount of times that the key is in there is orders of magnitude larger, it's better to catch the exception when it doesn't (as that is slightly faster) --- cura/Settings/CuraContainerStack.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/cura/Settings/CuraContainerStack.py b/cura/Settings/CuraContainerStack.py index 68371568fb..8ee190b213 100755 --- a/cura/Settings/CuraContainerStack.py +++ b/cura/Settings/CuraContainerStack.py @@ -393,9 +393,11 @@ class CuraContainerStack(ContainerStack): if property_name == "settable_per_extruder": # Setable per extruder isn't a value that can ever change. So once we requested it once, we can just keep # that in memory. - if key not in self._settable_per_extruder_cache: + try: + return self._settable_per_extruder_cache[key] + except KeyError: self._settable_per_extruder_cache[key] = super().getProperty(key, property_name, context) - return self._settable_per_extruder_cache[key] + return self._settable_per_extruder_cache[key] return super().getProperty(key, property_name, context) From bc67b057eafdc844f3a38d8ace189ff8236dfe22 Mon Sep 17 00:00:00 2001 From: Jaime van Kessel Date: Fri, 14 Aug 2020 15:19:59 +0200 Subject: [PATCH 6/7] Add missing required typing --- cura/Settings/CuraContainerStack.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/cura/Settings/CuraContainerStack.py b/cura/Settings/CuraContainerStack.py index 8ee190b213..f594ad3d0c 100755 --- a/cura/Settings/CuraContainerStack.py +++ b/cura/Settings/CuraContainerStack.py @@ -1,7 +1,7 @@ # Copyright (c) 2018 Ultimaker B.V. # Cura is released under the terms of the LGPLv3 or higher. -from typing import Any, cast, List, Optional +from typing import Any, cast, List, Optional, Dict from PyQt5.QtCore import pyqtProperty, pyqtSignal, QObject from UM.Application import Application @@ -60,7 +60,7 @@ class CuraContainerStack(ContainerStack): import cura.CuraApplication #Here to prevent circular imports. self.setMetaDataEntry("setting_version", cura.CuraApplication.CuraApplication.SettingVersion) - self._settable_per_extruder_cache = {} + self._settable_per_extruder_cache = {} # type: Dict[str, Any] self.setDirty(False) From 04c216462ab3ad1d54eb23bbe0a76e3571ac45ba Mon Sep 17 00:00:00 2001 From: Jaime van Kessel Date: Fri, 14 Aug 2020 15:41:40 +0200 Subject: [PATCH 7/7] Fix the unit test Previously we would only look at the state, but that isn't the only thing it should look at. It should override the value of a resolve if it's defined in user changes or QualityChanges. --- tests/Settings/TestGlobalStack.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/Settings/TestGlobalStack.py b/tests/Settings/TestGlobalStack.py index ab9c034e24..79d326ddae 100755 --- a/tests/Settings/TestGlobalStack.py +++ b/tests/Settings/TestGlobalStack.py @@ -410,13 +410,13 @@ def test_getPropertyInstancesBeforeResolve(global_stack): value = unittest.mock.MagicMock() #Sets just the value. value.getProperty = unittest.mock.MagicMock(side_effect = getValueProperty) - value.getMetaDataEntry = unittest.mock.MagicMock(return_value = "quality") + value.getMetaDataEntry = unittest.mock.MagicMock(return_value = "quality_changes") resolve = unittest.mock.MagicMock() #Sets just the resolve. resolve.getProperty = unittest.mock.MagicMock(side_effect = getResolveProperty) with unittest.mock.patch("cura.Settings.CuraContainerStack.DefinitionContainer", unittest.mock.MagicMock): #To guard against the type checking. global_stack.definition = resolve - global_stack.quality = value + global_stack.qualityChanges = value assert global_stack.getProperty("material_bed_temperature", "value") == 10