Itk: MacOS itk vtk glue

[okaerin]

Description

I have a segmentation pipeline relying on itk 4.13 and vtk 8.1 . It is supposed to load a *.vtk structured points file. Somehow I get errors of "Determinant must not be 0" or "slice spacing is zero".
The header of the vtk volume producing the outcome:

# vtk DataFile Version 3.0
VTK File Generated by Insight Segmentation and Registration Toolkit (ITK)
BINARY
DATASET STRUCTURED_POINTS
DIMENSIONS 369 369 160 
SPACING 1.0000000000000000e+00 1.0000000000000000e+00 2.5000000000000000e+00 
ORIGIN -1.9830000305175781e+02 -1.8500000000000000e+02 -4.0750000000000000e+02 
POINT_DATA 21785760
SCALARS scalars short 1
LOOKUP_TABLE default

After doing further investigation I noticed that the ITKVtkglue tests fail on my machine:

Total Test time (real) = 404.70 sec

The following tests FAILED:
    1296 - itkCastImageFilterTest (Failed)
    2539 - itkVtkMedianImageFilterTest (SEGFAULT)
    2543 - QuickViewTest (SEGFAULT)
    2544 - itkVtkConnectedComponentImageFilterTest (Failed)
[ERROR_MESSAGE] 
Errors while running CTest

I am thinking that it might be connected.

Steps to Reproduce

Reproduce reading

      typedef itk::Image<short, 3> ImageType;
      typedef itk::ImageFileReader<ImageType> ImageFileReaderType;
      ImageFileReaderType::Pointer reader = ImageFileReaderType::New();
      try {
         reader->SetFileName(imageDataPath);
         reader->Update();
      } catch (itk::ExceptionObject &err) {
         std::cerr << err << std::endl;
      }

Reproduce test outcome

1. Build Vtk 8.1

cmake .. \
      -DCMAKE_CXX_STANDARD=11 \
      -DCMAKE_BUILD_TYPE=Release \
      -DBUILD_SHARED_LIBS=ON \
      -DVTK_LEGACY_REMOVE=ON \
      -DCMAKE_MACOSX_RPATH=ON \
      -DCMAKE_INSTALL_RPATH=\$ORIGIN \
      -DBUILD_TESTING=OFF \
      -DBUILD_EXAMPLES=OFF \
      -DModule_vtkRenderingExternal=ON
cmake --build . -j`nproc`

1. Build itk 4.13-release

cmake .. \
      -DVTK_DIR=${VTK_DIR} \
      -DCMAKE_CXX_STANDARD=11 \
      -DCMAKE_CXX_EXTENSIONS=OFF \
      -DCMAKE_CXX_STANDARD_REQUIRED=ON \
      -DCMAKE_BUILD_TYPE=Release \
      -DBUILD_SHARED_LIBS=ON \
      -DITK_LEGACY_REMOVE=ON \
      -DCMAKE_INSTALL_RPATH=\$ORIGIN \
      -DModule_ITKVtkGlue=ON \
      -DModule_IOSTL=ON \
      -DModule_LesionSizingToolkit=ON \
      -DBUILD_TESTING=ON \
      -DBUILD_EXAMPLES=OFF \
      -DITK_SKIP_PATH_LENGTH_CHECKS=ON 
cmake --build . -j`nproc`

1. Run tests

Expected behavior

Tests to complete without errors

Actual behavior

itkVtkConnectedComponentImageFilterTest

* thread #1, stop reason = signal SIGSTOP
  * frame #0: 0x00007fff796376f2 libsystem_platform.dylib`_platform_strlen + 18
    frame #1: 0x000000010e5393a5 ITKVtkGlueTestDriver`std::__1::char_traits<char>::length(__s=0x0000000000000000) at __string:218:53
    frame #2: 0x000000010e523f9c ITKVtkGlueTestDriver`std::__1::basic_ostream<char, std::__1::char_traits<char> >& std::__1::operator<<<std::__1::char_traits<char> >(__os=0x00007fffaf1a1770, __str=0x0000000000000000) at ostream:865:57
    frame #3: 0x000000010e71f93e ITKVtkGlueTestDriver`itkVtkConnectedComponentImageFilterTest(argc=0, argv=0x00007ffee16f0a98) at itkVtkConnectedComponentImageFilterTest.cxx:45:28
    frame #4: 0x000000010e5333df ITKVtkGlueTestDriver`main(ac=0, av=0x00007ffee16f0a98) at ITKVtkGlueTestDriver.cxx:162:14
    frame #5: 0x00007fff7944f3d5 libdyld.dylib`start + 1

Tests fail or segfault

Reproducibility

100%

Versions

4.13.3

Environment

ProductName:    Mac OS X
ProductVersion: 10.14.6
BuildVersion:   18G5033

Apple clang version 11.0.0 (clang-1100.0.33.17)
Target: x86_64-apple-darwin18.7.0
Thread model: posix
InstalledDir: /Library/Developer/CommandLineTools/usr/bin



md5-f0b02cd68bdc8847b425696bc5a4e8c3



cmake version 3.17.3

Additional Information

[dzenanz]

ImageReader not working properly sounds like a compilation gone wrong, and not overly related to VTKGlue. Can you do a clean build (build ITK in a new binary directory) and see whether the problem persists?

[okaerin]

ImageReader not working properly sounds like a compilation gone wrong, and not overly related to VTKGlue. Can you do a clean build (build ITK in a new binary directory) and see whether the problem persists?

@dzenanz The clean build of ITK changed nothing

[thewtex]

Please try ITK 5.1.1 -- ITK 4.13 is not actively maintained.

[okaerin]

@thewtex @dzenanz I found the issue by myself after a sudden epiphany. Unfortunately the locale doesn't match when writing and reading the *.vtk file. This is the case for 4.13 and as well the newer version 5.1.1 . The concerning source file can be found here itkVTKImageIO.cxx .
The issue stems from the code that writes the vtk file using std::ofstream and reading with sscanf. The trouble there is that ofstream uses, if no locale is defined, the C standard locale and sscanf uses the LC_NUMERIC sscanf man page environment variable. In my case LC_NUMERIC is de_CH.UTF-8. Unfortunately it requires commas when reading floating point numbers whereas the C locale uses dots. Therefore the the number parser from sscanf fails and ouputs 0 for the spacing spacing results (which is not true). VTK solves this by temporarily setting the locale to C and when it's done reading it reverts it back with this snippet (taken from vtkDataReader.cxx):

// Save current locale settings and set standard one to
// avoid locale issues - for instance with the decimal separator.
this->CurrentLocale = std::locale::global(std::locale::classic());
....
// Restore the previous locale settings
std::locale::global(this->CurrentLocale);

Here is the code to support my claim and with the output:

#include <iostream>
#include <fstream>
#include <clocale>
#include <cstdlib>

int main() {
  std::ofstream os;
  std::cout << "LC_NUMERIC: " << setlocale(LC_NUMERIC, "") << std::endl;
  std::cout << "LC_ALL: " << setlocale(LC_ALL, "") << std::endl;
  std::cout << "ofstream locale: " << os.getloc().name() << std::endl;
  std::cout << "default locale: " << std::locale().name() << std::endl;

  os.imbue(std::locale(getenv("LC_NUMERIC")));
  std::cout << "ofstream locale aware: "<<os.getloc().name() << std::endl;
}
//output:
LC_NUMERIC: de_CH.UTF-8
LC_ALL: en_US.UTF-8/en_US.UTF-8/de_CH.UTF-8/de_CH.UTF-8/de_CH.UTF-8/en_US.UTF-8
ofstream locale: C
default locale:C
ofstream locale aware de_CH.UTF-8

[dzenanz]

Some VTKGlue tests fail on my computer which has English-US locale.

#include <iostream>
#include <fstream>
#include <clocale>
#include <cstdlib>

int
main()
{
  try
  {
    std::ofstream os;
    std::cout << "LC_NUMERIC: " << setlocale(LC_NUMERIC, "") << std::endl;
    std::cout << "LC_ALL: " << setlocale(LC_ALL, "") << std::endl;
    std::cout << "ofstream locale: " << os.getloc().name() << std::endl;
    std::cout << "default locale: " << std::locale().name() << std::endl;

    os.imbue(std::locale(getenv("LC_NUMERIC")));
    std::cout << "ofstream locale aware: " << os.getloc().name() << std::endl;
  }
  catch (std::exception & exc)
  {
    std::cout << "std::exception: " << exc.what();
  }
}

LC_NUMERIC: English_United States.utf8
LC_ALL: English_United States.utf8
ofstream locale: C
default locale: C
std::exception: bad locale name

I guess this issue has nothing to do with those failing tests.

Can you make a PR to fix this in ITK using VTK's trick?

[okaerin]

@dzenanz No the failing tests wont be fixed, though I know why they fail (that's another issue entirely) but at least I know how to load the vtk volume ;). Will do, as soon as I get to it.
The issue with the tests is that they use argv with an an index that does not exist. So i suspect that the tests don't supply the required filename as a parameter. I haven't investigated any further on this front.

[okaerin]

@dzenanz I created a pull request https://github.com/InsightSoftwareConsortium/ITK/pull/2297. I fixed all the locations using the sscanf "faulty" version. I did not touch any Thirdparty Modules, as I thought that this issue has to be adressed in the original repos (e.g. GDCM). This is just an easy fix without much changing the code base. I think the better approach would be to update the entire code to remove those "C-Style" functions and to use the C++ versions of it (e.g. string streams etc.). This would remove the different behavoir cases and would also improve internationalization (e.g. windows sscanf cannot format utf8 string, it just fills in empty strings), as these are utf8 aware.

[thewtex]

@okaerin thank you for diving in and addressing this! :pray: :medal_sports:

@malaterre any thoughts on the desired approach for GDCM re: internationalization and sscanf?

@floryst

[malaterre]

@okaerin it seems as if you wrote the patch by hand ? what are your scripts, can you share them ?

This issue has been in GDCM since forever, maybe time to fix it for real (see GDCM_TESTING_USE_LC_NUMERIC). I am pretty sure this is the reason why @lamyj started odil :)

[okaerin]

@malaterre Yes I did it by hand as there were so few locations. To identify the locations i used this command in the ITK repo:

grep -zoPr "sscanf[^;]+%\w*f" | tr '\0' '\n'

[okaerin]

@malaterre @thewtex sscanf is not the only internationalization issue. When it comes to windows it is not possible to open files (with any kind of function/std IO) when they contain non ASCII characters. This is due to the fact, that the windows C++ interfaces do not accept utf8 char strings, but needs wchar_t paths. To make it work we would have to implement/use the I18n infrastructure (already in place in ITK) to open/access files.

[issakomi]

windows C++ interfaces do not accept utf8 char strings, but needs wchar_t paths

Why not, they do, since Windows 10, May 2019 update. E.g. add _manifest_ file to executable and handle the rest in the same way as e.g. on Linux with UTF-8 locale.

<?xml version="1.0" encoding="UTF-8" standalone="yes"?>
<assembly manifestVersion="1.0" xmlns="urn:schemas-microsoft-com:asm.v1">
  <application>
    <windowsSettings>
      <activeCodePage xmlns="http://schemas.microsoft.com/SMI/2019/WindowsSettings">UTF-8</activeCodePage>
    </windowsSettings>
  </application>
</assembly>

S. details at docs.microsoft.com.

Many apps switched to UTF-8 on Windows, e.g. Slicer >= 4.11. _WCHAR_ has advantage to work with any Windows version, of course. BTW, UTF-8 approach doesn't break existing UTF-16 _WCHAR_ code, it looks like Microsoft recommends UTF-8 now.

[malaterre]

@okaerin I fail to understand your patch now. Why are you only fixing the reading of the file and not the writing. Looking at the code from @dzenanz I would have expected the patch to contain also a:

os.imbue(std::locale(""));

when writing the file out...

[malaterre]

@okaerin

@malaterre @thewtex sscanf is not the only internationalization issue. When it comes to windows it is not possible to open files (with any kind of function/std IO) when they contain non ASCII characters. This is due to the fact, that the windows C++ interfaces do not accept utf8 char strings, but needs wchar_t paths. To make it work we would have to implement/use the I18n infrastructure (already in place in ITK) to open/access files.

This issue has been fixed for >1 year in GDCM. windows does support UTF-8, and this is what the GDCM API expect (a UTF-8 encoding of the filename).

[okaerin]

windows C++ interfaces do not accept utf8 char strings, but needs wchar_t paths

Why not, they do, since Windows 10, May 2019 update. E.g. add _manifest_ file to executable and handle the rest in the same way as e.g. on Linux with UTF-8 locale.

<?xml version="1.0" encoding="UTF-8" standalone="yes"?>
<assembly manifestVersion="1.0" xmlns="urn:schemas-microsoft-com:asm.v1">
  <application>
    <windowsSettings>
      <activeCodePage xmlns="http://schemas.microsoft.com/SMI/2019/WindowsSettings">UTF-8</activeCodePage>
    </windowsSettings>
  </application>
</assembly>

S. details at docs.microsoft.com.

Many apps switched to UTF-8 on Windows, e.g. Slicer >= 4.11. _WCHAR_ has advantage to work with any Windows version, of course. BTW, UTF-8 approach doesn't break existing UTF-16 _WCHAR_ code, it looks like Microsoft recommends UTF-8 now.

You are right, but only with a manifest, which seems like it would complicate the ITK build with a special build rule. Furthermore it would exclude "older" Windows users, which seems like a bad choice for a library used in a medical context (there are some old systems floating around)

[okaerin]

@okaerin I fail to understand your patch now. Why are you only fixing the reading of the file and not the writing. Looking at the code from @dzenanz I would have expected the patch to contain also a:

os.imbue(std::locale(""));

when writing the file out...

That is because the std::ofstream seems to be defaulting already to the "C" locale, as you can see from my previous example. Therfore it isn't necessary to set it there.

[issakomi]

it would complicate the ITK build with a special build rule

it doesn't belong to ITK build, only to executable. BTW, i have nothing against _WCHAR_ UTF-16 Windows code, just FYI.

[okaerin]

Just to show the outcome with the utf8, when there is no handling (like @issakomi suggested, or using the wchar api):

#include <windows.h>
#include <iostream>
#include <cstring>
#include <codecvt>
#include <fstream>
#include <locale>

int main()
{
    //helper
    std::wstring_convert<std::codecvt_utf8_utf16<wchar_t>> converter;
    //create file
    std::ofstream out1;
    out1.open(u8"File-1-日本語");//char utf8
    out1 << u8"私は日本語で働いています";
    //create file unicode aware
    std::ofstream out2;
    out2.open(L"File-2-日本語");//wide char utf8
    out2 << converter.to_bytes(L"私は日本語で働いています");

    return 0;
}

[malaterre]

@okaerin For reference:

• https://en.cppreference.com/w/cpp/locale/LC_categories

[okaerin]

it would complicate the ITK build with a special build rule

it doesn't belong to ITK build, only to executable. BTW, i have nothing against _WCHAR_ UTF-16 Windows code, just FYI.

Wouldn't you have to attach this kind of manifest also to the itk shared libraries?

[issakomi]

Wouldn't you have to attach this kind of manifest also to the itk shared libraries?

I am not sure, probably not. You may ask Slicer developers. I use static ITK libs. Qt app works well without any changes, only with manifest.

[issakomi]

and this is what the GDCM API expect (a UTF-8 encoding of the filename).

BTW, bare GDCM (didn't test via ITK IO) should work on old Windows too, only push somehow UTF-8 string, it is no problem with e.g. Qt. I got it working on Windows 7 too after gdcmSystem.cxx update last year. CP_UTF8 was defined on Windows long time ago, it was only not possible to use it as _activeCodePage_ till May 2019 update. Many other 3rd party libs in ITK IO probably will not work without manifest, IMHO, may be i am wrong.