Skip to content

Conversation

@N-Dekker
Copy link
Member

@N-Dekker N-Dekker commented Mar 4, 2020

Replaced _ELASTIX_BUILD_LIBRARY preprocessor definition by static member function, BaseComponent::IsElastixLibrary().

Anticipates sharing the intermediate binary object files between the Elastix executable and the Elastix library, possibly building them within the same project, or the same (Visual Studio) solution.

@N-Dekker N-Dekker requested review from mstaring and stefanklein March 4, 2020 16:11
Replaced `_ELASTIX_BUILD_LIBRARY` preprocessor definition by static member function, `BaseComponent::IsElastixLibrary()`.

Anticipates sharing the intermediate binary object files between the Elastix executable and the Elastix library, possibly building them within the same project, or the same (Visual Studio) solution.
@N-Dekker N-Dekker force-pushed the Remove-_ELASTIX_BUILD_LIBRARY branch from 8c72767 to 64ced59 Compare March 4, 2020 16:19
@N-Dekker
Copy link
Member Author

N-Dekker commented Mar 4, 2020

@mstaring @stefanklein Please check carefully! 😃 I think we need to remove the dependencies on _ELASTIX_BUILD_LIBRARY, in order to be able to build the exe and the lib within one and the same "solution file", sharing the same *.obj" files with Visual C++.

@N-Dekker N-Dekker marked this pull request as ready for review March 5, 2020 10:17
@N-Dekker
Copy link
Member Author

N-Dekker commented Mar 5, 2020

@stefanklein As you suggested this morning @ LUMC, I compared the output directory of the example from "elastix\dox", as generated by the official elastix 5.0.0 release to the output directory generated from this pull request. I think they look quite comparable 😄

I ran:

elastix.exe -f exampleinput/fixed.mhd -m exampleinput/moving.mhd -out exampleoutput\ -p exampleinput/parameters_Rigid.txt -p exampleinput/parameters_BSpline.txt

result.0.mhd, result.0.raw, result.1.mhd, result.1.raw, and TransformParameters.0.txt are exactly the same in both output directories.

elastix.log has the same number of lines. I only observed different current date and time, and slightly different durations.

@mstaring
Copy link
Member

mstaring commented Mar 5, 2020

Glancing through it, it looks good and I see no reason to worry :-)

As indeed before and after looks the same, I trust the PR and you can merge

@N-Dekker N-Dekker merged commit 633c684 into develop Mar 5, 2020
@N-Dekker N-Dekker deleted the Remove-_ELASTIX_BUILD_LIBRARY branch March 5, 2020 13:14
N-Dekker added a commit that referenced this pull request May 1, 2020
Kasper Marstal reported yesterday via direct messaging that the introduction of `IsElastixLibrary()`, pull request #231, commit 64ced59 (March 2, 2020, "Replace _ELASTIX_BUILD_LIBRARY by BaseComponent::IsElastixLibrary"), did break SimpleElastix.  SimpleElastix depends on the library interface, while it does not construct an `elastix::ELASTIX` library object.

The most elegant and reliable fix appears to be flipping the default value returned by `IsElastixLibrary()`, from `false` to `true`, and then ensuring that the value is set to `false` at the start of each run of the executable.
N-Dekker added a commit that referenced this pull request May 1, 2020
Kasper Marstal reported yesterday via direct messaging that the introduction of `IsElastixLibrary()`, pull request #231, commit 64ced59 (March 2, 2020, "Replace _ELASTIX_BUILD_LIBRARY by BaseComponent::IsElastixLibrary"), did break SimpleElastix.  SimpleElastix depends on the library interface, while it does not construct an `elastix::ELASTIX` library object.

The most elegant and reliable fix appears to be flipping the default value returned by `IsElastixLibrary()`, from `false` to `true`, and then ensuring that the value is set to `false` at the start of each run of the executable.

This applies to the transformix executable as well.
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.

3 participants