scaffolding for PDAL 1.0.0b1 support

[hobu]

Chris,

This implements just about everything you need to take in and read any PDAL-readable with the current codebase.

Howard

[c42f]

Great, thanks for the patch!

I got around to updating and building the latest pdal but unfortunately it just segfaults with this patch applied. Will need a little digging.

In the process, it looks like my cmake build system needs some love to work well in the case that the external libs aren't built automatically. The dvox utility is also causing some problems since it assumes laslib is available. (dvox is experimental anyway so I should probably just disable building it by default.)

[hobu]

I will rebase with your latest master and make another attempt. I never did get to testing the code, and I just made sure it compiled past the PDAL stuff.

[hobu]

I'm issuing cmake with the following:

cmake  -DDISPLAZ_USE_LAS=ON -DDISPLAZ_USE_PDAL=ON -DPDAL_INCLUDE_DIR="/Users/hobu/dev/git/pdal/include;/Users/hobu/dev/git/pdal/util" -DPDAL_LIBRARY=/Users/hobu/dev/git/pdal/lib/libpdalcpp.dylib .

I keep getting stuff like

[hobu@pyro displaz (master)]$ make
[  1%] Built target doc
[  2%] Performing build step for 'ilmbase'
[  4%] Built target eLut
[  9%] Built target toFloat
make[5]: *** No rule to make target `/Users/hobu/dev/git/displaz/thirdparty/external/ilmbase-prefix/src/ilmbase/Half/toFloat.h', needed by `Half/CMakeFiles/Half.dir/half.cpp.o'.  Stop.
make[4]: *** [Half/CMakeFiles/Half.dir/all] Error 2
make[3]: *** [all] Error 2
make[2]: *** [thirdparty/external/ilmbase-prefix/src/ilmbase-stamp/ilmbase-buil

[c42f]

Hi Howard no problems, I didn't expect it to work first time given my build system has broken for you in weird and wonderful ways.

I've done a few cleanups to the build and the code which might help you, including turning off the build of the experimental dvox utility by default and fixing a couple of missing target level dependencies against the automatically built external packages.

Your error seems to be related to building the external ilmbase package, in particular toFloat.h is an autogenerated file, though not sure why that would be a problem on your system but not mine. It's possible to disable building of third party deps using -DDISPLAZ_BUILD_EXTERNAL=FALSE. ilmbase may be built with the following

cd $ILMBASE_DIR
wget http://download.savannah.nongnu.org/releases/openexr/ilmbase-2.2.0.tar.gz
tar -xzf ilmbase-2.2.0.tar.gz
rm -Rf ilmbase-build
mkdir ilmbase-build && cd ilmbase-build
cmake -DNAMESPACE_VERSIONING=OFF -DBUILD_SHARED_LIBS=OFF -DCMAKE_BUILD_TYPE=Release -DCMAKE_INSTALL_PREFIX=$ILMBASE_DIR/ilmbase-install ../ilmbase-2.2.0
make -j4 install

I followed that by

mkdir build
cd build
cmake -DDISPLAZ_BUILD_EXTERNAL=FALSE \
    -DILMBASE_INCLUDES=$ILMBASE_DIR/ilmbase-install/include/ \
    -DILMBASE_LIBRARIES="$ILMBASE_DIR/ilmbase-install/lib/libImath.a;$ILMBASE_DIR/ilmbase-install/lib/libIex.a" \
    -DDISPLAZ_USE_LAS=ON -DDISPLAZ_USE_PDAL=ON \
    -DPDAL_INCLUDE_DIR="/home/chris/src/pdal/include/;/home/chris/src/pdal/util" \
    -DPDAL_LIBRARY=/home/chris/local/lib/libpdalcpp.so ..
make -j4

[c42f]

Strangely, with the above build setup and incorporating the various cleanups from master, the segfault has now disappeared - could have been something weird in my build setup I suppose.

However, the above code is a lot slower (about 5x) than the corresponding code using lastools, which if I remember correctly wasn't the case with the original pdal support. Any thoughts about the "right" way to address this? My eye is drawn to the big switch statement inside PointBuffer::getFieldAs which can't be good inside an inner loop. It's a pity to pay such a high price for the ability to read data as a user-specified type.

[abellgithub]

Did you build PDAL in release mode (-DCMAKE_BUILD_TYPE=Release)?

We've looked into the code that you mention and we've found very little overhead when optimized.

[c42f]

I definitely checked for Release mode as a first step. I could be doing something stupid though in my build environment - I did have a very old version of pdal installed previously. I'll make sure to blow away all remnants of the install and start from scratch again.

[c42f]

Well, I've torn apart my build setup, deleted previously installed versions, and rebuilt everything but I'm still seeing pdal to be a lot slower (~5x) than expected. I did the same for laszip just to be sure. Here's the exact settings:

laszip:

cd ~/src/laszip
mkdir build
cd build
cmake -DCMAKE_INSTALL_PREFIX=$HOME/local -DCMAKE_BUILD_TYPE=Release ..
make -j8 install

pdal:

cd ~/src/pdal
mkdir build
cd build
cmake   \
    -DCMAKE_BUILD_TYPE=Release \
    -DCMAKE_INSTALL_PREFIX=$HOME/local \
    -DWITH_PYTHON=OFF \
    -DWITH_TESTS=OFF \
    -DWITH_LASZIP=ON \
    -DWITH_GEOTIFF:BOOL=OFF \
    -DLASZIP_INCLUDE_DIR=$HOME/local/include \
    -DLASZIP_LIBRARY=$HOME/local/lib/liblaszip.so \
    -DCMAKE_CXX_FLAGS=-std=c++11 \
    -DBUILD_PLUGIN_PGPOINTCLOUD:BOOL=OFF \
    ..
make -j8 install

I had a quick look at some approximate profiles with the valgrind callgrind tool (valigrind --tool=callgrind --simulate-cache=yes) which shows 75% of cycle estimation spent inside LasReader::loadPointV10(), with a lot of it inside C++ standard library stream machinery. Looking in that function I think you should really avoid involving the standard streams for binary unpacking, especially at the level of a single point. With a quick hack I was able to make las reading go from 14.87s to 6.62s by removing a lot of the abstraction. Unfortunately this is still a long way from lastools which takes 1.37 to load the same las file of 17 million points.

[c42f]

Righto, I've rebased and merged this, since I don't think it's anything we've done in the displaz code itself which is causing the slowdown, and it's definitely good to have support for a more recent pdal version.