Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow TIFF, LZO, and SuperLU libraries to be found when compiling for… #37

Closed
wants to merge 1 commit into from

Conversation

Szalacinski
Copy link

… Linux

I'm going to work on this some more tomorrow, but it's past midnight here in Tennessee, and I figured I might as well share the results of me digging through the code for a little bit. For full Linux support, the main cmake file is going to have to have another option for Linux, boost libraries will have to be found, and some C++ files will have to be rewritten specifically for #34

@@ -1,6 +1,6 @@
# looks for libtiff(4.0.3 modified)
find_path(TIFF_INCLUDE_DIR NAMES tiffio.h HINTS ${SDKROOT} PATH_SUFFIXES tiff-4.0.3/libtiff/)
find_library(TIFF_LIBRARY NAMES libtiff.a HINTS ${SDKROOT} PATH_SUFFIXES tiff-4.0.3/libtiff/.libs NO_DEFAULT_PATH)
find_path(TIFF_INCLUDE_DIR NAMES tiffio.h HINTS ${SDKROOT} /usr/include/i386-linux-gnu/ /usr/include/x86_64-linux-gnu/ PATH_SUFFIXES tiff-4.0.3/libtiff/)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As a concept, using the shared libraries already on the system is probably a decent idea. It might be better to use pkgconfig to retrieve the locations/flags though, as that's already a dependency + should be more cross platform compatible.

As an example on OSX:

$ pkg-config --libs libtiff-4
-L/usr/local/Cellar/libtiff/4.0.6/lib -ltiff
$ pkg-config --cflags libtiff-4
-I/usr/local/Cellar/libtiff/4.0.6/include

Those values should be usable to pass into appropriate variables.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@justinclift
Copy link

As a side note, after getting everything to compile ok on OSX, I attempted to switch from the bundled tiff 4.0.3 to the latest upstream tiff (4.0.6). That failed with a variety of errors, so it looks (for now) like the dependency on tiff 4.0.3 is not easily changed. 😉

No idea if the bundled tiff-4.0.3 has been customised. I don't have the inclination to dig into it just now. Going to tear myself away from the computer for a bit instead. 😄

@roentgen
Copy link
Member

I tried get tiff out before I know the original toonz version had used tiff-modified-4.0.3. So I give it up.
But I guess that does not prevent for linux-build while you use opentoonz/thirdparty/tiff-4.0.3.

@justinclift
Copy link

Yeah. The changes in tiff 4.0.3 will probably need to be ported to tiff 4.0.6, and (perhaps) submitted to the upstream tiff project, depending on what they are.

@Szalacinski
Copy link
Author

Pull request #51 looks a lot more thorough than this starter patch. A lot more people hopped on this project than I initially thought they would. FOSS is wonderful.
If you feel it would be better, feel free to close this pull request and merge his instead.
With how quickly the work is going along, I wouldn't be surprised if we have a working Linux build within a week.

@justinclift
Copy link

I looked at the modifications made to tiff 4.0.3, to see how much effort would be required to update them into tiff 4.0.6 (the latest available at this time):

    https://github.com/justinclift/opentoonz/commits/tiff406

The only major difference is the addition of the file libtiff/tif_getimage_64.c, plus a few tweaks to the supporting files to ensure it is included.

This new tif_getimage_64.c file is a copy of tif_getimage.c (in the same directory), with substantial changes to add 64 bit TIFF support.

Someone with an understanding of TIFF file formats + and good at C coding (not myself at the moment) would be able to update this to use the tiff 4.0.6 tif_getimage.c as it's base. That should probably be done, since tiff-4.0.3 has known CVE's for it.

I don't think I'll do much more with this. Maybe this investigation will help someone else get it done though. 😄

@Fordi
Copy link

Fordi commented Mar 27, 2016

I don't think this is the way to do it. There's a bit in toonz/sources/CMakeLists.txt, line 8, the determines the content of THIRDPARTY_LIBS_HINTS - this is the search path for external libraries.

I found that if I glom this - eliminating the branches and just setting it to "/usr/include" and nothing else, it gets past liblzo, libusb, libsuperlu, and liblz4. Adding "/usr/include" and flipping it (so it's if (WIN32) ... else() ... endif()) is probably fine, too. But adding system-specific pathnames (such as /usr/include/i386-linux-gnu/ /usr/include/x86_64-linux-gnu) is usually a big no-no.

Here's the patch:

 diff --git a/toonz/sources/CMakeLists.txt b/toonz/sources/CMakeLists.txt
 index a1749d9..c0dbbab 100644
 --- a/toonz/sources/CMakeLists.txt
 +++ b/toonz/sources/CMakeLists.txt
 @@ -4,10 +4,10 @@ project(OpenToonz)
  get_filename_component(SDKROOT ../../thirdparty/ ABSOLUTE)
  message("SDK Root:" ${SDKROOT})
  set(CMAKE_MODULE_PATH ${CMAKE_MODULE_PATH} "../cmake")
 -if(APPLE)
 -    set(THIRDPARTY_LIBS_HINTS "/usr/local/Cellar/" "/opt/include" ${SDKROOT})
 -elseif(WIN32)
 +if(WIN32)
      set(THIRDPARTY_LIBS_HINTS ${SDKROOT})
 +else()
 +    set(THIRDPARTY_LIBS_HINTS "/usr/local/Cellar/" "/opt/include" "/usr/include" ${SDKROOT})
  endif()

  message("Thirdpary Library Search path:" ${THIRDPARTY_LIBS_HINTS})

Other than that, following the instructions for OS-X have gotten me boost and libtiff - I'm sure they did that for version-specific reasons. The list of packages I've had to install so far to build on my machine (Ubuntu 15.10) is:

qt5-default
qtbase5-dev
libqt5svg-dev
qtscript5-dev
libsuperlu-dev
liblz4-dev
libusb-dev
liblzo2-dev

There may be other Qt libs I already had installed (I build a lot of things); I'll have to check what CMake is asking for.

Right now, I'm stuck on a bunch of NOTFOUND vars, including things like CARBON_LIB and COCOA_LIB, which shouldn't be necessary. I'll figure out what's populating them and adjust CMake accordingly.

[Edits: Added more dev deps]

[Edit: Had a lot to shore up in the CMakeLists for linux. Also, had to basically refactor the godawful mess that was tatomicvar.h (basically, lots of platform-specific cruft from before the c++11 era, where atomics are standardized - now TAtomicVar is just a wrapper around std::atomic). Next item is missing qdatetime in tsystem.h - probably just a missed qt dep.]

@Szalacinski
Copy link
Author

@Fordi's patch set looks a lot cleaner than mine. It would probably be better to use his. I'll keep this pull request open for the sake of conversation.

Edit: FYI, this is my first time actually using CMake rather than GNU make, so I'm kind of dabbling a bit. I view the conversation of how things should be done correctly, and how the code should be refactored as an incredibly important one.

@pwllm
Copy link

pwllm commented Mar 27, 2016

@Fordi : had to add "/usr/include/qt" to THIRDPARTY_LIBS_HINTS too (on Debian)
and install of package qttools5-dev

@Fordi
Copy link

Fordi commented Mar 27, 2016

I've gotten pretty far on the Ubuntu build, but I had to be done at 3am.
I'll push what I've got to my fork later, when I get home.

A large number of changes are to do with case sensitivity. One big change
I realized could have been done more easily is going from defined(LINUX)
to __linux__ - GCC doesn't define LINUX, but it should be easy enough to
copy the constant over (rather than replacing instances as I've done).

Presently I'm stuck on the XImplementation of some GL stuff - which I'm not
entirely convinced is necessary - the OSX version uses Qt for its
implementation without recourse to Carbon/Cocoa, so I might be able to get
away with a similar trick there (but then, it was 3am, and I needed to be
done). Anyway, I'll update with progress later tonight.
On Mar 27, 2016 3:44 PM, "pwllm" notifications@github.com wrote:

@Fordi https://github.com/Fordi : had to add "/usr/include/qt" to
THIRDPARTY_LIBS_HINTS too (on Debian)


You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
#37 (comment)

find_path(LZO_INCLUDE_DIR NAMES lzoconf.h HINTS ${THIRDPARTY_LIBS_HINTS} PATH_SUFFIXES lzo/2.09/include/lzo lzo/2.03/include/lzo)
find_library(LZO_LIBRARY NAMES liblzo2.a lzo2_64.lib HINTS ${THIRDPARTY_LIBS_HINTS} PATH_SUFFIXES lzo/2.09/lib lzo/2.03/lib/LZO_lib)
find_path(LZO_INCLUDE_DIR NAMES lzoconf.h HINTS ${THIRDPARTY_LIBS_HINTS} /usr/include/lzo/ PATH_SUFFIXES lzo/2.09/include/lzo lzo/2.03/include/lzo)
find_library(LZO_LIBRARY NAMES liblzo2.a lzo2_64.lib HINTS ${THIRDPARTY_LIBS_HINTS} /usr/lib/i386-linux-gnu/ /usr/lib/x86_64-linux-gnu/ PATH_SUFFIXES lzo/2.09/lib lzo/2.03/lib/LZO_lib)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

find_library() shouldn't use either "i386-linux-gnu" nor "x86_64-linux-gnu" directly because it won't work for the other CPU architectures such as ARM. Instead, CMAKE_LIBRARY_ARCHITECTURE variable can be used here.

@ideasman42
Copy link
Collaborator

Was just looking into linking issues - #866.

This PR is quite old, makes some suspicious changes (hard coding architectures in the path), and not even sure its needed - opentoonz now builds on latest Ubuntu and ArchLinux, so closing this PR.

If there are problems finding libraries, best report this as a bug, with details on your Linux version.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

8 participants