Reading .ot (Visual Studio): tree type isn't registered therefore read() fails

[merfels]

Hi,

As discussed in the Google Groups, there seems to be an issue with Visual Studio and the File Factory pattern when trying to read in .ot trees. The expected behavior is that the tree type should be recognized when being read from file, but the static member initialization technique which should register the tree type doesn't work in this setup, which results in an unrecognized tree type when trying to load it.

System: Windows 7 64 bit, Windows SDK 7.1, Visual Studio 2010, octomap-1.6.1

Example which shows the behavior:

OcTree tree (0.01);
tree.write("simple_tree.ot");
AbstractOcTree * readTree = AbstractOcTree::read("simple_tree.ot");
if(readTree) {
OcTree * storedTree = dynamic_cast<OcTree*>(readTree);
} else {
// couldn't load tree
}

The tree can not be read in from file. The problem is that the instantiation of a new OcTree should result in the creation of the static StaticMemberInitializer ocTreeMemberInit which is defined in OcTree.h and subsequently register the tree type. Loading from file could then associate the stored file with this tree type. This seems to be an issue with the Visual Studio compiler.

One possible workaround is to put a function "void dummy() {} ;" in the class StaticMemberInitializer and calling it from within the OcTree constructor. I tried that and apparently this doesn't get optimized away, but it adds of course some bloat and is uncleaner / less comprehensible.

Best,
Christian

[ahornung]

Thanks for your detailed analysis. I agree that a cleaner solution would be preferable. What about adding volatile to static StaticMemberInitializer ocTreeMemberInit;? Another option would be adding a VS-specific pragma:

#pragma optimize( "", off )
//...
#pragma optimize( "", on)

http://msdn.microsoft.com/en-us/library/chh3fb0k.aspx

In any case, the smallest possible change that helps would be desirable since it has to be added to every derived octree class.

[merfels]

I tried adding the volatile keyword and disabling optimization but that didn't help. I struggle to find out the real cause behind this behavior so finding a good solution is hard. If you have any more ideas then I'm happy to help you.

[ahornung]

Alas, I don't really know Visual Studio much (don't even have it running here) so I don't know how to tell their compiler to behave. There should be no reason to optimize that code out in the first place. I found some similar reports on the net, that's why I was hoping that either volatile or the optimize pragma would help:
http://stackoverflow.com/questions/1229430/how-do-i-prevent-my-unused-global-variables-being-compiled-out/
http://stackoverflow.com/a/12243667
http://bytes.com/topic/net/answers/497057-static-instance-class-optimized-out-existence

[merfels]

Thanks for the additional input. I agree that the compiler should not discard that code as it clearly has side-effects and it is kind of frustrating to find the right tweak to get it to work. For now, I will probably go for my own derived tree class with the workaround I proposed earlier, which isn't the cleanest solution but works just fine.

[ahornung]

I'm inclined to close this issue. The "test_color_tree" unit test succeeds with a current MSVC (Visual Studio 2013 and 2015, Win7 x64). Since I can't reproduce the issue I'm assuming it's fixed in the compiler.

[merfels]

Closing the issue is fine with me. I am still using the proposed workaround under MSVC 2010. As I don't have MSVC 2013 or 2015, I can't confirm whether or not the test_color_tree unit test works there.

[ahornung]

Seems like that was too early, the problem still persists but manifests differently with MSVC 2013 / 2015. Now ColorOcTree works but no longer OcTree (causing test_io to fail).

Here's another pointer that could work (actually your workaround hidden in a #define):
http://stackoverflow.com/questions/599035/force-visual-studio-to-link-all-symbols-in-a-lib-file

[ahornung]

There's now a pull request with the dummy function you described: #101

Does that look OK to you, @merfels? The question is whether to make it transparent or hide it in a #define.

[merfels]

That's exactly how I implemented it and it works fine under VS 2010.

I personally prefer to not put in a #define as things get less clear with more #define's. If people want to write their own tree implementation, they need to know what this workaround does. My suggestion is to put a comment in Doxygen explaining the issue and linking to this page.

[ahornung]

Yes, I agree that it's the best to have it as clear as possible!

[ahornung]

Merged into devel now, closing!

[leavesnight]

is this problem solved now?

[leavesnight]

System: Windows 8 64 bit, Visual Studio 2012, octomap-1.8.0, qt 5
I've found the octovis can not open the ".ot" file which uses the class ColorOcTree.
So after some time of my debug work, I localize the problem at the register in the classIDMapping() which shows that the key named "ColorOcTree" is never registered through the whole running.
Then I've tried a lot and finally find there's even no ColorOcTree.h is included. therefore, I add
"#include <octomap/ColorOcTree.h>" in the main.cpp of the octovis project.
But it still not work, I guess that's the reason the compiler omits the head file because the class declared in that head file is never used. then I add a new line to define one global variable.
"octomap::ColorOcTree colortreeTmp(0);"
Then the problem is solved. the octovis can succeed in opening the ".ot" file.
And I try to delete the dummy functions supposed by the post starter.
"//void ensureLinking() {};" in OcTree.h and ColorOcTree.h
"//ocTreeMemberInit.ensureLinking();" in OcTree.cpp
"//colorOcTreeMemberInit.ensureLinking();" in ColorOcTree.cpp
And I tried the case supposed by the questioner, and I found no problem with these deletions.
Any way, with these adds it's of course no problem. but I still want to point out the problem in the octovis project.
And the final solution I found is to add two lines in the main.cpp:

#include <octomap/ColorOcTree.h>
octomap::ColorOcTree colortreeTmp(0);

[ahornung]

Yes, this should have been fixed already.

Can you run the unit tests and post their output? There are specific tests for the classIdMapping, and last I checked they ran successfully under Windows.

ColorOcTree.h is included from ColorOcTreeDrawer.h, which in turn is included through ViewerGui.cpp.

[leavesnight]

Do you mean the result of the "ctest -C Release" command?
If so, the result is truly successful as the first time I've built it. It's like this:
[HANDLER_OUTPUT]
Test project D:/0Interest/1Programme/C++/octomap-devel/build

  Start  1: MathVector

1/14 Test #1: MathVector ....................... Passed 0.01 sec
Start 2: MathPose
2/14 Test #2: MathPose ......................... Passed 0.01 sec
Start 3: InsertRay
3/14 Test #3: InsertRay ........................ Passed 1.37 sec
Start 4: InsertScan
4/14 Test #4: InsertScan ....................... Passed 1.30 sec
Start 5: ReadGraph
5/14 Test #5: ReadGraph ........................ Passed 0.05 sec
Start 6: StampedTree
6/14 Test #6: StampedTree ...................... Passed 1.07 sec
Start 7: OcTreeKey
7/14 Test #7: OcTreeKey ........................ Passed 0.02 sec
Start 8: test_scans
8/14 Test #8: test_scans ....................... Passed 0.23 sec
Start 9: test_raycasting
9/14 Test #9: test_raycasting .................. Passed 3.26 sec
Start 10: test_io
10/14 Test #10: test_io .......................... Passed 0.96 sec
Start 11: test_pruning
11/14 Test #11: test_pruning ..................... Passed 0.04 sec
Start 12: test_iterators
12/14 Test #12: test_iterators ................... Passed 0.89 sec
Start 13: test_mapcollection
13/14 Test #13: test_mapcollection ............... Passed 0.01 sec
Start 14: test_color_tree
14/14 Test #14: test_color_tree .................. Passed 0.13 sec

100% tests passed, 0 tests failed out of 14

Total Test time (real) = 9.53 sec
And I found what you said. the ColorOcTree.h is really included in ViewGui.cpp while not included in main.cpp. And my problem still holds(octovis cannot open ".ot" files) unless I add this definition in ViewGui.cpp or add two lines as I mentioned last time:
#include <octovis/ViewerGui.h>
#include <octovis/ColorOcTreeDrawer.h>
#include <octomap/MapCollection.h>
octomap::ColorOcTree colortreeTmp(0);//add one definition
And I think it may be still the reason that the compiler omits the head file because the class declared in that head file is never used...

[ahornung]

Which line exactly are you adding to ViewerGui.cpp? Can you submit a pull request with this (minimal) workaround?

[leavesnight]

Ok.
I've submitted it just now~

[ahornung]

Thanks!