VC2010 & ImageCalibration

[NKV]

Hi,
I have compiled x32 version ImageCalibration and x64 IntensityTransformations under Win7 x64 VS2010.
But I getting error during compilation x64 ImageCalibration module. See log below.

Whats wrong ?

Best regards,
Nikolay.

Code: [Select]

1>------ Build started: Project: ImageCalibration, Configuration: Release x64 ------
1>Build started 07.09.2011 8:49:03.
1>InitializeBuildStatus:
1>  Creating "x64\Release\ImageCalibration.unsuccessfulbuild" because "AlwaysCreate" was specified.
1>ClCompile:
1>  DefectMapInstance.cpp
1>  DefectMapInterface.cpp
1>  DefectMapParameters.cpp
1>  DefectMapProcess.cpp
1>  ImageCalibrationInstance.cpp
1>..\..\ImageCalibrationInstance.cpp(484): warning C4305: 'initializing' : truncation from 'double' to 'const float'
1>..\..\ImageCalibrationInstance.cpp(593): warning C4305: 'initializing' : truncation from 'double' to 'const float'
1>..\..\ImageCalibrationInstance.cpp(833): warning C4099: 'pcl::CalibrationThreadData' : type name first seen using 'class' now seen using 'struct'
1>          c:\pcl\src\modules\processes\imagecalibration\ImageCalibrationInstance.h(66) : see declaration of 'pcl::CalibrationThreadData'
1>..\..\ImageCalibrationInstance.cpp(1193): warning C4267: 'argument' : conversion from 'size_t' to 'int', possible loss of data
1>..\..\ImageCalibrationInstance.cpp(1225): warning C4267: 'argument' : conversion from 'size_t' to 'int', possible loss of data
1>..\..\ImageCalibrationInstance.cpp(2195): warning C4267: '+=' : conversion from 'size_t' to 'int', possible loss of data
1>  ImageCalibrationInterface.cpp
1>..\..\ImageCalibrationInterface.cpp(191): warning C4267: 'argument' : conversion from 'size_t' to 'int', possible loss of data
1>  ImageCalibrationModule.cpp
1>  ImageCalibrationParameters.cpp
1>  ImageCalibrationProcess.cpp
1>Link:
1>     Creating library x64\Release\ImageCalibration.lib and object x64\Release\ImageCalibration.exp
1>  Generating code
1>c:\pcl\src\modules\processes\imagecalibration\imagecalibrationinstance.cpp(2336): fatal error C1001: An internal error has occurred in the compiler.
1>  (compiler file 'f:\dd\vctools\compiler\utc\src\p2\main.c[0x5F212D0C:0x00000028]', line 183)
1>   To work around this problem, try simplifying or changing the program near the locations listed above.
1>  Please choose the Technical Support command on the Visual C++
1>   Help menu, or open the Technical Support help file for more information
1> 
1>LINK : fatal error LNK1000: Internal error during IMAGE::BuildImage
1> 
1>    Version 10.00.40219.01
1> 
1>    ExceptionCode            = C0000005
1>    ExceptionFlags           = 00000000
1>    ExceptionAddress         = 5F212D0C (5F0D0000) "C:\Program Files (x86)\Microsoft Visual Studio 10.0\VC\bin\x86_amd64\c2.dll"
1>    NumberParameters         = 00000002
1>    ExceptionInformation[ 0] = 00000000
1>    ExceptionInformation[ 1] = 00000028
1> 
1>  CONTEXT:
1>    Eax    = 07190290  Esp    = 0038E8F4
1>    Ebx    = 00000000  Ebp    = 0038E928
1>    Ecx    = 00000008  Esi    = 00000000
1>    Edx    = 072A39D0  Edi    = 071B5838
1>    Eip    = 5F212D0C  EFlags = 00010297
1>    SegCs  = 00000023  SegDs  = 0000002B
1>    SegSs  = 0000002B  SegEs  = 0000002B
1>    SegFs  = 00000053  SegGs  = 0000002B
1>    Dr0    = 00000000  Dr3    = 00000000
1>    Dr1    = 00000000  Dr6    = 00000000
1>    Dr2    = 00000000  Dr7    = 00000000
1>
1>Build FAILED.
1>
1>Time Elapsed 00:01:00.66
========== Build: 0 succeeded, 1 failed, 0 up-to-date, 0 skipped ==========


[Juan Conejero]

Hi Nikolay,

The 64-bit Visual C++ compiler is buggy. This is a good example. I have been living with hundreds of compiler crashes like this one in VC++ 2008 during years. The 64-bit compiler in VC++ 2010 is better, but it still crashes sometimes. A real shame.

The only solution is turning off or lowering optimizations. Open the .vcproj file with Visual Studio and select "Custom" as the optimization level (it is -O2 by default). This usually helps. Otherwise you'll have to disable optimization completely for the file that is causing the compiler crash. Good luck!

[NKV]

The 64-bit Visual C++ compiler is buggy.

You are right !!!
Take the bug and solution:
In the ImageCalibrationInstance::ExecuteGlobal() there are code like that:

Code: [Select]

try
{
for(;;)
{
try
{
break;
continue;
}
catch (...)
{
}
}
}
catch (...)
{
}

if you comment operator "continue ;" >> "//continue ;"
or define condition in cycle "for ( ; ; )" >> "for (;true; )"
the compilation will successful. 

[NKV]

Hi Juan,
It's also bug of VC2010 or ?
Look at comment in line 125 in AberrationSpotterInstance.cpp http://pixinsight.com/forum/index.php?topic=3411.0

Note: Module compiled under VC2008 work without problem.

Code: [Select]

template <class P>
class CopyRect
{
public:
CopyRect(const Generic2DImage<P>& source, Generic2DImage<P>* target, const int& size)
:s(source), t(target), size(size) //conststructor with 3 arguments
{
r.ResizeTo(size,size);
}

void Copy(const int& sX, const int& sY, const int& tX, const int& tY)
{
Console().Write(""); // Without this line the route work not properly. sX = sX-size. Why?
r.MoveTo(sX,sY);
s.SelectRectangle(r);
t->SelectPoint(tX,tY);
t->Apply(s);
}

private:
const Generic2DImage<P>& s;
Generic2DImage<P>* t;
const int& size;
Rect r;
};

[Juan Conejero]

Hi Nikolay,

If I understand it well, this code compiles on VC++ 2008 and VC++ 2010, but for some odd reason you need to call "Console.Write("");" or otherwise it doesn't work as expected???

If this is a compiler bug, then it is extraordinary. Now I understand a few things about Windows 

Could you be more specific? What's what isn't working well without that line and why?

A general recommendation: don't use references to constant integers as function parameters. An int occupies 4 bytes in memory, but a reference (which ultimately is a pointer) requires 4 bytes (32-bit) or 8 bytes (64-bit) and an additional indirection. It is much more efficient passing basic types (char, int, float, double) directly by value instead of by reference to const. Any competent C++ compiler will optimize this out anyway so there is no practical performance penalty, but this is much more readable and elegant:

void foo( int bar )

than this:

void foo( const int& bar )

[NKV]

this code compiles on VC++ 2008 and VC++ 2010

Yes, compilation going without problem (no warning, no error during compilation under both compiler versions.)

, but for some odd reason you need to call "Console.Write("");" or otherwise it doesn't work as expected???

Only VC2010. Module compiled under VC2010 at Win7x64 it doesn't work as expected.

Could you be more specific? What's what isn't working well without that line and why?

Without "Console.Write("");" operator "r.MoveTo(sX,sY);" move rectangle to wrong position: "r.MoveTo(sX-size,sY);" 

A general recommendation:

Thank you for the lesson. I going to rewrite the module.

[Juan Conejero]

Without "Console.Write("");" operator "r.MoveTo(sX,sY);" move rectangle to wrong position: "r.MoveTo(sX-size,sY);"

Interesting

There is something in your code that is very strange. First you declare a data member:

private:
   const int& size;

Then you declare the following constructor:

   CopyRect(const Generic2DImage<P>& source, Generic2DImage<P>* target, const int& size)
      :s(source), t(target), size(size) //conststructor with 3 arguments
   {
      r.ResizeTo(size,size);
   }

I don't quite understand how this code compiles. It shouldn't because there is an evident  syntax error: size is both a function argument and a data member. The initializer:

..., size(size)

is obviously an indetermination that should not pass any minimally decent C++ lexical analyzer. If this code compiles then I am not surprised at all that you get any sort of weird result.

I would rewrite your class as follows:

Code: [Select]

template <class P>
class CopyRect
{
public:

   CopyRect( const Generic2DImage<P>& source, Generic2DImage<P>& target, int size ) :
   m_source( source ), m_target( target ), m_size( size ), m_rect( 0 )
   {
      m_rect.ResizeTo( m_size, m_size );
   }

   void Copy( int sX, int sY, int tX, int tY )
   {
      m_rect.MoveTo( sX, sY );
      m_source.SelectRectangle( m_rect );
      m_target.SelectPoint( tX, tY );
      m_target.Apply( m_source );
   }

private:

   const Generic2DImage<P>& m_source;
   Generic2DImage<P>&       m_target;
   int                      m_size;
   Rect                     m_rect;
};

[Juan Conejero]

Also, in PixInsight/PCL/PJSR it is customary to specify source and target arguments in the "Intel order", that is target, source. For the sake of coherence with PCL classes and functions, your constructor should be:

   CopyRect( Generic2DImage<P>& target, const Generic2DImage<P>& source, int size ) :

instead of:

   CopyRect( const Generic2DImage<P>& source, Generic2DImage<P>& target, int size ) :

Think in terms of:

target <- source

instead of:

source -> target

If you know some 80x86 assembler, then you can think as in:

mov ax, bx

Of course, all this stuff is optional, but advisable IMO.

[Juan Conejero]

And a final follow-up. In your original code, you declared a reference data member:

const int& size;

This is dangerous. If you initialize your class passing a reference to an automatic variable as the size argument, and that variable gets out-of-scope before all class instances are destroyed, you can expect wrong behaviors of any kind, ranging from spurious results to a core dump.

The reason is that a reference to an object assumes that the object referenced exists, or otherwise the reference becomes invalid (and poison). For example:

class Foo
{
public:
   Foo( int& bar ) : m_bar( bar )
   {
   }

   void stuff( int what )
   {
      m_bar = what;
   }

private:
   int& m_bar;
};

Foo f1()
{
   int bar = 0;
   return Foo( bar );
}

void f2()
{
   Foo foo = f1();
   foo.stuff( 3 ); // ERROR: f1()'s bar does not exist
}

In the example, the m_bar member of foo references a nonexistent (out-of-scope) automatic variable when its stuff() member function is invoked in f2(). This is perhaps a too evident example but I can show you much worse and subtle side effects. Use reference data members strictly when necessary, or much better, try to avoid them whenever possible. If your classes own their data then you'll be on the safe side.

[NKV]

I would rewrite your class as follows:

Code: [Select]

   CopyRect( const Generic2DImage<P>& source, Generic2DImage<P>& target, int size ) :
   m_source( source ), m_target( target ), m_size( size ), m_rect( 0 )
   

It's very helpful! ", m_rect( 0 )"  - it is key to solve. Now the module work fine.

Juan, thank you for your help. Especially for C++ lessons.

[Juan Conejero]

Nikolay,

You're welcome. I'm always glad to see more active development on PixInsight --it is our main goal, in fact. So thank you for getting involved in PCL development.