InsightSoftwareConsortium / InsightSoftwareConsortium/ITK

ENH: Route ImageIO file opens through `itkInternationalizationIOHelpers` for non-ASCII Windows filenames (Tier B)

Open
#6,739 1 comment 0 reactions 0 assignees View on GitHub
area:IO type:Enhancement
Dominant language
C++
Stars
1.7k
Forks
748
Avg merge
1d 1h
Merged PRs (30d)
64

Description

ITK's Windows UTF-8 filename fix (Tier A, #6231/#6244) embeds a UTF-8 active-code-page manifest in ITK's own executables, but `python.exe` is shipped by upstream Python and cannot carry an ITK manifest — so SimpleITK and the ITK Python wrapping still fail on non-ASCII filenames on Windows. Tier B routes the library-side file opens in `itkImageIOBase.cxx`, `itkPNGImageIO.cxx`, and `itkJPEGImageIO.cxx` through the already-present-but-entirely-unused `itkInternationalizationIOHelpers`.

Background — how we got here (#4388, #4390, Tier A)

PR #4390 (@Pfleiderer-Adrian) surfaced ITK's Windows non-ASCII filename failure mode, reported as issue #4388. The PR proposed rejecting non-ASCII paths in the Python wrapper; reviewers (@N-Dekker, @blowekamp, @dzenanz) rejected that as both too strict and Python-only.

**Tier A** (merged: #6231 `ENH: Embed UTF-8 active-code-page manifest in Windows test executables (supersedes #4390)`, plus #6244 `COMP: Move CMake's include(ITKWindowsUtf8) for external modules`) mirrors the elastix recipe (SuperElastix/elastix#1401, Bernhard Fröhler): ship a Windows application manifest declaring `UTF-8` plus a runtime `SetConsoleOutputCP(CP_UTF8)`, auto-injected into every MSVC executable via a CMake helper. Library code untouched.

**Tier B (this issue)** is the case Tier A structurally cannot reach: a process ITK does not build. To make non-ASCII filenames work for Python/SimpleITK consumers on Windows, ITK must convert UTF-8 `char *` paths to UTF-16 `wchar_t *` internally before calling `_wfopen` / `CreateFileW` / the wide `std::ifstream` overload.

Evidence — the machinery already exists and is completely unused

`Modules/IO/ImageBase/include/itkInternationalizationIOHelpers.h` (~298 lines) provides `i18n_open`, `i18n_fopen`, and UTF-8↔UTF-16 conversion helpers, gated on `ITK_SUPPORTS_WCHAR_T_FILENAME_CSTYLEIO`. It was written, reviewed, and merged years ago — and then never wired into anything.

Verified against current `main`:

```
$ git grep -ln "i18n_open\|i18n_fopen" upstream/main -- Modules
(no output)
```

Zero hits anywhere under `Modules/`. That makes Tier B a *wire-up-existing-code* task rather than a design-from-scratch task. (Because the grep is empty even for the header's own declarations, confirm the exact current entry-point spelling in the header before starting.)

Target call sites:

- `Modules/IO/ImageBase/src/itkImageIOBase.cxx` — two raw `std::ifstream` / `std::ofstream` open sites (around lines 610 and 662 as of the original survey; re-locate rather than trusting the line numbers).
- `Modules/IO/PNG/src/itkPNGImageIO.cxx` and `Modules/IO/JPEG/src/itkJPEGImageIO.cxx` — raw `fopen` / `std::ifstream` sites in the two most user-visible image-IO formats. These cover the SimpleITK loading hot path.

Roughly **349** narrow-API file-open sites exist across non-ThirdParty `Modules/`. Tier B deliberately does *not* migrate all of them; the ImageIOBase + PNG + JPEG triplet is the defensible single-PR scope.

Suggested approach

1. **Establish the contract.** Decide whether `SetFileName(const char *)` is *documented* as accepting UTF-8 on all platforms (a public-API guarantee) or remains an implementation detail. Recommendation: document it — ITK 6 is the license-to-revise window.
2. **Wire `i18n_open` into `itkImageIOBase.cxx`**, replacing the identified open sites. Verify by running the ImageIO unit tests on Windows under a non-UTF-8 active code page (e.g. CP1252).
3. **Wire `i18n_fopen` into `itkPNGImageIO.cxx` and `itkJPEGImageIO.cxx`**, same pattern.
4. **Add a Windows CI test** that writes and reads a file whose path contains `ä ü ö` plus CP1252-incompatible characters (CJK), round-tripping through `ImageFileReader` / `ImageFileWriter`.
5. **Add an ITK 6 migration-guide entry** announcing the UTF-8 filename contract.

Out of scope

- The remaining ~340 narrow-API open sites — a full audit plus KWSys coordination. That is Tier C, a separate multi-PR effort.
- ABI-affecting `SetFileName(const wchar_t *)` overloads — would require Python-wrapping support and SimpleITK signature changes.

Related

- Issue #4388 — original bug report
- PR #4390 — @Pfleiderer-Adrian, Python-side ASCII reject; superseded by Tier A
- PR #6231, PR #6244 — Tier A (manifest + `SetConsoleOutputCP`), merged
- SuperElastix/elastix#1401 — the manifest + console-code-page recipe Tier A mirrors
- Issue #540 (`itksys::SystemTools::FileIsDirectory does not handle accented characters`) — **closed**; still the useful historical reference for the Tier C KWSys work
- `Modules/IO/ImageBase/include/itkInternationalizationIOHelpers.h` — the existing, unused i18n machinery

Contributor guide

Open the contributing guide

Research direction

Start by confirming the entry-point names in Modules/IO/ImageBase/include/itkInternationalizationIOHelpers.h, then inspect the open sites in itkImageIOBase.cxx, itkPNGImageIO.cxx, and itkJPEGImageIO.cxx. Run the relevant ImageIO tests on Windows under a non-UTF-8 code page. Done means the specified calls use the helpers, a Windows non-ASCII path round-trip test passes, and the ITK 6 migration guide documents the filename contract.

Written by the indexing model from the issue text.

Assessment

Tech stack
cpp, python
Domain
computer-vision, documentation, operating-systems, testing-qa
Issue type
Feature
Difficulty
4/5
Estimated time
3-5 days
Activity status
Quiet
Clarity
Mostly clear
Newbie friendliness
52/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.