[philippose]

Hello again Oliver and Bruno,

A Good Evening!

So... I guess after that initial out-burst... people seem to have settled down :-)!

I am back home now, and did some deep digging into the depths of C and C++.... and these are the points I discovered:

1. The run-time array construction approach used in some parts (I have also seen the more typical "new-delete" approach in Engrid) of the code is known as "Variable Length Arrays (VLAS)", and was introduced into the C99 standards. Since it is a "C" standard and not a "C++" Standard, only some compilers have support for this construct, and GCC is one of them, and MSVC is not one of them and does not plan to be one them.

However, I do not think this is a very safe method to use for dynamic allocation of possibly very large arrays as in the case of Engrid, because the VLAS allocates memory for the arrays on the stack and not on the heap.

By default, Linux has a stack size of 8MB and Windows has a stack size of only 1MB (Ref: http://www.cs.nyu.edu/exact/core/doc/stackOverflow.txt), whereas the heap-size as usual, goes up to about 2GB (or 4GB in Linux?). This could lead to potential segmentation faults and stack-overflows.

I also checked up the C++0x (upcoming C++ standards), and I don't think this feature is going to be included into these standards too... which implies that it will remain a "C" Standard and will not become a "C++" Standard.

A few days ago when I got Engrid to compile on MSVC++ in Windows, I replaced all these constructs with "QVector<vtkIDType>" and it worked fine.... this might be the better way to go.



2. The "or" construct is called an "operator synonym" and was introduced into C++ in 1998 (Ref: http://en.wikipedia.org/wiki/Operators_in_C_and_C%2B%2B ..... riiight at the bottom of the article), but remains still as one of those features which live dangerously on the edge of the standards. Hence, this construct is also not available on most C++ compilers.... and again, GCC is one of those compilers which does support it, and as far as I can see, MSVC++ does not support it, and I think most C++ programmers would not have even heard of it.

Again, I would suggest a shift back to the standard operators as soon as possible not only to maintain compatibility, but also to improve the consistency of the code.



3. The Template "error".... is actually not an error, but again, a manifestation of the "Variable Length Arrays" construct, because the Template variable was assigned to a normal variable, and this normal variable was then used to define the array.... which is basically, a run-time allocation of the array using a variable for the size.




If Bruno and you are interested, my suggestion would be to try and quickly spend say.... around 4 days to a week (I am quite sure this will be more than sufficient) to modify the current state of the code in the Git Repository to make it compliant with the C++ standards (We could say.... compile capability using MSVC++ Express 2008 == standards compliant ??), and once that is done, to change the programming style used in the further development of Engrid to stick to normal everyday C++ as much as possible.

Is this ok? Or do you think it does not make sense to go out of the way to ensure that the code compiles on anything else other than GCC (and its variants)?


Have a great evening ahead!

Philippose

[ogloth]

Hello,

I guess the easiest would be to use the GIT repository on SourceForge.

A branch "named" windows is now in the repository on SourceForge. Both of you probably have SourceForge accounts already, so please send me a request to join and then you can have write access to the repository.

There is also a Mantis bug-tracker on SourceForge which we should start using for enGrid ... I guess I should make some sort of announcement to promote this.

A quick answer to Philippose comments:
1. I have no problem to use the QVector<> variant -- I guess the vtkIdType pts[n] version was shear laziness ...

2. As I said I wasn't even aware of this; it might have been an effect of writing some Python code in-between ... I completely agree with your proposal

3. For the template problem I suggest to use new and delete[] instead of QVector<>, because there might be performance issues otherwise.

Btw., thanks for offering to help!


Cheers,
Oliver

[philippose]

Hello Oliver,

A Good Afternoon to you!

Thanks a lot for your response.

Day before yesterday, I was able to successfully compile the latest Git version of Engrid on Windows using Qt-4.7.1 (LGPL) + Microsoft Visual C++ 2008 Express.

I made the changes I suggested in the post and used "QVector" for the arrays... Qt claims that QVector is extremely quick and that there should be no directly visible loss in performance... so we could probably use this and see if it makes a difference and consider a switch to the classical "new - delete []" approach if required.

Basically I compiled netgen within your framework (i.e.... also as a static lib)... and then compiled libengrid.... and finally compiled the engrid application.

I had to explicitly export "guiMainWindow" and its destructor from the libengrid DLL because Windows by default does not export any symbols in a DLL unless asked to (I think we need to think of another approach for this..... do you really want even the main GUI functionality to come from the libengrid.dll file? Or is it still work-in-progress?)

Now, everything seems to be working except for the fact that for some reason QFileDialog (used to open a dialog when loading or saving files) keeps crashing on my system..... but I confirmed that this has nothing to do with your code.... I am still looking into this....

I have not gone deeper into the functionality because I have been unable to load any geometry file into Engrid yet because of this QFileDialog issue...

Shall send you a request for write access to the Git repo today..... and push the version as it is so far.... we can then see what else needs to be done.

Have a great day!

Philippose

[wyldckat]

Greetings to all!

Philippose, as soon as you can commit your modifications to the windows branch, I can test them with Qt 4.5.1 I have here at the office.
As for the problem you're having could be related to Qt 4.7 having changed some specs internally on how to handle the file open dialog. It honestly doesn't seem very logical that the version were to be blamed, but with the experience I've had with building ParaView + Qt, leads me to believe otherwise.

As for QVector vs new/delete: QVector is essentially a wrapper for new/delete, with the added bonus of a few helpful methods. The compiler should do a pretty good job of making the wrapper seem-less.
QList on the other hand might not be as efficient as QVector in these cases, since it's devised to work with morphing lists. But since we're talking about QVector, then there shouldn't be any problems.

Best regards,
Bruno