Skip to content

COMP: Fix Windows build error#525

Merged
fedorov merged 3 commits intoQIICR:masterfrom
lassoan:fix-windows-build
Jan 26, 2026
Merged

COMP: Fix Windows build error#525
fedorov merged 3 commits intoQIICR:masterfrom
lassoan:fix-windows-build

Conversation

@lassoan
Copy link
Contributor

@lassoan lassoan commented Jan 25, 2026

By default, windows.h adds macros "min" and "max", which interfere with other "max" symbols (e.g., in numeric_limits::max, max in vnl_sse.h). If "NOMINMAX" is defined before including windows.h then the min and max macros do not get defined.

ITK headers include NOMINMAX before including windows.h, therefore we include an ITK header before using DCMTK to ensure that min and max macros are not added, avoiding any name clash.

By default, windows.h adds macros "min" and "max", which interfere with other "max" symbols (e.g., in numeric_limits::max, max in vnl_sse.h).
If "NOMINMAX" is defined before including windows.h then the min and max macros do not get defined.

ITK headers include NOMINMAX before including windows.h, therefore we include an ITK header before using DCMTK to ensure that min and max macros are not added, avoiding any name clash.
@lassoan
Copy link
Contributor Author

lassoan commented Jan 26, 2026

The CI error seems a false alarm to me. mkdir should get -p argument so that it does not return with error when the build folder already exists (probably due to successful caching).

@sonarqubecloud
Copy link


namespace dcmqi
{
class DCMTK_DCMSEG_EXPORT DcmBinToLabelConverter
Copy link
Member

Choose a reason for hiding this comment

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

@lassoan I am curious - why did you remove this decorator?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The export directive is only needed if the target is built as a shared library. This is built as a static library, so export directive is not necessary. Moreover, this export directive is just copied from somewhere, probably from a DCMTK example, so having it here would have made the build fail.

Copy link
Member

Choose a reason for hiding this comment

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

Thank you Andras! Yes, likely this was just copied from DCMTK when @michaelonken wrote this class.

Copy link
Member

Choose a reason for hiding this comment

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

The export directive is only needed if the target is built as a shared library. This is built as a static library, so export directive is not necessary. Moreover, this export directive is just copied from somewhere, probably from a DCMTK example, so having it here would have made the build fail.

Yes exactly, it was accidentally copied from DCMTK. Its not required within dcmqi code.

Thanks for support Andras!

@fedorov fedorov merged commit 5532f21 into QIICR:master Jan 26, 2026
7 checks passed
@lassoan lassoan deleted the fix-windows-build branch January 26, 2026 12:01
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

Comments