Skip to content

Add Visual Studio specific debug helpers to ITKCommon project#6699

Draft
dzenanz wants to merge 1 commit into
InsightSoftwareConsortium:mainfrom
dzenanz:natvis
Draft

Add Visual Studio specific debug helpers to ITKCommon project#6699
dzenanz wants to merge 1 commit into
InsightSoftwareConsortium:mainfrom
dzenanz:natvis

Conversation

@dzenanz

@dzenanz dzenanz commented Jul 24, 2026

Copy link
Copy Markdown
Member

Copying these files to the installation directory will not be required any more.

PR Checklist

@github-actions github-actions Bot added type:Infrastructure Infrastructure/ecosystem related changes, such as CMake or buildbots area:Core Issues affecting the Core module labels Jul 24, 2026
Copying these files to VS installation directory will no longer be required.
@hjmjohnson

Copy link
Copy Markdown
Member

Reviewed this on two axes (repo standards + does it do what the description claims). The standards side is minor, but the spec side has one blocker: a project-embedded .natstepfilter is not loaded by Visual Studio, so "copying these files will not be required any more" holds for ITK.natvis but not for the new ITK.natstepfilter.

Two things worth fixing regardless of what you decide about the step filter:

  • xmlns="https://schemas.microsoft.com/vstudio/debugger/natstepfilter/2010" should be http://. XML namespaces are compared as opaque strings, so https silently fails to match and VS ignores the file. Note this was already fixed once — 7f3f616 (N. Dekker, 2023) deliberately reverted this exact http:https: change.
  • itk::SmartPointer.+ should be .* to match the sibling default.natstepfilter; .+ requires a trailing character, so a bare itk::SmartPointer frame isn't filtered.
Spec: does the change deliver what the description claims?
  1. .natstepfilter has no project-item support. VS auto-loads .natvis from a project's item list (and can embed it in the PDB), but step filters are read only from %VSInstallDir%\Common7\Packages\Debugger\Visualizers\ and %USERPROFILE%\Documents\Visual Studio 20xx\Visualizers\. Adding it to ITKCommon_SRCS makes it appear in Solution Explorer and nothing more — the manual copy remains mandatory for the step filter.

  2. Visual Studio generator only. The .natvis-in-project mechanism is an MSBuild/VS feature. MSVC is also true under Ninja, where the file is just a non-compiled source and is ignored. Worth stating the limitation somewhere, since Ninja+MSVC is a common ITK configuration.

  3. Nothing added to the INSTALL/export path. Utilities/Debugger/ isn't installed or exported, so consumers of an installed ITK get neither file. For a PR about not needing to copy files, that's the case that still requires copying.

  4. Stale instructions left in place. Utilities/Debugger/ITK.natvis line 3 still says "e.g. copy into ...\Visualizers\ITK.natvis" and default.natstepfilter line 2 says "Copy into ...\default.natstepfilter". If copying is no longer required, those comments now contradict the shipped behaviour.

  5. The new file duplicates existing rules. Both rules in ITK.natstepfilter are a strict subset of default.natstepfilter, which already carries itk::SmartPointer.* and itk::Object::Register — a second place to maintain the same two patterns.

Standards: repo conventions
  1. elseif(MSVC) placement. That if/elseif chain (if(CMAKE_CXX_COMPILER MATCHES "^icc") ... elseif(CMAKE_COMPILER_IS_GNUCXX) ... elseif(MSVC)) exists solely to set floating-point flags on itkCompensatedSummation.cxx. Because it's elseif, an Intel-compiler-on-Windows build takes the first branch and never reaches the MSVC arm — those users silently get no visualizers. A separate if(MSVC) block would decouple the two concerns.

  2. Path computation. ${CMAKE_CURRENT_LIST_DIR}/../../../../Utilities/Debugger hard-codes the module's depth in the tree. ${ITK_SOURCE_DIR}/Utilities/Debugger is the established idiom here (see Modules/Core/GPUCommon/CMakeLists.txt:4) and drops the get_filename_component call entirely.

  3. Variable naming. ITK_Debugger_DIR reads like a public cache variable; local helpers in this same file use a leading underscore (e.g. _itk_python_target).

  4. The two list(APPEND ITKCommon_SRCS ...) calls could be a single call with both files.

The explanatory comments on the two list(APPEND) lines are worth keeping — attaching XML resources to a source list is not an obvious paradigm to developers who don't work in Visual Studio.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:Core Issues affecting the Core module type:Infrastructure Infrastructure/ecosystem related changes, such as CMake or buildbots

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants