fix: set transform parameters by value when reconstructing from JSON - #1581
Merged
thewtex merged 1 commit intoAug 7, 2026
Conversation
CopyInFixedParameters/CopyInParameters std::copy into the transform's existing m_FixedParameters/m_Parameters without resizing them; the base class documents the requirement as "the range of values must conform to std::copy(begin, end, m_Parameters)". That holds for transforms that size those members in their constructor, but not for itk::BSplineTransform: its parameter count is unknown until the grid is set, and it keeps its coefficients in its own m_InternalParametersBuffer, so the base m_Parameters stays empty. Copying into it runs off the end of a zero-length buffer, which segfaults natively and aborts the module with no diagnostic under WebAssembly. Use SetFixedParameters and SetParametersByValue instead, which every transform implements as a copy, and check the parameter count so a mismatch reports instead of corrupting memory. Fixed parameters are set first: for a B-spline they define the grid, and hence the number of parameters. Fixed in both places that reconstruct a transform from TransformListJSON: itkWasmTransformToTransformFilter, in its composite and non-composite branches, and the copy of that pattern in the downsample package's resample pipelines. Adds itkWasmTransformInterfaceBSplineTest, which round-trips a composite containing a B-spline through the interface. No existing test exercised a B-spline on this path, which is why the defect went unnoticed; the new test segfaults without this change. Closes InsightSoftwareConsortium#1580
Member
|
@vboussot thank you! Since we changed the ITKWebAssemblyInterface module, we will need to first release a new docker base image, then release the downsample package built against this image. I will merge this after CI for the toolchain docker base images finishes green, then follow-up with those steps. |
thewtex
merged commit Aug 7, 2026
f8e99da
into
InsightSoftwareConsortium:main
60 of 87 checks passed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
CopyInFixedParameters/CopyInParameters std::copy into the transform's existing m_FixedParameters/m_Parameters without resizing them; the base class documents the requirement as "the range of values must conform to std::copy(begin, end, m_Parameters)". That holds for transforms that size those members in their constructor, but not for itk::BSplineTransform: its parameter count is unknown until the grid is set, and it keeps its coefficients in its own m_InternalParametersBuffer, so the base m_Parameters stays empty. Copying into it runs off the end of a zero-length buffer, which segfaults natively and aborts the module with no diagnostic under WebAssembly.
Use SetFixedParameters and SetParametersByValue instead, which every transform implements as a copy, and check the parameter count so a mismatch reports instead of corrupting memory. Fixed parameters are set first: for a B-spline they define the grid, and hence the number of parameters.
Fixed in both places that reconstruct a transform from TransformListJSON: itkWasmTransformToTransformFilter, in its composite and non-composite branches, and the copy of that pattern in the downsample package's resample pipelines.
Adds itkWasmTransformInterfaceBSplineTest, which round-trips a composite containing a B-spline through the interface. No existing test exercised a B-spline on this path, which is why the defect went unnoticed; the new test segfaults without this change.
Closes #1580