fix: Compute origin using largest possible region

[ramonemiliani93]

Fixes #1065.

[ramonemiliani93]

@thewtex I have a couple of questions before tackling the negative indices issue:

1. Would it be okay to always compute the origin using the index of the largest possible region? Or would this be sub-optimal in terms of performance?
2. Could you guide me on how to add a test? 😬 I'm a bit new to this, so any references or guides on best practices and how to proceed would be greatly appreciated! 🙌

[thewtex]

@ramonemiliani93 thank you! 🙏

Would it be okay to always compute the origin using the index of the largest possible region? Or would this be sub-optimal in terms of performance?

Yes, the patch looks great. There should not be a significant performance impact.

Could you guide me on how to add a test? 😬 I'm a bit new to this, so any references or guides on best practices and how to proceed would be greatly appreciated! 🙌

Yes, you could add a test similar to this:

https://github.com/InsightSoftwareConsortium/itk-wasm/blob/main/test/itkWasmImageInterfaceTest.cxx

which is configured in the build here:

https://github.com/InsightSoftwareConsortium/itk-wasm/blob/50bbd4f21970c4b881ac68375b2399065648f885/test/CMakeLists.txt#L101-L108

You could add a pad filter in the new test and check that the origin of the output is expected. ITK's test driver has --compare functionality that checks metadata in addition to the pixel data. The baseline image content link would be added here:

https://github.com/InsightSoftwareConsortium/itk-wasm/tree/main/test/Baseline

To generate a content link, use https://content-link-upload.itk.org/

More information on how the data is handled can be found here: https://docs.itk.org/en/latest/contributing/data.html

🙌

[ramonemiliani93]

@thewtex Thanks a lot 🙌 I'll take a look and update the PR 💪

[ramonemiliani93]

@thewtex Added the test with the files and updated the testings deps 🙌 Let me know if I have to update things to follow the contributing guidelines or any naming convention 💪

[thewtex]

@ramonemiliani93 looks excellent!

One minor tweak: the file:

DATA{Input/negative_idx_padded.mha}

could be moved to:

DATA{Baseline/negative_idx_padded.mha}

since it is a baseline, i.e. expected output, test image.

[thewtex]

@ramonemiliani93 well done! 👏

Failing native Python CI builds are unrelated.

I will update the docker images in another PR.