Get voronoi build working on OS X

[Aeon]

With boost installed through macports, I had to add the library and include paths explicitly to the build options.

I also had to explicitly include the header that defines std::runtime_error.

Everything compiles fine now, though there are a few warnings.

[thegrandpoobah]

First off, thanks so much for your Mac OS X build support. Before I merge this though, I have two comments:

1) The #ifdef __APPLE__ 's are not necessary around #include <stdexcept>. They (should) do no harm in VC++ and are probably required on Linux as well. So if you could take those out it would be great.

2) Not sure what the effects of hard-coding the paths for Mac OS are on the Linux build. Wouldn't want to break that build. Hopefully @applio (The person who added Linux support) can comment on this.

3) http://msdn.microsoft.com/en-us/library/b0084kay%28v=vs.100%29.aspx claims that WIN32 is defined for both 32 and 64 bit targets, but I will double check this tonight.

Once again, thanks so much (and looking forward to seeing what your TSP branch does :) )

[Aeon]

Great :)

1) I wasn't sure about the #include <stdexcept> so just wanted to be on the safe side. I'll take them out. 2) I am not sure how to set the paths in Makefile depending on the system. I know absolute paths is a bad idea, but I don't know what the normal way is. I tried doing something like

ifeq ($(TARGET_OS),macos)
    CXXFLAGS += -I/opt/local/include -I/opt/local/include/boost
    LNKFLAGS += -L/opt/local/lib/
endif

but that didn't work.

[Aeon]

Argh, now I realize that after upgrading to latest xcode it no longer compiles. I'll take another look tonight and see if I can track it down.

[Aeon]

Ok, looks like clang doesn't support OpenMP, we have to explicitly specify gcc as the compiler to have it work with latest apple dev tools installed. I also made the flags conditional on platform.

By the way, are any of these compilation warnings important?

g++ -Wall -fpermissive -fopenmp -O2 -I/opt/local/include -I/opt/local/include/boost -I./picopng -I./stippler -I./voronoi -c stippler/stippler_api.cpp -o stippler/stippler_api.o
g++ -Wall -fpermissive -fopenmp -O2 -I/opt/local/include -I/opt/local/include/boost -I./picopng -I./stippler -I./voronoi -c stippler/stippler.cpp -o stippler/stippler.o
In file included from stippler/stippler.cpp:31:
stippler/VoronoiDiagramGenerator.h:185: warning: extra qualification ‘VoronoiDiagramGenerator::’ on member ‘ELgethash’
stippler/VoronoiDiagramGenerator.h:207: warning: extra qualification ‘VoronoiDiagramGenerator::’ on member ‘openpl’
stippler/VoronoiDiagramGenerator.h:208: warning: extra qualification ‘VoronoiDiagramGenerator::’ on member ‘line’
stippler/VoronoiDiagramGenerator.h:209: warning: extra qualification ‘VoronoiDiagramGenerator::’ on member ‘circle’
stippler/VoronoiDiagramGenerator.h:210: warning: extra qualification ‘VoronoiDiagramGenerator::’ on member ‘range’
stippler/stippler_impl.h: In constructor ‘Stippler::Stippler(const StipplingParameters&)’:
stippler/stippler_impl.h:85: warning: ‘Stippler::parameters’ will be initialized after
stippler/stippler_impl.h:81: warning:   ‘float Stippler::displacement’
stippler/stippler.cpp:33: warning:   when initialized here
stippler/stippler_impl.h:81: warning: ‘Stippler::displacement’ will be initialized after
stippler/stippler_impl.h:79: warning:   ‘float* Stippler::vertsX’
stippler/stippler.cpp:33: warning:   when initialized here
g++ -Wall -fpermissive -fopenmp -O2 -I/opt/local/include -I/opt/local/include/boost -I./picopng -I./stippler -I./voronoi -c stippler/utility.cpp -o stippler/utility.o
g++ -Wall -fpermissive -fopenmp -O2 -I/opt/local/include -I/opt/local/include/boost -I./picopng -I./stippler -I./voronoi -c stippler/VoronoiDiagramGenerator.cpp -o stippler/VoronoiDiagramGenerator.o
In file included from stippler/VoronoiDiagramGenerator.cpp:30:
stippler/VoronoiDiagramGenerator.h:185: warning: extra qualification ‘VoronoiDiagramGenerator::’ on member ‘ELgethash’
stippler/VoronoiDiagramGenerator.h:207: warning: extra qualification ‘VoronoiDiagramGenerator::’ on member ‘openpl’
stippler/VoronoiDiagramGenerator.h:208: warning: extra qualification ‘VoronoiDiagramGenerator::’ on member ‘line’
stippler/VoronoiDiagramGenerator.h:209: warning: extra qualification ‘VoronoiDiagramGenerator::’ on member ‘circle’
stippler/VoronoiDiagramGenerator.h:210: warning: extra qualification ‘VoronoiDiagramGenerator::’ on member ‘range’
stippler/VoronoiDiagramGenerator.cpp: In member function ‘void VoronoiDiagramGenerator::clip_line(VoronoiDiagramGenerator::Edge*)’:
stippler/VoronoiDiagramGenerator.cpp:752: warning: unused variable ‘temp’
g++ -Wall -fpermissive -fopenmp -O2 -I/opt/local/include -I/opt/local/include/boost -I./picopng -I./stippler -I./voronoi -c voronoi/parse_arguments.cpp -o voronoi/parse_arguments.o
g++ -Wall -fpermissive -fopenmp -O2 -I/opt/local/include -I/opt/local/include/boost -I./picopng -I./stippler -I./voronoi -c voronoi/voronoi.cpp -o voronoi/voronoi.o
g++ -fopenmp -O2 -L/opt/local/lib/ -o voronoi_stippler picopng/picopng.o stippler/bitmap.o stippler/stippler_api.o stippler/stippler.o stippler/utility.o stippler/VoronoiDiagramGenerator.o voronoi/parse_arguments.o voronoi/voronoi.o -lboost_program_options

[thegrandpoobah]

Looks great. I'll merge this later tonight after I compile under VC++ and make sure nothing breaks (I highly doubt it will, but you never know right?). A longer term solution may be to use CMake (I've filed #22 for that) which is meant for this purpose apparently, but that is for another time.

As for the warnings, they all look harmless to me. They also look fairly easy to fix. I'll put Visual C++ into pedantic mode tonight and see if I can clean up the warnings a bit.

[Aeon]

huzzah! thanks for the awesome library.

[applio]

@Aeon : The current code from your repo (commit 5cbd457256a0d074361dd5c4a496101f94cf9ed2) does not build on Ubuntu 11.10 (g++ 4.6.1) due to a missing #include in stippler/stippler.h (which is also an issue in @thegrandpoobah 's latest.

@thegrandpoobah : The current code from your repo (commit 65d0aa6b2a1195b810cdd2adf0fbf0349e25aeeb) does not build on Ubuntu 11.10 (g++ 4.6.1) due to (1) missing #include in 2 places (picopng/picopng.cpp, stippler/stippler.h) which @Aeon has already fixed in his, and (2) missing #include in stippler/stippler.h.

@Aeon : Your references to /opt/local are more a necessity than a bad thing, given the hybrid environment that you are working with. Because you are using Apple's supplied compiler (clang-based), it does not not automagically look in the /opt/local directories for additional include files or libraries. While you could set up other configurations for the compiler to solve this, including doing things like outright switching to use flavors of the gnu compiler installed via macports that are already aware of /opt/local, that might result in a confusing mess for others. Not everyone uses macports either -- not everyone who uses homebrew installs into the same place -- there are enough variants of hybrid environments to be more than a little annoying. Arguably the cleanest thing you could do is to either (1) offer a build solution that does not depend upon macports (or /opt/local) and instead advocates/requires that a person download and install boost (and its dependencies) in a local directory for use by the voronoi project, or (2) offer a build solution that depends entirely on macports (including one of the gnu 4.2/4.4 compilers they currently offer) and potentially clean up the Makefile correspondingly. For my take on it, I've done all three at different times (the third being the way you've got it now) for different reasons. (Note: I believe in counting laziness as a valid reason sometimes.)

I see no real issue with leaving the Makefile the way you have it now to support osx.

[applio]

Doh! My post got partially stripped: the missing include in both repos is "#include ".

Also, @thegrandpoobah , I wonder if introducing CMake (which you mused about) wouldn't be overkill here. Then again, if you're interested in learning about CMake...

[thegrandpoobah]

Oh for sure! CMake is definitely overkill for the size of the project. Like you said, If I was to implement it, I would only do it as a learning excerise on a lazy Sunday.

I highly doubt any more platforms would be added, so I totally agree that the Makefile solution contributed by you and @Aeon is good right now. If more somehow do come up, then the need for CMake does become more pressing.

As for cstring.. I have to start being careful to not break the other platforms now that they exist (this will be a big change for me :S). Linux should be easy enough once I get around to making a usable VM, but Mac OS will lag behind since I don't have the necessary hardware on hand.

[thegrandpoobah]

Score. Thank you kindly @Aeon and @applio. I'll try not to break it this time :P

[applio]

Please do continue breaking stuff -- it makes me take a look just like contributions from good folks like @Aeon do.

Thanks again for having created such a nice tool.

[Aeon]

hear, hear.