search

r-lidar-lab / lidRviewer

An OpenGL point cloud viewer for R able to handle several millions of points
75 stars 16 forks source link

Update to SDL2 #16

Closed H4estu closed 4 months ago

H4estu commented 1 year ago

Overview

This updates the relevant C++ code in lidRviewer to use the updated Simple DirectMedia Layer 2.0 (SDL2) library. The old version, SDL1.2, is no longer supported on many OS's, so this should allow more people to use this package if they like.

This works on my machine with the following specs:

R

C++ Compiler

Machine

Notes

Jean-Romain commented 1 year ago

Can you explain the change in src/Makevar ?

H4estu commented 1 year ago

If I recall correctly, I needed

PKG_CXXFLAGS = -I/usr/local/Cellar/sdl2/2.0.16/include/ -I/opt/X11/include

to point the gcc to the new SDL2 header files, as well as to the X11 headers for rendering.

I honestly don't fully recall why I needed:


PKG_LIBS= -framework OpenGL -framework GLUT -lSDL2

but I can dig back in after work (I'm on Pacific Standard Time). At the very least, I needed to link the new SDL2 library, but I don't recall off the top of my head why I needed to link OpenGL and GLUT as well.

caiohamamura commented 4 months ago

I can confirm that this is much better for Windows systems as well, because rtools* will already have installed the SDL2 library, so you only need to link to them.

We can do that by modifying Makevars.win to:

PKG_LIBS = $(shell pkg-config --libs sdl2 glut glu)
PKG_CPPFLAGS = $(shell pkg-config --cflags sdl2 glut glu)

I would actually use Makevars.ucrt instead of Makevars.win, since I don't know for sure it will work with versions before UCRT was introduced.

Jean-Romain commented 4 months ago

Rtools have SDL2 really? That changes everything...

caiohamamura commented 4 months ago

I would rather rely on pkg-config for linking on unix systems as well as it will deal with packages not installed in the default paths and different sets of dependencies regarding different systems having GLUT embedded or not.

In MSYS2 windows it is a little broken though, so you actually need to manually add glut glu, but I think that won't be an issue under unix systems. Otherwise you can try using sdl2-config. If you really want that to be robust and test multiple configurations and linking I could pull request a configure.ac + configure scripts for testing appropriate configurations.

Jean-Romain commented 4 months ago

Sure if you can PR something that works on all platforms I'll be happy. The development of the package is abandoned mainly because I don't know how to make it working on all platforms. If you are able to PR something workable at least on Unix/Windows then I could start new projects on this package.

I never merged this PR because of the hard code path PKG_CXXFLAGS = -I/usr/local/Cellar/sdl2/2.0.16/include/ -I/opt/X11/include

H4estu commented 4 months ago

I never merged this PR because of the hard code path PKG_CXXFLAGS = -I/usr/local/Cellar/sdl2/2.0.16/include/ -I/opt/X11/include

Yeah, sorry for the hardcoded path there. I haven't thought about this PR in a long time, so feel free to close it. Sounds like @caiohamamura has a better path forward for multi-platform support anyway.

Jean-Romain commented 4 months ago

Otherwise I should rewrite the package without SDL using GLU which is natively available for R. This is why I abandoned the development.

Jean-Romain commented 4 months ago

Ok I checked in Rtools 43 and indeed there are some SDL2 folders. The game is changing. @caiohamamura some help may be require. If have 0 expertise on Windows but we could have this package working again.