Skip to content

Fix free slicer#3970

Open
jellybean2004 wants to merge 7 commits into
mainfrom
fix_free_slicer
Open

Fix free slicer#3970
jellybean2004 wants to merge 7 commits into
mainfrom
fix_free_slicer

Conversation

@jellybean2004
Copy link
Copy Markdown
Member

@jellybean2004 jellybean2004 commented May 29, 2026

Description

This patch fixes an issue where closing a 1D slicer plot could leave its associated slicer visible on the original 2D plot, causing it to behave incorrectly when moved.

  • Added slicer ownership tracking to slicer-generated PlotterData objects using a weak reference to the owning 2D plot.
  • Updated the plot close event handling so that, when a slicer-generated 1D plot is closed, the owning 2D plot is notified.
  • Added cleanup logic in Plotter2D to remove all slicers associated with the closed 1D plot window.
  • Ensured plot-stacked slicer outputs are handled by checking all data objects in the closing 1D plot.
  • Cleared slicers from the 2D canvas and the slicer parameter window

The BoxSum (which does not have a plot but a widget) also closes cleanly with its widget, so the associated slicer is removed too.

Fixes #3969

How Has This Been Tested?

Manually tested in build and added tests

Review Checklist:

Documentation

  • There is nothing that needs documenting
  • Documentation changes are in this PR
  • There is an issue open for the documentation (link?)

Installers

  • There is a chance this will affect the installers, if so
    • Windows installer (GH artifact) has been tested (installed and worked)
    • MacOSX installer (GH artifact) has been tested (installed and worked)
    • Wheels installer (GH artifact) has been tested (installed and worked)

Licensing

  • The introduced changes comply with SasView license (BSD 3-Clause)

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

@jellybean2004 jellybean2004 changed the base branch from release_6.2.0 to main May 29, 2026 14:37
@jellybean2004 jellybean2004 marked this pull request as ready for review May 29, 2026 14:37
@jellybean2004 jellybean2004 requested a review from Copilot May 29, 2026 15:22
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 10 out of 10 changed files in this pull request and generated 2 comments.

Comment on lines +304 to +306
# Notify the slicer parameters dialog (if open) so its list stays in sync
if (slicer_widget := getattr(self, 'slicer_widget', None)):
slicer_widget.updateSlicersList()
Comment on lines +384 to +387
# Notify the slicer parameters dialog (if open) so its list stays in sync
if (slicer_widget := getattr(self, 'slicer_widget', None)):
slicer_widget.updateSlicersList()

@pkienzle
Copy link
Copy Markdown
Contributor

Some of the test failures are fixed with #3961; they may need an update to the newest sasmodels as well.

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.

Slicer turn free after its 1D plot is closed

3 participants