Safe flag#3910
Conversation
|
Hi @pkienzle! Would you be happy to review this PR? |
pkienzle
left a comment
There was a problem hiding this comment.
I think there is a simpler way to do things. The problem we are trying to avoid is that setting the value in the model parameter box from the interactor triggers the same callback as typing the value in the box.
Instead of using the update_model flag we may be able to block the signal in SlicerModel.setModelFromParams using blocked = self._model.blockSignals(True) in a try-block with self._model.blockSignals(blocked) in the finally-block.
blockSignals is already being used in MultiSlicerBase._synchronized_movend, though without the try...finally structure. This can probably be removed if we block signals in SlicerModel.setModelFromParams.
d3b034d to
1e20015
Compare
|
Hi @pkienzle! Thank you for your review, and sorry for taking time to get back on this. I have implemented the changes you suggested, which include using |
92f403a to
8ff029c
Compare
|
Slicer is not updating from dialog. Try showing a box slicer and typing values for parameters into the dialog. I find that the box does not update until the cursor hovers on one of the slicer handles. Worse, it's not where the handle is displayed, but rather where it will appear after it is moved. To see this, move the box out in qx then set the qx location of the box center. Move the cursor horizontally until the box suddenly springs to the new position. This occurs at the new qx position. I'm testing on a mac with pyside 6.9.1 |
I could replicate the problem on Windows 11 -- it seems to be only in box sum, with all other slicers working fine. But I found the same issue on main, so it is not introduced by changes here. I will see if I can find a fix for this regardless. For now, I will raise an issue for this. |
Description
Fixes #1576 by implementing a shared context manager that restores the previous flag value safely, wires it into current call sites, and adds tests to prove restoration on exceptions.
How Has This Been Tested?
Ran the new tests and tested manually.
Review Checklist:
Documentation
Installers
Licensing (untick if necessary)