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

fix(WebAssemblyInterface): support vector/multi-component images #1166

Conversation

jadh4v
Copy link
Member

@jadh4v jadh4v commented Jun 21, 2024

Fix buffer size calculations and number of components for handling vector images.

@jadh4v jadh4v requested a review from thewtex June 21, 2024 18:04
@@ -84,7 +84,7 @@ class ITK_TEMPLATE_EXPORT OutputImage

const auto dataAddress = reinterpret_cast< size_t >( wasmImage->GetImage()->GetBufferPointer() );
using ConvertPixelTraits = DefaultConvertPixelTraits<typename ImageType::PixelType>;
const auto dataSize = wasmImage->GetImage()->GetPixelContainer()->Size() * sizeof(typename ConvertPixelTraits::ComponentType) * ConvertPixelTraits::GetNumberOfComponents();
const auto dataSize = wasmImage->GetImage()->GetPixelContainer()->Size() * sizeof(typename ConvertPixelTraits::ComponentType);
Copy link
Member Author

Choose a reason for hiding this comment

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

GetPixelContainer()->Size() already includes number of components.

@jadh4v
Copy link
Member Author

jadh4v commented Jun 21, 2024

@thewtex I'm guessing we don't have any multi-component image already in test/input. So I might have to add a new multi-component image and either update itkWasmImageInterfaceTest.cxx or write a new test for this to test conversion in both directions (itkImage <--> WasmImage).

@thewtex
Copy link
Member

thewtex commented Jun 21, 2024

@thewtex I'm guessing we don't have any multi-component image already in test/input. So I might have to add a new multi-component image and either update itkWasmImageInterfaceTest.cxx or write a new test for this to test conversion in both directions (itkImage <--> WasmImage).

@jadh4v thanks for the patch!

Yes, could you please add a test, similar to:

https://github.com/InsightSoftwareConsortium/ITK-Wasm/blob/main/test/itkWasmImageInterfaceTest.cxx

that uses test/Input/apple.jpg as an input and, itk::VectorImage as the image type?

@jadh4v jadh4v force-pushed the fix-WebAssemblyInterface-vector-images branch from 7d7fb2b to 68f2174 Compare June 21, 2024 22:32
@jadh4v
Copy link
Member Author

jadh4v commented Jun 21, 2024

My test still fails on image comparison. I think this maybe because of jpeg format??

Trying itk::WriteImage(convertedImage, outputImageFile)
<DartMeasurement name="ImageError apple.jpg" type="numeric/double">1934</DartMeasurement>
<DartMeasurement name="ImageError" type="numeric/double">1934</DartMeasurement>
<DartMeasurement name="ImageError Minimum" type="numeric/double">3</DartMeasurement>
<DartMeasurement name="ImageError Maximum" type="numeric/double">8</DartMeasurement>
<DartMeasurement name="ImageError Mean" type="numeric/double">3.42037</DartMeasurement>
<DartMeasurementFile name="DifferenceImage" type="image/png">/home/jadhav/code/native-itkwasm/ITK-Wasm-build/Testing/Temporary/WebAssemblyInterface/itkWasmVectorImageInterfaceTest.jpg.diff.png</DartMeasurementFile>
<DartMeasurementFile name="BaselineImage" type="image/png">/home/jadhav/code/native-itkwasm/ITK-Wasm-build/Testing/Temporary/WebAssemblyInterface/itkWasmVectorImageInterfaceTest.jpg.base.png</DartMeasurementFile>
<DartMeasurementFile name="TestImage" type="image/png">/home/jadhav/code/native-itkwasm/ITK-Wasm-build/Testing/Temporary/WebAssemblyInterface/itkWasmVectorImageInterfaceTest.jpg.test.png</DartMeasurementFile>
<DartMeasurement name="BaselineImageName" type="text/string">apple.jpg</DartMeasurement>

@thewtex
Copy link
Member

thewtex commented Jun 24, 2024

@jadh4v thanks!

My test still fails on image comparison. I think this maybe because of jpeg format??

Yes, we can use .mha -- I will create a patch, add it to the branch. I will build / push the docker images and update the branch accordingly.

@thewtex thewtex force-pushed the fix-WebAssemblyInterface-vector-images branch from 68f2174 to b89bcb6 Compare June 24, 2024 17:54
Copy link
Member

@thewtex thewtex left a comment

Choose a reason for hiding this comment

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

👏

@thewtex thewtex merged commit 9ab6074 into InsightSoftwareConsortium:main Jun 25, 2024
95 checks passed
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.

2 participants