refactor: unified thumbnails for board and document mode#1162
Merged
kaamui merged 3 commits intoOpenBoard-org:devfrom Feb 11, 2025
Merged
refactor: unified thumbnails for board and document mode#1162kaamui merged 3 commits intoOpenBoard-org:devfrom
kaamui merged 3 commits intoOpenBoard-org:devfrom
Conversation
01ddb51 to
07f073c
Compare
- use the same thumbnail items and the same thumbnail scene for Board and Document mode. Thumbnails are just rearranged depending on the mode - create a UBDocument class for page handling. This class has functions for creating, deleting, moving and copying of pages. It takes care to call the proper functions on the UBPersistenceManager and to manage the thumbnail items. All other code shall only use these functions - use a specific, concrete derived class of an abstract UBThumbnailArranger to handle the differences between Board and Document mode - adapt the UBDocumentController and UBBoardController to the new thumbnails - adapt the UBDocumentThumbnailsView and UBBoardThumbnails view to the new thumbnails - remove cross-referencing includes in thumbnail views - remove thumbnail pixmap handling from UBDocumentContainer - fix keyboard selection in Document mode - ensure last selected thumbnails stays visible when changing size
- cache thumbnails for documents which have been opened in board mode
07f073c to
9e34c19
Compare
- reload thumbnail pixmap from file when a scene is persisted and the associated thumbnail was hidden and therefore not live updated
Collaborator
Author
|
I just added a commit fixing issue #1171 which also occurred on this branch. When the thumbnail palette is hidden, thumbnails are not live updated and are therefore outdated when the palette is shown later. We now reload a thumbnail from the thumbnail file when a scene was persisted and the thumbnail file was updated. |
Collaborator
Author
|
Note: There were fears that lazy loading of thumbnails would conflict with adding or removing pages while thumbnails are not completely loaded. A further analysis shows: this is not the case.
This works, because the renaming of files is a single, uninterruptible operation on the main thread. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR is the result of my considerations about a rework of the thumbnail architecture initially started here: letsfindaway#190.
I have also created a wiki page describing the new architecture here: https://github.com/letsfindaway/OpenBoard/wiki/New-Thumbnail-Architecture. Let me just describe the core ideas here:
UBThumbnailclass for both modes and to even use the same instances on the sameUBThumbnailScene. Switching between modes then simply means re-arranging the thumbnails. You do not have to care about synchronization at all.UBThumbnailSceneis managed by aUBDocumentclass. This class has functions to create, delete, move and copy pages and takes care to invoke theUBPersistenceManageras well as to handle the thumbnails associated with the affected pages. One could cache instances ofUBDocumenttogether with the associated thumbnails. This makes switching between documents very fast.Note Currently all
UBDocumentinstances are cached during a session. It is TBD whether a more sophisticated caching strategy is necessary and how it should look like.Edit I added a second commit implementing a ver simple caching strategy: Keep
UBDocumentinstances and therefore thumbnails for documents which have been opened in Board mode in the current session.UBThumbnailArrangerwith specific derived classes for the two modes.The resulting code is much easier to understand and to maintain. And in fact it needs 500 lines of code less than before. Also the memory consumption for thumbnails is halved, as we need only one instance for Board and Document mode. Rearrangement of thumbnails is fast (50ms for 2000 thumbnails on my computer), so the holdoff timer to reduce frequency of rearrangements is not necessary.
The
UBThumbnailSceneuses deferred loading of thumbnail. This means that the user has no waiting time when selecting a document with many pages in Document mode. They can immediately start working with the document, even if more thumbnails are still loaded in the background.I think that this PR is also a very good preparation for "Board thumbnails look like the last state of the view" and even more "Multiple documents opened side-by-side". The first idea benefits from the fact that synchronization is no longer necessary. The second one from the fact, that we have one thumbnail scene per document.
I also tested various aspects of the new implementation. Here is my test list: letsfindaway#190 (comment)
Here some more technical points:
UBDocumentclass for page handling. This class has functions for creating, deleting, moving and copying of pages. It takes care to call the proper functions on theUBPersistenceManagerand to manage the thumbnail items. All other code shall only use these functions.UBThumbnailArrangerto handle the differences between Board and Document mode.UBDocumentControllerandUBBoardControllerto the new thumbnails.UBDocumentThumbnailsViewandUBBoardThumbnailsview to the new thumbnails.UBDocumentContainer.It is probably worth to give just one example from the
UBBoardController::addScene():Before
After