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

Add qt to vtk image #882

Merged
merged 3 commits into from
Sep 4, 2019
Merged

Add qt to vtk image #882

merged 3 commits into from
Sep 4, 2019

Conversation

lassoan
Copy link
Member

@lassoan lassoan commented Sep 3, 2019

Add utility function to convert Qt image to VTK image and fix a few Python wrappings.

@lassoan lassoan requested a review from jcfr September 3, 2019 04:35
@finetjul
Copy link
Member

finetjul commented Sep 3, 2019

A QImage may not be RGBA, it has multiple pixel formats.

@finetjul
Copy link
Member

finetjul commented Sep 3, 2019

Alternatively, you may want to use vtkQImageToImageSource

@lassoan
Copy link
Member Author

lassoan commented Sep 3, 2019

I've tried to use vtkQImageToImageSource first. First problem was that the filter was not wrapped by VTK's Python wrapper. I've tried to add a wrapping around it from CTK, but it turned out it was not included in VTK_LIBRARIES. Instead of blindly adding one more dependency, I had a look at the vtkQImageToImageSource implementation and realized that the filter is overcomplicated and the operation can be implemented in a few lines of code and so it is not worth using vtkQImageToImageSource.

@finetjul
Copy link
Member

finetjul commented Sep 3, 2019

Fair enough

@lassoan
Copy link
Member Author

lassoan commented Sep 3, 2019

QImage may not be RGBA, it has multiple pixel formats.

We call inputQImage.convertToFormat(QImage::Format_RGBA8888), to get an RGBA image.

@finetjul
Copy link
Member

finetjul commented Sep 3, 2019

QImage may not be RGBA, it has multiple pixel formats.

We call inputQImage.convertToFormat(QImage::Format_RGBA8888), to get an RGBA image.
Yep, I saw that afterwards.

That's good with me then.

@pieper
Copy link
Member

pieper commented Sep 3, 2019

Do you plan to replace the corresponding code that's already in Slicer?

https://github.com/Slicer/Slicer/blob/a02f79c7c788708a3dccfeb9ff4df76b6a5868be/Libs/MRML/Widgets/qMRMLUtils.cxx#L151-L205

@lassoan
Copy link
Member Author

lassoan commented Sep 3, 2019

Do you plan to replace the corresponding code that's already in Slicer?

https://github.com/Slicer/Slicer/blob/a02f79c7c788708a3dccfeb9ff4df76b6a5868be/Libs/MRML/Widgets/qMRMLUtils.cxx#L151-L205

I've added this new function because I did not find it in Slicer (the closest was the one in CTK, which only provided conversion in the other direction).

I think it makes sense to have the feature available at the lowest-level library (and use that in higher-level libraries). So, I would move the best implementation of the converters to CTK and use that from qMRMLUtils.

lassoan and others added 3 commits September 3, 2019 15:05
Allows converting Qt image to VTK image.

Also added test for qImageToVTKImageData and vtkImageDataToQImage.
Get database folder settings key from ctkDICOMBrowser::defaultDatabaseDirectorySettingsKey().
@lassoan lassoan force-pushed the add-qt-to-vtk-image branch from f260a9e to 5f6bf67 Compare September 3, 2019 19:15
@lassoan lassoan merged commit 9512127 into commontk:master Sep 4, 2019
@lassoan
Copy link
Member Author

lassoan commented Sep 4, 2019

Thanks for the reviews. I've consolidated with the implementations in Slicer, added a test, and merged.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants