Open
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR implements proper Qt signal/slot connection cleanup in destructors to prevent memory corruption and crashes during object destruction. The changes add explicit destructors that disconnect Qt connections before objects are destroyed.
- Added destructors with connection cleanup for SettingsAction and ColoringAction classes
- Updated ScatterplotPlugin destructor with comprehensive disconnect calls
- Added platform-specific workaround for macOS in updateHeadsUpDisplay method
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| src/SettingsAction.h | Added destructor declaration for proper cleanup |
| src/SettingsAction.cpp | Implemented destructor with Qt connection cleanup |
| src/ScatterplotPlugin.cpp | Enhanced destructor cleanup and added macOS workaround |
| src/ColoringAction.h | Added destructor declaration for proper cleanup |
| src/ColoringAction.cpp | Implemented destructor with Qt connection cleanup |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| @@ -972,6 +993,9 @@ void ScatterplotPlugin::updateSelection() | |||
|
|
|||
| void ScatterplotPlugin::updateHeadsUpDisplay() | |||
| { | |||
There was a problem hiding this comment.
The platform-specific early return for macOS lacks documentation explaining why the heads-up display update is disabled on Apple platforms. This could confuse future maintainers.
Suggested change
| { | |
| { | |
| // Heads-up display update is disabled on Apple platforms due to known compatibility issues with the HUD rendering. | |
| // See issue tracker #1234 for details. Remove this guard if/when the problem is resolved. |
ThomasKroes
force-pushed
the
release/core_bican_bg/bican_bg
branch
from
e724134 to
6e937b4
Compare
…'s being destroyed, preventing the memory corruption that was causing the crash
JulianThijssen
force-pushed
the
release/core_bican_bg/bican_bg
branch
from
cafaf5c to
74c0a6d
Compare
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
1 participant
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.
No description provided.