Ultimaker / Cura

3D printer / slicing GUI built on top of the Uranium framework
GNU Lesser General Public License v3.0
6.18k stars 2.08k forks source link

Settings lister UI should not update 600+ times when searching stuff. #13098

Open Geitde opened 2 years ago

Geitde commented 2 years ago

Is your feature request related to a problem?

The settings lister UI updates every time for every added/removed entry. It needs to be silenced to improve speed, reduce CPU load and improve user experience by reducing the flickering to zero.

Type in a keyword like "stuff" into the settings search bar on a slow computer.

The list gets cleared and it adds every found entry and updates the list on every found entry. That's not even the worst part. Now click on the X within the search text input. Cura is now adding all the missing entries and visually updates the list for every single entry. This is just stupid, takes ages and is a wastes a ton of cpu resources for nothing. When doing it right, the list update gets disabled before adding/removing entries and there is just one visible update afterwards.

I am pretty sure there is an option to disabled/silence the settings lister before updating and enable it afterwards to just avoid this mess of flickering. The user does not benefit from flickering and scrolling entries for several seconds, just because the UI gets updated.

Sometimes it is worth to test software using a slower computer to see things like this.

Thanks for the work on Cura.

Describe the solution you'd like

Disable the settings lister UI, add/remove all entries, enable the lister UI.

This will speed up UI rendering by a million times as there is only one update and not 600+ flickering ones.

Test with a slower system to reproduce.

Describe alternatives you've considered

Thats just the way it should be done in the first place.

Updating the visible UI 600 times because you change 600 entries takes ages, while with disabling/enabling it is done with just one update.

This is the perfect example why modern software is so hardware demanding. Developers should use slower computers for testing, so they notice these issue them selfs.

Affected users and/or printers

It will improve user experience

Additional information & file uploads

I tried to use a bug report for this, but even that I filled all (*) fields it failed, so consider this also being a bug report.

daniels220 commented 2 years ago

Note also per the (duplicate, oops) #13159 that I reported that this is a regression since Arachne Beta 2 (and Cura 4). These older versions not only avoided many separate redraws, but managed to still have a nice slide-and-fade animation making it very visible where settings appeared and disappeared.

Also @Geitde you may be able to go back and add the Type: Bug label after submission, and you certainly can edit the content of the issue to make it more similar to the Bug Report template.

Piezoid commented 2 years ago

I've been trying to tackle this. In my opinion this is the most annoying aspect of Cura UI, the whole software looks clumsy because of this.

I've added print tracing in every PySlot in SettingDefinitionsModel which is the QAbstractListModel implementation for the settings panel. I also modified SettingDefinitionsModel._updateVisibleRows() to do a full model reset when no currently shown entries are kept by the update. I was hopping that, at least for this case, the update would be instantaneous. But it isn't. As you can see in the following clip:

https://user-images.githubusercontent.com/3748582/191253818-2af56f70-c145-44b4-9840-8a3edeec1a45.mp4

The view continues to flicker long after endResetModel() is called.

It is more likely that the latency comes from the thousand calls made by QML to data() and rowsCount() which are rather slow (not traced in the screencast above because they flood the console). I don't understand why there is a VisibleRole role key implemented but the items are removed from the list anyway. This void any caching at the QML level such that items added to the visible list are queried again with data(). It would be better to leave all the items in the QAbstractListModel at fixed row indices and toggle their visibility then emit batched batched signal self.dataChanged.emit(self.index(row_start, 0), self.index(row_end, 0), [self.VisibleRole])

This behavior goes back to Ultimaker/Uranium#144, but PyQt updates seems to have amplified the issue.

nallath commented 2 years ago

@Piezoid, if you want to do more profiling, I recommend having a look at https://github.com/sedwards2009/cura-big-flame-graph.

As for the flickering, this is because a change in the model results in UI elements being created. As they are created asynchronously, they flicker. If you have a debug version of qt, you can also run the QMLJS debugger, which allows you to actually profile QML. There you can see that it's the inheritance button that is eating up quite a lot of resources. I've been trying to get priority for this for a while now, but so far to no avail.

So, basically, this isn't a problem that only lives in python. We have two things that we can look at; Improving the speed of the model itself and improving the speed by which we can actually create the elements.

nallath commented 2 years ago

For instance, this is the result of using kernprof (a line by line profiler) on the data function. I've let it update quite a few times by searching. From this I'm fairly sure that it's not the python code that is at fault here (but rather, re-creating objects in qml).

Total time: 0.266273 s
File: /home/jaime/Development/Uranium/UM/Settings/Models/SettingDefinitionsModel.py
Function: data at line 528

Line #      Hits         Time  Per Hit   % Time  Line Contents
==============================================================
   528                                               @profile
   529                                               def data(self, index, role):
   530                                                   """Reimplemented from QAbstractListModel"""
   531                                           
   532     17253      19229.0      1.1      7.2          if not self._container:
   533                                                       return QVariant()
   534                                           
   535     17253      24256.0      1.4      9.1          if not index.isValid():
   536                                                       return QVariant()
   537                                           
   538     17253      17605.0      1.0      6.6          if role not in self._role_names:
   539                                                       return QVariant()
   540                                           
   541     17253      11242.0      0.7      4.2          try:
   542     17253      28265.0      1.6     10.6              definition = self._definition_list[self._row_index_list[index.row()]]
   543                                                   except IndexError:
   544                                                       # Definition is not visible or completely not in the list
   545                                                       return QVariant()
   546                                           
   547     17253      15624.0      0.9      5.9          if role == self.KeyRole:
   548      3157       4999.0      1.6      1.9              return definition.key
   549     14096      10577.0      0.8      4.0          elif role == self.DepthRole:
   550       581       1847.0      3.2      0.7              return len(definition.getAncestors())
   551     13515      10338.0      0.8      3.9          elif role == self.VisibleRole:
   552       581       1546.0      2.7      0.6              return definition.key in self._visible
   553     12934       9530.0      0.7      3.6          elif role == self.ExpandedRole:
   554        63        226.0      3.6      0.1              return definition.key in self._expanded
   555                                           
   556     12871      10446.0      0.8      3.9          role_name = self._role_names[role]
   557     12871       7717.0      0.6      2.9          try:
   558     12871      56427.0      4.4     21.2              data = getattr(definition, role_name.decode())
   559                                                   except AttributeError:
   560                                                       data = ""
   561                                           
   562     12871      16288.0      1.3      6.1          if isinstance(data, collections.OrderedDict):
   563        32         24.0      0.8      0.0              result = []
   564       155        186.0      1.2      0.1              for key, value in data.items():
   565       123         90.0      0.7      0.0                  if self._i18n_catalog:
   566                                                               value = self._i18n_catalog.i18nc(definition.key + " option " + key, value)
   567                                           
   568       123        138.0      1.1      0.1                  result.append({"key": key, "value": value})
   569        32         22.0      0.7      0.0              return result
   570                                           
   571     12839      11557.0      0.9      4.3          if isinstance(data, str) and self._i18n_catalog:
   572                                                       data = self._i18n_catalog.i18nc(definition.key + " " + role_name.decode(), data)
   573                                           
   574     12839       8094.0      0.6      3.0          return data

We could consider always keeping them in memory so we wouldn't have to recreate them, but I'm a bit worried that that might cause slowdowns in other places.

Piezoid commented 2 years ago

@nallath I know how to install a debug version of Qt6 on my system, but I have no idea how to how to make the PyQt6 managed by conan to use to it. Help on this would be very much appreciated.

I'm not questioning that this is the creation of the QML objects that is being slow, but why there are (re)created at all.

nallath commented 2 years ago

Legacy reasons are my biggest guess. I have noticed that the interface itself is faster when there are less active elements, but we might be able to achieve that if we correctly disable & hide them (and always keep them in memory).

I've not tried to get a debug version of Qt6 running yet with conan. @jellespijker is the guy to help you with that.

Piezoid commented 2 years ago

I added a displayed role (the visible role is related to the user preference of setting visibility, not to the filtering or collapsing). The number of elements in the ListModel is now static. Here's the result: https://user-images.githubusercontent.com/3748582/191321715-53fa4753-d12c-442f-9812-3716b8048f9e.mp4 The QML need a lot more polishing though.

jellespijker commented 2 years ago

@Piezoid These last two sprints I attempted to compile Cura with Qt6 and PyQt6 in our Conan workflow; Unfortunately I will have to let that rest for a bit, because it was turning out to be more work then expected on Windows and the original bug that we tried to solve could be fixed in an other way. That being said, if you're not on Windows you can probably use my latest state in this. Which consist of the following steps:

Make sure you use conan 1.51.1 or higher.

It involved updating the following recipes in the CCI (conan-center-index) I have PR's open, once these are merged the steps below shouldn't be necessary anymore. But for now you would have to create Conan packages for these (M4, autoconf, automake, libtool, tcl, tk, libffi, Qt) on your local computer. Once these packages are created they are stored in you local cache and preferred over the CCI remote recipes.

clone the Ultimaker fork of CCI

git clone https://github.com/Ultimaker/conan-center-index/
cd conan-center-index

m4

git switch CURA-9595_m4_conan_v2
conan create recipes/m4/all m4/1.4.19@_/_ --build=missing --update

autoconf

git switch CURA-8831_autoconf
conan create recipes/autoconf/all autoconf/2.71@_/_ --build=missing --update

automake

git switch CURA-9575_automake_conan_v2
conan create recipes/automake/all automake/1.16.5@_/_ --build=missing --update

libtool (might not be needed to update this recipe on Unix-based systems)

git switch CURA-9633_libtool_conan_v2
conan switch recipes/libtool/all libtool/2.4.7@_/_ --build=missing --update

libffi

git switch CURA-9628-libffi_conan_v2_ready
conan switch recipes/libffi/all libffi/3.4.2@_/_ --build=missing --update

tcl

git switch CURA-9614_update_tcl_recipe
conan switch recipes/tcl/all tcl/8.6.10@_/_ --build=missing --update

tk

git switch CURA-9614_update_tcl_recipe
conan create recipes/tk/all tk/8.6.10@_/_ --build=missing --update

qt

git switch GH_12601_use_latest_vulkan_loader_sdk_version
conan create recipes/qt/6.x.x/ qt/6.3.1@_/_ --build=missing --update

After creating packages for the above mentioned dependencies. You would need to update some helper recipes. These can be found in CUI (conan-ultimaker-index) which contains recipes which are very Ultimaker specific.

git clone https://github.com/Ultimaker/conan-ultimaker-index
cd conan-ultimaker-index
git switch CURA-8831_pyqt6_recipe
conan export umbase/ 
conan create standardprojectsettings/ --build=missing --update
conan create sipbuildtool/ --build=missing --update
conan create pyprojecttoolchain --build=missing --update

Apart from your normal compiler, you would now have to install the following sip build tools and ensure that these are found on your PATH. I personally do that by installing the via pip with the python 3.x system interpreter. In my case they're then installed in a user location (so I had to add an export PATH=$PATH:/.local/bin on my Linux computer)

python -m pip install sip
python -m pip install PyQt-builder

Now we can attempt to build the PyQt6 recipe. which can still be found in the CUI repo on the CURA-8831_pyqt6_recipe branch

conan create pyqt6/6.3.1@ultimaker/testing --build=missing --update

We can then change the conandata.yml file in the Cura repo and add the pyqt6 package as a requirement.

"5.2.0-alpha":
    requirements:
        - "pyarcus/(latest)@ultimaker/testing"
        - "curaengine/(latest)@ultimaker/testing"
        - "pysavitar/(latest)@ultimaker/testing"
        - "pynest2d/(latest)@ultimaker/testing"
        - "uranium/(latest)@ultimaker/testing"
        - "fdm_materials/(latest)@ultimaker/testing"
        - "cura_binary_data/(latest)@ultimaker/testing"
        - "cpython/3.10.4"
        - "pyqt6/6.3.1@ultimaker/testing"

.....

If we then clean-up the old virtual environment and do a conan install command we would have to remove the pypi installed PyQt6 folders afterwards (didn't get around to that yet). Since the activate scripts in the virtual environment points towards paths in the conan local package Cura should still run and use the compiled PyQt6 and Qt6 packages made with Conan. But we still need to re-add the Python PyQt6-sip module.

cd Cura
rm -rf venv
conan install . --build=missing --update -g VirtualPythonEnv -o cura:devtools=True
pushd .
cd venv/lib/site-packages
# rm -rf pyqt* Best to double check this since this reply is all done by out-of-memory
popd
source venv/bin/activate
pip install PyQt6-sip
python cura_app.py

if you want to enable debug for PyQt6 and Qt6 add the following flags to the conan install command. But I myself didn't get around to try that out yet. It could very well mean that you need to make some changes to the Qt and PyQt6 recipes, to enable those flags.

conan install . --build=missing --update -g VirtualPythonEnv -o cura:devtools=True -s qt:build_type=Debug -s qt6:build_type=Debug

Now it should run with the compiled Qt6 and PyQt6, keep in mind that the above steps are still WIP and are done from branches. Feel free to contact me at:

The Slack community for cpplang also has an very active and helpful Qt and Conan channel.

Piezoid commented 2 years ago

@jellespijker Thanks for the detailed writeup! Luckily, I was able to use my system PyQt6 built with --qml-debug which happens to be ABI compatible. I simply linked it into the venv: ln -sf /usr/lib/python3.10/site-packages/PyQt6 venv/lib/python3.10/site-packages/PyQt6 Then starting Cura with -qmljsdebugger=port:9030,block allowed me to attach qtcreator profiler: image