Improve scene handling - Githubissues

letsfindaway commented 8 months ago

This issue collects several other issues related to scene management, tries to provide a common overview and propose steps for solving. The issues are:

The effects for the user are

Performance issues when loading documents with a very large number of strokes.
High memory usage up to blocking the computer when loading such heavy documents.
Crash, possibly related to deleting a page under certain circumstances.

Reasons for these issues are

Loading of all scenes when opening a document.
Scenes in the scene cache are never deleted.
Asynchronous loading is not synchronized to the synchronous use of a scene.

Steps for addressing these issues

The following proposes a step-wise approach to solve these interrelated issues.

[x] When loading a document and creating the thumbnails, only assign a scene with the thumbnail of the current page. Use the thumbnail pixmaps for all other thumbnails.
[x] When switching to another page, detach the scene from the UBDraggableLivePixmapItem so that it can be released from the cache if necessary.
[x] Make size of thumbnail buttons independent from the pixmap scaling.
[x] Limit number of scenes in UBSceneCache and add a LRU policy for removing the least recently used scene. This is a requirement for reducing the memory usage on heavy documents with many pages. Note that most parts of this are already there and have been deactivated when scenes have been used for all thumbnails. As we're now using pixmaps again for inactive pages, we can reactivate this.
[x] Refactor isA relationship of scene cache to hasA.
[x] When checking for a scene in the scene cache, also check whether the requested scene is currently scheduled for loading. If yes, then finish loading in the main thread.
[x] When background loading a scene, we have to do this on the main thread (reasons see comment below). Lets split the loading process into small pieces and execute these pieces when the main thread is idle.
[ ] When terminating the application make sure all workers are finished.

letsfindaway commented 7 months ago

Note: There is one BIG problem with asynchronous loading:

All QGraphicsItem share a global object for storing data. This is a QHash<const QGraphicsItem *, QMap<int, QVariant> > data which stores the data for all items globally. Access to this hash is not protected against concurrent access from several threads.

As building a scene involves creating many items, where each item uses this map, we cannot safely do this on any other thread than the main application thread. This problem is completely independent of the thread affinity of objects and cannot be solved by the application.

The only possibility I see is to do loading on the main thread, but to cut it into small pieces, which are executed whenever the main thread is idle. Let me say, if the UBSvgSubsetAdaptor would have a function which reads data for a single item and creates that item, and then returns, we could call this function when the main thread has nothing else to do. Such a programming pattern requires that all context of loading is held in a variable which can be passed to this function. Let's see how big this context is! Best case it is only the XML reader and the scene.

letsfindaway commented 6 months ago

See https://github.com/OpenBoard-org/OpenBoard/pull/941 for my PR to address this issue.

Actually the context needed to load a file in small steps is the UBSvgSubsetReader if we move three local variables to member variables.

letsfindaway / OpenBoard

Improve scene handling #172

Steps for addressing these issues