Add option to change paint-erase brush size with mouse wheel#63
Conversation
|
The current implementation prevents changing slice with the wheel when in paint mode (without shift pressed) |
|
|
||
| def increase_brush_size(self, *_args, **_kwargs): | ||
| proxy = self._get_proxy() | ||
| proxy.brush_diameter *= 1.1 |
There was a problem hiding this comment.
Shouldn't 1.1 be a class variable with a default value defined as a global variable ?
There was a problem hiding this comment.
@Thibault-Pelletier should it be defined as a class constant in the segmentation effect, or in the pipeline and passed as a parameter, or somewhere else ?
There was a problem hiding this comment.
You can define it as an instance attribute to make it a bit cleaner.
Adding setter / getter is overkill in my opinion.
Jo-Byr
force-pushed
the
change-paint-brush-size-on-shift-wheel
branch
from
e26650f to
a96f71e
Compare
| def _on_effect_changed(self, _effect_name: str) -> None: | ||
| self._refresh_brush() | ||
| if self.is_active(): | ||
| self.effect.on_brush_diameter_changed.connect(self._set_brush_diameter) |
There was a problem hiding this comment.
Not sure this is the best way to connect responsiveness for parameters.
SegmentationEffects manage nodes in the scene and nodes in the scene impact the effect pipeline (when there are any).
The way to connect trame <-> Slicer is usually to monitor for scene changes and then to update the UI accordingly.
In Slicer application, we usually do UI updates from MRML and inverse.
I'll investigate how we can generalize this pattern to handle effects parameters as a whole.
Jo-Byr
force-pushed
the
change-paint-brush-size-on-shift-wheel
branch
from
407990c to
c841eac
Compare
Jo-Byr
force-pushed
the
change-paint-brush-size-on-shift-wheel
branch
from
c841eac to
d3bbc80
Compare
|
LGTM |
3 participants
No description provided.