Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Parameter node wrapper GUI connectors are missing for many widgets #7308

Open
lassoan opened this issue Oct 27, 2023 · 8 comments
Open

Parameter node wrapper GUI connectors are missing for many widgets #7308

lassoan opened this issue Oct 27, 2023 · 8 comments
Labels
Type: Bug Something isn't working correctly
Milestone

Comments

@lassoan
Copy link
Contributor

lassoan commented Oct 27, 2023

Summary

Many Qt widgets cannot be used with the recently added parameter node wrapper infrastructure.

Steps to reproduce

Try using the "not supported" widgets in a module GUI and link to parameter node.

Supported:

  • qt.QCheckBox
  • qt.QPushButton
  • qt.QSlider
  • qt.QSpinBox
  • qt.QDoubleSpinBox
  • ctk.ctkSliderWidget
  • slicer.qMRMLSliderWidget
  • qt.QComboBox
  • qt.QLineEdit
  • qt.QTextEdit
  • ctk.ctkRangeWidget
  • ctk.ctkPathLineEdit
  • ctk.ctkDirectoryButton
  • slicer.qMRMLNodeComboBox

Not supported:

  • qMRMLCheckableNodeComboBox
  • qMRMLColorPickerWidget
  • qMRMLCoordinatesWidget
  • qMRMLLinearTransformSlider
  • qMRMLListWidget
  • qMRMLMatrixWidget
  • qMRMLRangeWidget
  • qMRMLSegmentSelectorWidget
  • qMRMLSpinBox
  • qMRMLTransformSliders
  • ctkCheckableComboBox
  • ctkCheckBox
  • ctkColorPickerButton
  • ctkComboBox
  • ctkCoordinatesWidget
  • ctkCoordinatesWidget
  • ctkDoubleRangeSlider
  • ctkDoubleSlider
  • ctkDoubleSpinBox
  • ctkFittedTextBrowser
  • ctkMatrixWidget
  • ctkRangeSlider
  • ctkTreeComboBox

Expected behavior

All widgets that are available in Qt Designer should be usable with the parameter node wrapper's GUI connector.

Environment

  • Slicer version: Slicer-5.5.0-2023-10-26
  • Operating system: all
@lassoan lassoan added the Type: Bug Something isn't working correctly label Oct 27, 2023
@HarryDC
Copy link
Contributor

HarryDC commented Nov 2, 2023

I think we can turn this into a tracking issue for the unsupported widgets but there is a difference in the amount of effort it will take to implement some of these. For the following we already support the underlying data types which means we only need to implement one side of the mapping (from the widget to the datatype)

Implemented/In Progress

  • qMRMLRangeWidget
  • qMRMLSpinBox
  • ctkCheckBox
  • ctkComboBox
  • ctkDoubleRangeSlider
  • ctkDoubleSlider
  • ctkDoubleSpinBox
  • QLabel

Need Triage

  • qMRMLCheckableNodeComboBox
  • ctkCheckableComboBox
  • ctkFittedTextBrowser

Need more work

these will also need support for the datastructures that they map to

  • ctkRangeSlider
    Needs implementation of IntRange to mirror FloatRange
  • qMRMLColorPickerWidget
  • qMRMLCoordinatesWidget
  • qMRMLLinearTransformSlider
  • qMRMLMatrixWidget
  • qMRMLSegmentSelectorWidget
  • qMRMLTransformSliders
  • ctkColorPickerButton
  • ctkCoordinatesWidget
  • ctkMatrixWidget
  • ctkTreeComboBox

Needs discussion

  • qMRMLListWidget

@lassoan
Copy link
Contributor Author

lassoan commented Nov 2, 2023

@HarryDC thanks a lot for looking into this. Do you think you will have time to work on some of these?

@jcfr jcfr added this to the Backlog milestone Nov 2, 2023
@HarryDC
Copy link
Contributor

HarryDC commented Nov 2, 2023

@lassoan Yes I'll start with the "easy" list and we'll work our way down from there but i think PRs are probably going to come in pieces

@HarryDC
Copy link
Contributor

HarryDC commented Dec 14, 2023

Hit some issues when adding the ctkDoubleRangeSlider commontk/CTK#1157

Additionally found an issue when using the FloatRange annotation, when there is a RangeBounds markup and the bounds do not include 0, a default value has to be provided otherwise the connection fails as the default of FloatRange is 0,0. Right now i'm requiring a default value for the Ranges .

I want to add the CtkRangeSlider and then i'll put up a first PR

@jcfr
Copy link
Member

jcfr commented Dec 14, 2023

re: ctkDoubleRangeSlider

The following pull request should expose the relevant method to Python:

Can you test locally and see if anything else is needed ?

@HarryDC
Copy link
Contributor

HarryDC commented Dec 14, 2023

I can check this but i worked around it using the maximum and minimum property it made sense after the explanation

@jcfr
Copy link
Member

jcfr commented Dec 14, 2023

Using the setRange will ensure the range changed signal is emitted only once.

@HarryDC
Copy link
Contributor

HarryDC commented Jan 12, 2024

Note after discussion with @sjh26 we figured out that qMRMLListWidget is currently not used in the slicer core, would need more work wrt what the expected result of a selection is for this widget, might need more work in slicer to expose signals on selection.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Bug Something isn't working correctly
Development

No branches or pull requests

3 participants