From 42b72f81bb16ffa6912eb96aed29a3fd3349af56 Mon Sep 17 00:00:00 2001 From: Erwan MATHIEU Date: Wed, 13 Mar 2024 14:08:04 +0100 Subject: [PATCH 1/6] Optimize overridden calculated settings computation Instead of getting all the settings and checking whether they have a user state, just take the settings from the user changes containers, which is way faster. CURA-11475 --- cura/Settings/SettingInheritanceManager.py | 45 +++++++++++++++------- 1 file changed, 32 insertions(+), 13 deletions(-) diff --git a/cura/Settings/SettingInheritanceManager.py b/cura/Settings/SettingInheritanceManager.py index 5ae00ae271..26d0773369 100644 --- a/cura/Settings/SettingInheritanceManager.py +++ b/cura/Settings/SettingInheritanceManager.py @@ -32,6 +32,7 @@ class SettingInheritanceManager(QObject): self._global_container_stack = None # type: Optional[ContainerStack] self._settings_with_inheritance_warning = [] # type: List[str] self._active_container_stack = None # type: Optional[ExtruderStack] + self._active_containers = [] # type: List[ContainerInterface] self._update_timer = QTimer() self._update_timer.setInterval(500) @@ -101,6 +102,7 @@ class SettingInheritanceManager(QObject): new_active_stack = ExtruderManager.getInstance().getActiveExtruderStack() if not new_active_stack: self._active_container_stack = None + self._active_containers = [] return if new_active_stack != self._active_container_stack: # Check if changed @@ -114,6 +116,13 @@ class SettingInheritanceManager(QObject): self._active_container_stack.containersChanged.connect(self._onContainersChanged) self._update_timer.start() # Ensure that the settings_with_inheritance_warning list is populated. + ## Mash all containers for all the stacks together. + self._active_containers = [] + stack = self._active_container_stack + while stack: + self._active_containers.extend(stack.getContainers()) + stack = stack.getNextStack() + def _onPropertyChanged(self, key: str, property_name: str) -> None: if (property_name == "value" or property_name == "enabled") and self._global_container_stack: definitions = self._global_container_stack.definition.findDefinitions(key = key) # type: List["SettingDefinition"] @@ -171,7 +180,6 @@ class SettingInheritanceManager(QObject): def _settingIsOverwritingInheritance(self, key: str, stack: ContainerStack = None) -> bool: """Check if a setting has an inheritance function that is overwritten""" - has_setting_function = False if not stack: stack = self._active_container_stack if not stack: # No active container stack yet! @@ -179,9 +187,6 @@ class SettingInheritanceManager(QObject): if self._active_container_stack is None: return False - all_keys = self._active_container_stack.getAllKeys() - - containers = [] # type: List[ContainerInterface] has_user_state = stack.getProperty(key, "state") == InstanceState.User """Check if the setting has a user state. If not, it is never overwritten.""" @@ -189,6 +194,13 @@ class SettingInheritanceManager(QObject): if not has_user_state: return False + return self._userSettingIsOverwritingInheritance(key, stack, self._active_container_stack.getAllKeys()) + + def _userSettingIsOverwritingInheritance(self, key: str, stack: ContainerStack, all_keys) -> bool: + """Check if a setting has an inheritance function that is overwritten""" + + has_setting_function = False + # If a setting is not enabled, don't label it as overwritten (It's never visible anyway). if not stack.getProperty(key, "enabled"): return False @@ -199,12 +211,8 @@ class SettingInheritanceManager(QObject): if user_container and isinstance(user_container.getProperty(key, "value"), SettingFunction): return False - ## Mash all containers for all the stacks together. - while stack: - containers.extend(stack.getContainers()) - stack = stack.getNextStack() has_non_function_value = False - for container in containers: + for container in self._active_containers: try: value = container.getProperty(key, "value") except AttributeError: @@ -227,6 +235,7 @@ class SettingInheritanceManager(QObject): if has_setting_function: break # There is a setting function somewhere, stop looking deeper. + return has_setting_function and has_non_function_value def _update(self) -> None: @@ -237,10 +246,20 @@ class SettingInheritanceManager(QObject): return # Check all setting keys that we know of and see if they are overridden. - for setting_key in self._global_container_stack.getAllKeys(): - override = self._settingIsOverwritingInheritance(setting_key) - if override: - self._settings_with_inheritance_warning.append(setting_key) + if self._active_container_stack: + all_keys = self._active_container_stack.getAllKeys() + + # Get user settings from all stacks + user_settings_keys = set() + stack = self._active_container_stack + while stack: + user_settings_keys.update(stack.userChanges.getAllKeys()) + stack = stack.getNextStack() + + for setting_key in user_settings_keys: + override = self._userSettingIsOverwritingInheritance(setting_key, self._active_container_stack, all_keys) + if override: + self._settings_with_inheritance_warning.append(setting_key) # Check all the categories if any of their children have their inheritance overwritten. for category in self._global_container_stack.definition.findDefinitions(type = "category"): From 8308b640ca20096c8ed6ca3dc5cae7ce32698af1 Mon Sep 17 00:00:00 2001 From: Erwan MATHIEU Date: Thu, 14 Mar 2024 16:02:35 +0100 Subject: [PATCH 2/6] Revert "Optimize overridden calculated settings computation" This reverts commit 42b72f81bb16ffa6912eb96aed29a3fd3349af56. --- cura/Settings/SettingInheritanceManager.py | 45 +++++++--------------- 1 file changed, 13 insertions(+), 32 deletions(-) diff --git a/cura/Settings/SettingInheritanceManager.py b/cura/Settings/SettingInheritanceManager.py index 26d0773369..5ae00ae271 100644 --- a/cura/Settings/SettingInheritanceManager.py +++ b/cura/Settings/SettingInheritanceManager.py @@ -32,7 +32,6 @@ class SettingInheritanceManager(QObject): self._global_container_stack = None # type: Optional[ContainerStack] self._settings_with_inheritance_warning = [] # type: List[str] self._active_container_stack = None # type: Optional[ExtruderStack] - self._active_containers = [] # type: List[ContainerInterface] self._update_timer = QTimer() self._update_timer.setInterval(500) @@ -102,7 +101,6 @@ class SettingInheritanceManager(QObject): new_active_stack = ExtruderManager.getInstance().getActiveExtruderStack() if not new_active_stack: self._active_container_stack = None - self._active_containers = [] return if new_active_stack != self._active_container_stack: # Check if changed @@ -116,13 +114,6 @@ class SettingInheritanceManager(QObject): self._active_container_stack.containersChanged.connect(self._onContainersChanged) self._update_timer.start() # Ensure that the settings_with_inheritance_warning list is populated. - ## Mash all containers for all the stacks together. - self._active_containers = [] - stack = self._active_container_stack - while stack: - self._active_containers.extend(stack.getContainers()) - stack = stack.getNextStack() - def _onPropertyChanged(self, key: str, property_name: str) -> None: if (property_name == "value" or property_name == "enabled") and self._global_container_stack: definitions = self._global_container_stack.definition.findDefinitions(key = key) # type: List["SettingDefinition"] @@ -180,6 +171,7 @@ class SettingInheritanceManager(QObject): def _settingIsOverwritingInheritance(self, key: str, stack: ContainerStack = None) -> bool: """Check if a setting has an inheritance function that is overwritten""" + has_setting_function = False if not stack: stack = self._active_container_stack if not stack: # No active container stack yet! @@ -187,6 +179,9 @@ class SettingInheritanceManager(QObject): if self._active_container_stack is None: return False + all_keys = self._active_container_stack.getAllKeys() + + containers = [] # type: List[ContainerInterface] has_user_state = stack.getProperty(key, "state") == InstanceState.User """Check if the setting has a user state. If not, it is never overwritten.""" @@ -194,13 +189,6 @@ class SettingInheritanceManager(QObject): if not has_user_state: return False - return self._userSettingIsOverwritingInheritance(key, stack, self._active_container_stack.getAllKeys()) - - def _userSettingIsOverwritingInheritance(self, key: str, stack: ContainerStack, all_keys) -> bool: - """Check if a setting has an inheritance function that is overwritten""" - - has_setting_function = False - # If a setting is not enabled, don't label it as overwritten (It's never visible anyway). if not stack.getProperty(key, "enabled"): return False @@ -211,8 +199,12 @@ class SettingInheritanceManager(QObject): if user_container and isinstance(user_container.getProperty(key, "value"), SettingFunction): return False + ## Mash all containers for all the stacks together. + while stack: + containers.extend(stack.getContainers()) + stack = stack.getNextStack() has_non_function_value = False - for container in self._active_containers: + for container in containers: try: value = container.getProperty(key, "value") except AttributeError: @@ -235,7 +227,6 @@ class SettingInheritanceManager(QObject): if has_setting_function: break # There is a setting function somewhere, stop looking deeper. - return has_setting_function and has_non_function_value def _update(self) -> None: @@ -246,20 +237,10 @@ class SettingInheritanceManager(QObject): return # Check all setting keys that we know of and see if they are overridden. - if self._active_container_stack: - all_keys = self._active_container_stack.getAllKeys() - - # Get user settings from all stacks - user_settings_keys = set() - stack = self._active_container_stack - while stack: - user_settings_keys.update(stack.userChanges.getAllKeys()) - stack = stack.getNextStack() - - for setting_key in user_settings_keys: - override = self._userSettingIsOverwritingInheritance(setting_key, self._active_container_stack, all_keys) - if override: - self._settings_with_inheritance_warning.append(setting_key) + for setting_key in self._global_container_stack.getAllKeys(): + override = self._settingIsOverwritingInheritance(setting_key) + if override: + self._settings_with_inheritance_warning.append(setting_key) # Check all the categories if any of their children have their inheritance overwritten. for category in self._global_container_stack.definition.findDefinitions(type = "category"): From 5a346e3ce648c63b154c69cceec15372b2b5f34a Mon Sep 17 00:00:00 2001 From: Erwan MATHIEU Date: Mon, 18 Mar 2024 11:25:08 +0100 Subject: [PATCH 3/6] Optimized SettingInheritanceManager update Instead of getting all the settings and checking for their state, run only once through all the containers and gather only the keys having a User state. CURA-11475 --- cura/Settings/SettingInheritanceManager.py | 51 ++++++++++++---------- 1 file changed, 29 insertions(+), 22 deletions(-) diff --git a/cura/Settings/SettingInheritanceManager.py b/cura/Settings/SettingInheritanceManager.py index 5ae00ae271..4455b24bdb 100644 --- a/cura/Settings/SettingInheritanceManager.py +++ b/cura/Settings/SettingInheritanceManager.py @@ -1,6 +1,6 @@ # Copyright (c) 2017 Ultimaker B.V. # Cura is released under the terms of the LGPLv3 or higher. -from typing import List, Optional, TYPE_CHECKING +from typing import List, Optional, Set, TYPE_CHECKING from PyQt6.QtCore import QObject, QTimer, pyqtProperty, pyqtSignal from UM.FlameProfiler import pyqtSlot @@ -168,27 +168,13 @@ class SettingInheritanceManager(QObject): def settingsWithInheritanceWarning(self) -> List[str]: return self._settings_with_inheritance_warning - def _settingIsOverwritingInheritance(self, key: str, stack: ContainerStack = None) -> bool: - """Check if a setting has an inheritance function that is overwritten""" + def _userSettingIsOverwritingInheritance(self, key: str, stack: ContainerStack, all_keys: Set[str]) -> bool: + """Check if a setting known as having a User state has an inheritance function that is overwritten""" has_setting_function = False - if not stack: - stack = self._active_container_stack - if not stack: # No active container stack yet! - return False - - if self._active_container_stack is None: - return False - all_keys = self._active_container_stack.getAllKeys() containers = [] # type: List[ContainerInterface] - has_user_state = stack.getProperty(key, "state") == InstanceState.User - """Check if the setting has a user state. If not, it is never overwritten.""" - - if not has_user_state: - return False - # If a setting is not enabled, don't label it as overwritten (It's never visible anyway). if not stack.getProperty(key, "enabled"): return False @@ -229,17 +215,38 @@ class SettingInheritanceManager(QObject): break # There is a setting function somewhere, stop looking deeper. return has_setting_function and has_non_function_value + def _settingIsOverwritingInheritance(self, key: str, stack: ContainerStack = None) -> bool: + """Check if a setting has an inheritance function that is overwritten""" + + if not stack: + stack = self._active_container_stack + if not stack: # No active container stack yet! + return False + + if self._active_container_stack is None: + return False + + has_user_state = stack.getProperty(key, "state") == InstanceState.User + """Check if the setting has a user state. If not, it is never overwritten.""" + + if not has_user_state: + return False + + all_keys = self._active_container_stack.getAllKeys() + + return self._userSettingIsOverwritingInheritance(key, stack, all_keys) + def _update(self) -> None: self._settings_with_inheritance_warning = [] # Reset previous data. # Make sure that the GlobalStack is not None. sometimes the globalContainerChanged signal gets here late. - if self._global_container_stack is None: + if self._global_container_stack is None or self._active_container_stack is None: return - # Check all setting keys that we know of and see if they are overridden. - for setting_key in self._global_container_stack.getAllKeys(): - override = self._settingIsOverwritingInheritance(setting_key) - if override: + # Check all user setting keys that we know of and see if they are overridden. + all_keys = self._active_container_stack.getAllKeys() + for setting_key in self._active_container_stack.getAllKeysWithUserState(): + if self._userSettingIsOverwritingInheritance(setting_key, self._active_container_stack, all_keys): self._settings_with_inheritance_warning.append(setting_key) # Check all the categories if any of their children have their inheritance overwritten. From ce6642e9924c1421106bd604df9cdcd8ab452f1c Mon Sep 17 00:00:00 2001 From: Erwan MATHIEU Date: Mon, 18 Mar 2024 13:24:52 +0100 Subject: [PATCH 4/6] Improve UI responsiveness Treat settings to check for errors one by one instead of by batch of 10, because this can take a few 100s of milliseconds, which makes the UI choppy and barely allows for user interactions CURA-11475 --- cura/Machines/MachineErrorChecker.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cura/Machines/MachineErrorChecker.py b/cura/Machines/MachineErrorChecker.py index cc6560378d..5edee0778f 100644 --- a/cura/Machines/MachineErrorChecker.py +++ b/cura/Machines/MachineErrorChecker.py @@ -49,7 +49,7 @@ class MachineErrorChecker(QObject): self._keys_to_check = set() # type: Set[str] - self._num_keys_to_check_per_update = 10 + self._num_keys_to_check_per_update = 1 def initialize(self) -> None: self._error_check_timer.timeout.connect(self._rescheduleCheck) From d16c7a84a929379b34d70c9fbc2a884b0592370e Mon Sep 17 00:00:00 2001 From: Casper Lamboo Date: Tue, 19 Mar 2024 11:37:04 +0100 Subject: [PATCH 5/6] Remove documenting comment in middle of function CURA-11475 --- cura/Settings/SettingInheritanceManager.py | 1 - 1 file changed, 1 deletion(-) diff --git a/cura/Settings/SettingInheritanceManager.py b/cura/Settings/SettingInheritanceManager.py index 4455b24bdb..5004c0efb5 100644 --- a/cura/Settings/SettingInheritanceManager.py +++ b/cura/Settings/SettingInheritanceManager.py @@ -227,7 +227,6 @@ class SettingInheritanceManager(QObject): return False has_user_state = stack.getProperty(key, "state") == InstanceState.User - """Check if the setting has a user state. If not, it is never overwritten.""" if not has_user_state: return False From 70f18c11431b777623dd12d5ddd71d6f2d7ff002 Mon Sep 17 00:00:00 2001 From: Erwan MATHIEU Date: Tue, 19 Mar 2024 13:00:26 +0100 Subject: [PATCH 6/6] Apply code consistency suggestions CURA-11475 --- cura/Settings/SettingInheritanceManager.py | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/cura/Settings/SettingInheritanceManager.py b/cura/Settings/SettingInheritanceManager.py index 5004c0efb5..a49aa254fe 100644 --- a/cura/Settings/SettingInheritanceManager.py +++ b/cura/Settings/SettingInheritanceManager.py @@ -168,7 +168,7 @@ class SettingInheritanceManager(QObject): def settingsWithInheritanceWarning(self) -> List[str]: return self._settings_with_inheritance_warning - def _userSettingIsOverwritingInheritance(self, key: str, stack: ContainerStack, all_keys: Set[str]) -> bool: + def _userSettingIsOverwritingInheritance(self, key: str, stack: ContainerStack, all_keys: Set[str] = set()) -> bool: """Check if a setting known as having a User state has an inheritance function that is overwritten""" has_setting_function = False @@ -180,11 +180,14 @@ class SettingInheritanceManager(QObject): return False user_container = stack.getTop() - """Also check if the top container is not a setting function (this happens if the inheritance is restored).""" + # Also check if the top container is not a setting function (this happens if the inheritance is restored). if user_container and isinstance(user_container.getProperty(key, "value"), SettingFunction): return False + if not all_keys: + all_keys = self._active_container_stack.getAllKeys() + ## Mash all containers for all the stacks together. while stack: containers.extend(stack.getContainers()) @@ -231,9 +234,7 @@ class SettingInheritanceManager(QObject): if not has_user_state: return False - all_keys = self._active_container_stack.getAllKeys() - - return self._userSettingIsOverwritingInheritance(key, stack, all_keys) + return self._userSettingIsOverwritingInheritance(key, stack) def _update(self) -> None: self._settings_with_inheritance_warning = [] # Reset previous data.