Fix crash deleting scene with media item#1119
Merged
kaamui merged 2 commits intoOpenBoard-org:devfrom Sep 30, 2024
Merged
Conversation
- remove code from ~UBGraphicsMediaItem - mMediaObject is a child and automatically deleted - no need to call stop before deletion - this avoids emitting the playbackStateChanged signal during deconstruction of the object
- do not accept a drag operation when dragging an arbitrary object to the board thumbnails view
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 fixes two crashes in two commits.
Crash when deleting a scene with a video item
The first one is a crash occurring with Qt 6 when a scene containing a video item is deleted.
~UBGraphicsMediaItem()was callingstop()on theQMediaPlayer, which is a child object of the media item.playbackStateChangedsignal to be emitted.UBGraphicsVideoItem::mediaStateChanged.UBGraphicsVideoItemis a class derived fromUBGraphicsMediaItem.UBGraphicsVideoItem`: The destructors are invoked starting from the derived class and then up to the base class.~UBGraphicsVideoItem()is invoked. There is no destructor defined, so the default destructor is used. After that, the object looses its property of being an object of that class.~UBGraphicsMediaItem()destructor is invoked, which was calling thestop()function and emitting the signal as explained above.UBGraphicsVideoItem. This part of the object was however already destructed.This sequence lead to the crash in Qt 6. In Qt 5 a quite similar sequence was executed, but for some reason this did not result in a crash.
Analyzing the code I found out that the complete destructor of
UBGraphicsMediaItemwas not necessary:QMediaPlayerobject explicitly, as it is a child object ofUBGraphicsMediaItemand is therefore automatically deleted.So my solution is to completely remove the code from that destructor.
Crash when dragging an arbitrary object to the board thumbnail palette
During my tests I discovered another crash. I accidentally dragged an object from the library palette to the thumbnail palette. As soon as the mouse is over the palette, OpenBoard crashed.
The thumbnail palette implements Drag-and-drop to allow reordering of pages. If you long click on a page, then you initiate a DnD operation and can drop the page at another place. There is no code which would handle dragging any other object to the palette.
In the code the starting point for the DnD operation is
UBBoardThumbnailsView::dragEnterEventwhich is invoked, when the cursor first enters the drop target area. Here it is decided, whether an object can be dropped on this target. The code here first checked whether the source area is the same as the target area. This is the case when dragging pages within the thumbnail palette. If this is the case, then a move operation is initiated.In all other cases however the code simply accepted any proposed action. It was however not prepared to process that later, which lead to the crash.
The problem is solved by removing that code accepting the drop of foreign objects.
This is my first PR which solves two severe problems by just removing some lines of code ;-)