Skip to content

Fix crash deleting scene with media item#1119

Merged
kaamui merged 2 commits intoOpenBoard-org:devfrom
letsfindaway:fix-crash-media-delete
Sep 30, 2024
Merged

Fix crash deleting scene with media item#1119
kaamui merged 2 commits intoOpenBoard-org:devfrom
letsfindaway:fix-crash-media-delete

Conversation

@letsfindaway
Copy link
Copy Markdown
Collaborator

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.

  • The destructor ~UBGraphicsMediaItem() was calling stop() on the QMediaPlayer, which is a child object of the media item.
  • This again caused the playbackStateChanged signal to be emitted.
  • This signal was connected to UBGraphicsVideoItem::mediaStateChanged. UBGraphicsVideoItem is a class derived from UBGraphicsMediaItem.
  • Now what happens during destruction of such a UBGraphicsVideoItem`: The destructors are invoked starting from the derived class and then up to the base class.
  • First ~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.
  • Then the ~UBGraphicsMediaItem() destructor is invoked, which was calling the stop() function and emitting the signal as explained above.
  • However the destination of the signal was a function of 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 UBGraphicsMediaItem was not necessary:

  • It is not necessary to stop the player immediately before it will be deleted. It will stop anyway when deleted.
  • It is not necessary to delete the QMediaPlayer object explicitly, as it is a child object of UBGraphicsMediaItem and 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::dragEnterEvent which 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 ;-)

- 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
@kaamui kaamui merged commit 37b5bc0 into OpenBoard-org:dev Sep 30, 2024
@letsfindaway letsfindaway deleted the fix-crash-media-delete branch September 30, 2024 12:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants