[Insight-developers] CMake questions related to SimpleITK C# wrapping

[David Cole]

On Sun, Jun 19, 2011 at 12:10 AM, Dan Mueller <dan.muel at gmail.com> wrote:
> Hi Bradley,
>
> Thanks for your response. See inline below.
>
> On 18 June 2011 21:11, Bradley Lowekamp <blowekamp at mail.nih.gov> wrote:
>> Hello Dan,
>> Thanks for looking into the C# with simpleITK. SimpleITK should be a
>> exciting and simple way to use ITK when easy of use and rapid prototyping
>> are important along with  the many other benefits is has.
>> On Jun 18, 2011, at 1:17 AM, Dan Mueller wrote:
>>
>> Hi Insight Developers,
>>
>> I have found some time here and there to fix/improve SimpleITK C#
>> wrapping -- I can now generate SimpleITK C# wrappers on Windows using
>> .NET or Mono, and on Linux using Mono.
>>
>> Great!
>> I think one of the biggest things we are missing for C# is the nightly
>> testing. It looks like I should look into the Mono thing some.
>> It the build process fully automated with your patch? For some reason I
>> thought there was a step or two that needed to be done by hand?
>
> Yes, I have fixed the dependency issue, so the build is now fully automated.
>
>> However, I still have a number of minor issues for which I need some advice:
>>
>> 1. LIBRARY_OUTPUT_DIRECTORY in Visual Studio:
>> At CMake configure/generate time, I add a custom command (via the
>> swig_add_module macro):
>>    swig_add_module(SimpleITKCSharpNative csharp SimpleITK.i)
>> For various reasons I need to force the output directory:
>>    set_target_properties(SimpleITKCSharpNative PROPERTIES
>> LIBRARY_OUTPUT_DIRECTORY ${CSHARP_BINARY_DIRECTORY})
>> where CSHARP_BINARY_DIRECTORY =  /SimpleITK/Build/Wrapping/CSharpBinaries
>>
>> Under Linux the C# binary is nicely created in:
>>    /SimpleITK/Build/Wrapping/CSharpBinaries
>> However, under Windows (using Visual Studio 10 generator) the binary
>> is created in:
>>    /SimpleITK/Build/Wrapping/CSharpBinaries/Release
>> (notice the configuration name "Release" at the end).
>>
>> Why is $(Configuration) added to the library output path? Can I prevent
>> this?
>> (This is not a big deal, but it requires yet another "if (WIN32)"
>> check in the CMakeLists.txt which I'd prefer to avoid...)


Finally! Time to answer this question that's been in my Inbox all week... :-)

$(Configuration) is appended by default to provide separate locations
for Debug and Release binaries as a convenience for those who are
building both in a single CMake-configured-and-generated binary tree.
And for maintaining backwards-compatibility because that's the way
it's been for a while now...

You may override the output directories on a per-configuration basis
using the target properties:

  http://cmake.org/cmake/help/cmake-2-8-docs.html#prop_tgt:LIBRARY_OUTPUT_DIRECTORY_CONFIG
  http://cmake.org/cmake/help/cmake-2-8-docs.html#prop_tgt:RUNTIME_OUTPUT_DIRECTORY_CONFIG

If you set those properties, nothing will be appended to the value you
give. So you could set the properties LIBRARY_OUTPUT_DIRECTORY_RELEASE
and RUNTIME_OUTPUT_DIRECTORY_RELEASE to be
${CMAKE_BINARY_DIR}/Wrapping/CSharpBinaries and the dlls will end up
in there. Set the _DEBUG variants too, if you also want Debug builds
to put their library and executable files there.


>>
>>
>>
>> 2. Determining the platform in Linux:
>> On Windows using Visual Studio generator, there is a VS macro
>> $(PlatformShortName) which allows me to detect the platform name (x86
>> or x64), eg:
>>   add_custom_command(
>>    COMMENT "Compiling C# ${type} ${name}"
>>    OUTPUT ${name}.${output}
>>    COMMAND ${CSHARP_COMPILER}
>>    ARGS /t:${type} /out:${name}.${output}
>> /platform:$(PlatformShortName) ${refs} ${sources} # <<< Use platform
>> here
>>    WORKING_DIRECTORY ${CSHARP_BINARY_DIRECTORY}
>>    DEPENDS "${sources_dep}"
>>  )
>>
>> Is there an equivalent to $(PlatformShortName) in UNIX make files? How
>> can I determine the platform at CMake configuration/generation time
>> and/or compile time under Linux?
>>
>>
>> This sounds like to you need a CMake Guru to help you here. I CC'd David
>> Cole for hopes of a response.
>>

This #2 question, I simply do not know. A Linux+Mono guru is what you
need here. If I were to do this from a purely CMake/C++-ish angle, I'd
set a variable "arch" to either "x86" or "x86_64" based on whether the
C++ compile is a 64-bit build or not...

Something like:

if(MSVC)
  set(arch "$(PlatformShortName)")
elseif(CMAKE_SIZEOF_VOID_P EQUAL 8)
  set(arch "x86_64")
else()
  set(arch "x86")
endif()

Then use ${arch} where you are presently using $(PlatformShortName) ...


HTH,
David C.



>> 3. PixelIDValueEnum is too complex for some parsers/compilers:
>> sitkPixelIDValues.h defines the following enum:
>>  enum PixelIDValueEnum {
>>    sitkUnknown = -1,
>>    sitkUInt8 = PixelIDToPixelIDValue< BasicPixelID<uint8_t>
>>
>> ::Result,   // Unsigned 8 bit integer
>>
>>    sitkInt8 = PixelIDToPixelIDValue< BasicPixelID<int8_t> >::Result,
>>   // Signed 8 bit integer
>>    sitkUInt16 = PixelIDToPixelIDValue< BasicPixelID<uint16_t>
>>
>> ::Result, // Unsigned 16 bit integer
>>
>>    ...
>>  }
>>
>> I get a number of strange issues related to the (complex) nature of
>> the templated code assigning the enum values.
>>
>> These issues are two fold:
>> (a) the SWIG parser does not seem to be able to determine the integer
>> enum value. Here is the invalid C# code it generates:
>>  public enum PixelIDValueEnum {
>>    sitkUnknown = -1,
>>    sitkUInt8 = PixelIDToPixelIDValue<(BasicPixelID<(uint8_t)>)>::Result,
>>    sitkInt8 = PixelIDToPixelIDValue<(BasicPixelID<(int8_t)>)>::Result,
>>    ...
>>  }
>> SWIG should be generating code like this (and does so nicely for other
>> simpler SimpleITK enums):
>>  public enum PixelIDValueEnum {
>>    sitkUnknown = -1,
>>    sitkUInt8 = 1,
>>    sitkInt8 = 2,
>>    ...
>>  }
>> Fortunately I can turn on "type safe enum" wrapping, which generates
>> valid (but not so nice) C# code like this:
>>  public sealed class PixelIDValueEnum {
>>    public static readonly PixelIDValueEnum sitkUnknown = new
>> PixelIDValueEnum("sitkUnknown", SimpleITKPINVOKE.sitkUnknown_get());
>>    public static readonly PixelIDValueEnum sitkUInt8 = new
>> PixelIDValueEnum("sitkUInt8", SimpleITKPINVOKE.sitkUInt8_get());
>>    public static readonly PixelIDValueEnum sitkInt8 = new
>> PixelIDValueEnum("sitkInt8", SimpleITKPINVOKE.sitkInt8_get());
>>    ...
>>  }
>>
>> I am glad the you have found a work around. While the C++ compiler can
>> figure out the wacky meta-programmed PixelID value at compile time, it just
>> too much for most other tools. Most of the target languages, just have some
>> constant variables or similar structure to represent the PixelIDValueEnum.
>> If the "type safe enum" is just too "not so nice", I suppose we could create
>> an executable which prints the actual value of the enums. This output could
>> then be fed into SWIG to make a more suitable interface.
>
> I think type safe enums are fine for the moment. Just wanted to
> mention it to see if other languages have s similar issue. (I forgot
> to mention that Java has the same issue, but uses the type safe enums
> by default so it worked out-of-the-box).
>
>> (b) When compiling SimpleITK on Linux, I get lots of warnings stemming
>> from PixelIDValueEnum:
>> gcc version 4.5.2 (Ubuntu/Linaro 4.5.2-8ubuntu4)
>>
>>
>> OK, most of that was just the instantiation structure of simpleITK's meta
>> programming. I would recommend using an in editor compiler, where it
>> highlights the error message and you can go directly to the cause of the
>> error. Emacs easily does this.
>> The following is the only actual warning is the block you sent:
>>
>> /home/dan/ITK/SimpleITK/Code/Common/sitkMemberFunctionFactory.txx:86:7:
>> warning: case value ‘2’ not in enumerated type ‘itk::Image<signed
>> char, 3u>::<anonymous enum>’
>>
>> This is rather unfortunate, and I it's most definitely a false positive. Do
>> you know if there is a warning suppression for this particular message.
>> It's caused by the usage of the following to define the ImageDimension
>> member for the image classes, ( in ITK ):
>> #define itkStaticConstMacro(name, type, value) enum { name = value }
>> ( it would be nice is we could just use the static constant feature of
>> C++0x, which has been a compiler extension for a while on many platforms )
>> And the following code in SimpleITK:
>>  switch( TImageType::ImageDimension )
>>       {
>>       case 3:
>>         Superclass::m_PFunction3[ pixelID ] = Superclass::BindObject( pfunc,
>> m_ObjectPointer );
>>         break;
>>       case 2:
>>         Superclass::m_PFunction2[ pixelID ] = Superclass::BindObject( pfunc,
>> m_ObjectPointer );
>>         break;
>>       default:
>>         break;
>>       }
>> Perhaps the following should quiet it:
>>  switch( int(TImageType::ImageDimension) )
>>       {
>> Additionally, there has just been a long topic to fix many warning just
>> pushed to master. We are working on it. However, this particular one is new
>> to me.
>>
>>
>> I'm not sure anything can be done to help SWIG or gcc with the
>> PixelIDValueEnum complexities, but I thought I should mention it...
>>
>> 4. Process for contributing changes:
>> I guess this is a question for the SimpleITK people...
>> What is the process to contribute these changes? I have cloned the
>> SimpleITK.git repository and have made the changes against this. Do I
>> simply push them to gerrit for review?
>>
>> Which repo have you clone? We recently moved the main one from git hub to
>> kitware, but git hub is still a mirror.
>> Basic info about the repository can be found here:
>> http://www.itk.org/Wiki/ITK_Release_4/SimpleITK
>> You should be able to use either gerrit, github, or as a last resort an
>> e-mail patch to contribute.
>> It depends on what you are most comfortable with. SimpleITK is currently a
>> small development team, so we have not had the opportunity to develop
>> detailed documentation for these procedures.
>
> I cloned http://itk.org/SimpleITK.git.
>
> I will submit my changes to gerrit in the coming days.
>
>>
>> 5. SimpleITK stability:
>> At the moment there is only one C# example.
>> How stable is SimpleITK? Is it stable enough that I can spend some
>> time writing more C# examples? Or will things change?
>>
>> We are planning on taking an alpha tomorrow. I don't expect there to be wild
>> changes in much of the API, just tweaks and what not. Along with expanding
>> to new features.
>> Writing new Examples woulds very much be appreciated. It is also important
>> that they get executed as test to ensure they remain up to date.
>> Additionally, it would help to ensure the stability of the C# interface.
>> Please let us know how your experience is using the SImpleITK interface, and
>> you are welcome to join the TCON to provide feedback, which is held on
>> wednesdays at 9:30am EST.
>
> This sounds pretty stable -- I will write more examples. I will also
> make sure I enable them for testing.
>