Make it possible to compile this on Linux

[ngld]

The changes to cmake/util.cmake aren't mine, I simply copied the file from the cmake branch. I added GTK as a dependency for the CEF headers and made some small changes to make gcc happy.

[ngld]

I'm making progress but I have a question about your code/chromium/Browser.cpp file. In Browser::SystemEvent() you access event.syswm.msg->msg.win directly (which only works on windows) to get the pressed key. Why are you processing system events and not keyboard events? You could use SDL's functions for keyboard events and avoid writing windows-only code...

[asarium]

CEF expects to receive operating system event codes which are not the same as the keyboard events received by the standard SDL keypress handlers.

[ngld]

So... it compiles and I can run a normal FS2 release and debug build. However, once I try to use CEF it crashes during the startup with

[0709/005542:WARNING:resource_bundle.cc(280)] locale_file_path.empty()
[0709/005542:FATAL:main_delegate.cc(499)] Check failed: !loaded_locale.empty(). Locale could not be found for en-US

I suppose it can't find the pak files... BTW I'm getting 331 "Why are you trying to free a NULL pointer?" warnings in my fs2_open.log. 36 from fsmemory.cpp(19) and 295 from lua.ccp(15334).

Any idea what's happening there?

[ngld]

I haven't tested this on Windows so my changes might break Windows comaptibility but I'll fix those issues once it's working on Linux.

[ngld]

It compiles and works pretty great for me. I've gone ahead and changed the browser implementation a bit:

• Browser instances now contain the x and y coordinates in addition to the width and height. This is necessary to map the mouse events to the correct x and y coordinates inside the web page. (This logic is implemented in Browser::MapMousePosition.)
• The position and size of a browser instance are saved in a CefRect for convinience.
• Since both position and size are available to the browser, I've implemented a render() method.
• Browser::MouseEvent now checks if the mouse is over the browser instead of checking if the browser has focus.
• LUA tables start at 1. I corrected lua::util::toLuaValue() accordingly.
• All files specific to chromium are now placed in the chromium subdirectory. I thought this is cleaner... What's your opinion on it?

cotire isn't working for me:

error: definition of macro 'CHROMIUM_PROCESS' differs between the precompiled header ('"$<TARGET_FILE_NAME:chromiumprocess>"') and the command line ('"chromiumprocess_3_7_1_AVX-DEBUG"')

So... what do you think?

[asarium]

The changes look very nice, moving the chromium process to a sub directory might not be ideal as AFAIK CMake expects files to generated in the root directories of the targets (or whatever is considered an output directory for that target) so there might be compilations with that. When this gets merged into the main repository the deployment process of FSO will need to be changed anyway so I don't see any particular reason to have it in it's own directory.

Other than that this looks really promising, thank you for your hard work.

[ngld]

Placing all chromium related files in a subfolder means that I don't have to place the chrome-sandbox, cef.pak, chromiumproces, etc. files and the locales folder in my FSO root folder.

I don't like having too many files in my FSO root folder but that's probably a matter of taste. I'll apply your suggested changes and compile this on Windows to check that I didn't break anything...

[asarium]

I am aware of this issue and even without all the .pak files there are still the required libraries that also need to be placed in the FSO root folder. The only solution is to not have the FSO executables in the FreeSpace folder at all but instead store them at some other location and only set the working directory to the correct folder.

[ngld]

I'm not sure if it works on Windows but during my current tests chromiumprocess and libcef.so are in the chromium subfolder. I didn't modify LD_LIBRARY_PATH but used the -Wl,-rpath option which modifies the runtime library search path.

It seems that adding the chromium subfolder to the PATH would work on Windows. I'll see if that works...

[asarium]

That solution only works if an external application (e.g. the launcher) sets the PATH environment variable correctly.

[ngld]

Could you move find_package(Lua51 REQUIRED) inside the else(DEFINED LUA_TARGET) ... endif(...) block in lib/LuaCppUtil/src/CMakeLists.txt ? Otherwise it breaks FSO_BUILD_INCLUDED_LIBS and FSO_USE_LUAJIT if Lua isn't found.

With this change it should build on Windows. The only drawback right now is that you have to set CMAKE_BUILD_TYPE or at least CEF_BUILD_TYPE because cmake/cef.cmake needs to know which CEF version you want to use. This isn't a problem on Linux since CMAKE_BUILD_TYPE is always set there but on Windows you can change it in VS which in turn breaks the linking to CEF if it doesn't match the CEF_BUILD_TYPE...

[asarium]

I have changed LuaCppUtil accordingly.

You shouldn't use CMAKE_BUILD_TYPE at all, instead use generator expressions whenever possible (I don't know if you can do that here though).

On windows you currently only need to link to to the cef_cpp_wrapper target which is an imported target for the pre-built CEF libraries. I didn't include those in the repository because they are quite big, I will have to figure out what to do with them later, for reference here is the CMakeLists.txt file which is used to add that target: https://gist.github.com/asarium/c01e1047df9612ab956f

[ngld]

Thanks!

The cef_cpp_wrapper (isn't it cef_dll_wrapper?) is actually the reason I need the CMAKE_BUILD_TYPE. Right now you can download a build of your choice from http://cefbuilds.com, extract it somewhere on your hard drive and set the CEF_PATH to the cef_binary_... folder. Then you have to build the libcef_dll_wrapper.vc(x)proj by hand (I couldn't figure out how to properly do this using CMake).

The CMake files will take care of linking against the correct libraries and copy the dll and pak files for you. I'm using the CMAKE_BUILD_TYPE to determine if I should link against the Debug or Release build of the CEF library.

I just read you CMakeListst.txt file and I might be able to remove the CMAKE_BUILD_TYPE requirement for windows. The way you did it looks cleaner, too.

[asarium]

I am using pre-built versions of the dll wrapper but I just found out that there is the include_external_msproject command in CMake which properly includes the project in the solution but the compiler doesn't link against it. If I get that to work it will make compiling much easier.

[asarium]

I am currently looking at validating the URL passed to Browser::Create so the user will get a nice warning if the URL isn't valid. After that I will merge these changes, I think that the include_external_msproject solution may work very well and that would be easier to maintain.

[ngld]

I'm currently fixing some issues on Windows and will commit as soon as it works.

[asarium]

I just pushed a commit that adds URL validation.

[ngld]

Thanks! Rebased against it and finished the CEF helper for CMake.

Now you just have to set the CEF_PATH and it will build the cef_dll_wrapper automatically and copy all needed files in the same directory as fs2_open.exe. Tested on Windows and Linux.

[asarium]

Excellent work, I am currently giving it a test run but it looks like everything is alright (there are a few minor issues I will resolve later). Thanks you very much for your effort!

[asarium]

Of course! That explains why no files got copied when I tried to compile.

[asarium]

I changed some things so it compiles and works again. I also added install rules to include the CEF files. Can you check if it works on linux?

[ngld]

It works but the CEF libraries are copied for every target (3 times). Preventing that was basically the point of the cef_files target. Not really important but the indentation is messed up (on GitHub) because you're mixing spaces and tabs.

[asarium]

I fixed the tabs vs. spaces issue. The problem of the cef_files target was that it used the embedfile target to determine the target location. I used copy_if_different to make sure that there wouldn't be any unnecessary copy operations but I think the message is still displayed.

[ngld]

Alright, thanks!