source-sdk-2013 icon indicating copy to clipboard operation
source-sdk-2013 copied to clipboard

Linux port of VBSP, VVIS, and VRAD

Open sortie opened this issue 12 years ago • 25 comments

Hi, I've almost ported the vbsp, vvis and vrad tools to Linux (well, POSIX, except there is a Linuxism or two in here) and I'd like to upstream this effort so others can benefit from it as well. My ported tools are able to compile maps that can be correctly run by the Linux 2013 SDK base.

There are a few issues, however, that prevent the tools from working fully, which my port works around by disabling some features:

  • The fgdlib source code is missing from the SDK, which means that vbsp cannot correctly handle func_instance entities.
  • The raytrace source code is missing from the SDK, which means vrad cannot calculate bounced light and only a crude first-pass is calculated.
  • The VMPI code has an external dependency on MySQL and other Windows-isms, but the tools work without VMPI.

Additionally, I encountered a couple bugs in the SDK and I fixed a number of non-portable code constructs and a large number of warnings. The utility source code is repeated twice in the repository, so I had to make each change twice (with a script) - the original files were identical, so you only need to look at one of the sp/ or mp/ versions.

My change set is 51 commits, below is an overview of what my changeset contains (please see the commits themselves for additional rationale). Note how some of the patches are optional as they are just odd fixes I decided would be appropriate to upstream along with this request, but the rest of the patches generally assume the above patches have been applied.

First up I found a bug in vbsp, it doesn't correctly handle the error case where a map have no skybox variable during cubemap generation.

6d419a245f8eb266c1a3521ad0540a7b6e3ae6f6 Fix vbsp missing skybox error case when building cubemaps.

Second, these commits fix gcc build of a number of files that contain non-portable code.

53ac2580ac9a515f8cb4ee6a1213ef253e8941f4 Fix SortEntry operator< not being const. 324b4f93dcdda1d4e50497d8ec69a6372a6817bd Fix bad class declaration in utils/vbsp/ivp. c5fb3851bb7650e3d83857f89555134acc36283f Fix use of temporary vector in utils/vrad/vraddetailprops.cpp. b64d2e878c93977ae5c155ec574c00c20cf821f9 Avoid conflict with gamma(3) in utils/vrad/vrad.h. 63325a7174ebae9766c1ede2d961208b07554641 Use Q_strncpy in utils/vbsp/detailobjects.cpp. 82fe37f0e65202c46a4bfb786ee7b679885f4df5 Fix bsp lump names being declared non-portably.

Third, these patches fixes a number of troublesome non-portable strict aliasing problems (kinda optional).

a1f272c5a79c55b228541e942e142030217912dc Fix tier0/basetypes.h strict aliasing problems. d669859ce843e98033cfca4c4189ecc90835fb72 Fix mathlib/mathlib.h and mathlib/ssemath.h strict aliasing problems. 1d451c960df8483bb275608399ef256862c5b516 Fix utils/common/bsplib.cpp strict aliasing problems. 42f8eefee5cab212b732d4bda5ed97a5fb8d6285 Fix public/mathlib/compressed_vector.h strict aliasing problems. 085137f98dc6dd44f34e413e7a310e755446c024 Fix strict aliasing problems in public/raytrace.h.

Fourth, here are patches that fix a few odd C++ problems that are revealed with -Wextra in gcc (kinda optional).

e86742e31b6af8e8c0aa0a036e279a30b37ab6d6 Initialize base classes in public/mathlib/vector.h copy constructors. b85e85ed63327a025ed81e2dae98281de0b7fadb Fix public/ member initialization reordering warnings. cfe54a4a91af61780037b9f4b33a396ce2553112 Fix utils/vrad/vismat.cpp member initialization reordering warnings. ca13ea90631d9f4cda6d7ad7cd87003c45b3e1ae Fix utils/vrad/vraddetailprops.cpp member initialization reordering warnings.

Fifth, it's best not to feed gcc visual studio macros as the results are undefined (gcc once started nethack in this case) (optional):

85b78a7f3f40a94f61bbc2718351317a893e68e6 Conditionalize Visual Studio specific pragmas.

Sixth, the source engine codebase heavily uses the deprecated char* foo = "bar"; construct that results in poor type safety and makes it hard to reason about whether a function modifies a string. I made a number of fixes to this problem (optional):

d3b4f153aeb4cadfd746250d0c20f5ecb6ea4f52 Fix writable constant strings in utils/common/bsplib. 5347b342e0fc026c55fdf64aa5b785403e7487f8 Fix writable constant strings in utils/vbsp/manifest. bf44cff66e4da00b70e31e472a37fd915bc1139f Fix writable string constants in utils/vbsp/map.cpp. f275077870b1c82916315916134b443e505c1393 Fix writable string constants in utils/vrad/lightmap.cpp. 41e41e954f00245ed93aa91f2a2341c91ac4528b Fix writable string constants in utils/vrad/vrad.cpp. c7ad8456d2fb4d1216c743fe40ac774fe64a279d Fix writable string constants in utils/vrad/vraddll.cpp. 60e121cd696a10c6f8a363ad12e762f2272dfed6 Remove unused expression in utils/common/scriplib.cpp.

Seventh, I fixed a number of comparisons between signed and unsigned values (optional):

09944869b00fd0836b99b945e37e966b77ee7144 Fix signed comparison in public/disp_powerinfo.cpp. 656fb5b042109a1d35fbccb37d6e1d85bb93c632 Fix signed comparisons in utils/common. 4a70ef007de9776ae345237476df0765a19abcd0 Fix signed comparisons in utils/vbsp. 349f98d1d61958f1ec32eb7396f8a38a624a9948 Fix signed comparisons in utils/vvis. b7f822baaf07363cab60a5e15913c70b31ce37d6 Fix signed comparisons in utils/vrad.

Eight, here are patches that resolve a few odd warnings (optional):

48ded7ad254114ff8e1d44063457760af59f95db Fix utils/vbsp/writebsp.cpp zero length format warning. 2ca3317de259ce441df83241d2c8f4b543b2a3ad Fix utils/vbsp parentheses warnings.

Ninth, here are patches that fix code that don't correctly handle the case-sensitive semantics or use backslashes without detecting the proper path separator:

6fdd85ed8f53b303f495a362e8daad1239a8f3ec Use correct path seperators when looking for config/SteamAppData.vdf. d2bcac21041884267bd4ef5ceec6ab1c3218f814 Fix case-sensitive path compatibility in utils/common/filesystem_tools.cpp. 08bfb013fa32e5cd33156c67689b5bfabf4ba44a Load the correctly cased vphysics shared library in vrad.

Tenth, a lot of code in the SDK #includes files, but the files use another case in the repository. These patches fix the #include statements so they use the correct cases when including files.

6cdae7e47bfcb66b03336d40b3f4dba42a323df3 Fix filepath case in fgblib includes. b8524df62c6389f5dd164c72fca815d4153283e3 Fix filepath case in public/loadcmdline.cpp includes. 3d0047df3df78a6969345929dd5983a474a734c0 Fix filepath case in utils/common/map_shared.h includes. bd4e7f0384b2c3d40dcd5895859221bcc2665641 Fix filename case in vbsp includes. c6deb029464b0da5023b41730a38455382daed5b Fix filepath case in vrad includes.

Eleventh, the utils/ code relies on the min and max macros, but valve_minmax_on.h header doesn't redeclare them on POSIX systems resulting in compilation errors. I suppose this is because redeclaring them causes problems somewhere else, I only managed to find such a problem in snappy.cpp. I reordered the #include statements there so the problem doesn't occur and then I fixed valve_minmax_on.h so it works correctly on POSIX systems.

22f5cab04ee5cde33f51eb00b70a454a9faf867c Reorder snappy includes to prevent minmax macro conflict. 4bba2dc246a6e6dc688d3e257e41f5d9ff0165c2 Fix min and max not being redeclared on POSIX.

Twelfth, I actually ported vbsp, vvis and vrad to to POSIX. The original code used a bunch of Windows APIs directly, but I conditionalized it to use POSIX APIs on POSIX systems. Additionally, I added some calls to pthreads - I'm not sure whether these tools should be using a generic portable threading API instead.

60a61d79bceac26c694fe790545f8993c9459214 Port utils/common to POSIX. c45695bddeb147d95bedeb87daa39d7c1139e5ac Port utils/vbsp to POSIX. 2c010d91e9ee44a393445c72e628d9059d96a2f8 Port utils/vvis to POSIX. a6b96a8200de320195fa0617849fb197c8366052 Port utils/vrad to POSIX.

Thirteenth, I encountered a number of problems with my fresh ports that I would like help with. In particular materialsystem.so asks filesystem_stdio.so for the interface SDLMgrInterface001 that it doesn't export - I modified the tools such that catches the situation by returning a fake stub implementation. Additionally, a number of libraries are absent from the SDK (in particular, fgdlib and raytrace) that I need to fully port vbsp and vrad. Please release their source code so I can port them or release static libraries along with the rest of the SDK. The vrad code is also fond of directly accessing simd values through union members, but these are not declared on POSIX systems - I added preprocessor conditionals to catch the situation. Finally, I disabled VMPI on non-windows in vvis and vrad because it had some external dependencies, but that's mostly a hack on my part that's not really meant to be upstreamed, but just included for completeness.

cd81345640e7c807df2b0b8c06d8889f4502cb39 Work-around materialsystem.so asking for SDLMgrInterface001. 8ac25641875a34c7c8c5839af7005de3afea2424 Disable fgdlib usage in utils/vbsp as its source is missing from the SDK. 1c32992892148f4f0f25df1fc6d292938980ccb7 Fix vrad direct access to simd vectors if USE_STDC_FOR_SIMD is 0. 97e3a033baf8b8e46a7fb5d2b36f6fc0b9c73e45 Only use MPI in VVIS on Windows. a49d2a4d6d325ec1d5b5112575e4baeece922a05 Only use MPI in VRAD on Windows. 5faa82e03cce0f1814784fca877eb27ed9de5c9f Only use raytrace in vrad on Windows.

I've read the CONTRIBUTING document and I hereby assert that I agree to its terms and that this is my own personal work. If you think I can improve this patch set so it's more suitable for upstreaming, please let me know. Note how I don't touch anything related to vpc: The tool didn't generate makefiles for the utilities, so I rolled my own makefiles that isn't included here.

(apologies for the SHA1 hashes not being links - there seems to be a github bug or a limit)

sortie avatar Aug 13 '13 01:08 sortie

As far as I'm aware, MySQL (at least the community edition) is cross-platform and under GPL.

MaddTheSane avatar Aug 16 '13 03:08 MaddTheSane

Indeed, it'd be trivial to link against the Linux version of MySQL. I disabled it to ease the porting, the latter patches that disable functionality are optional and only works around problems I encountered during porting. I should look into whether I can get VMPI working.

sortie avatar Aug 16 '13 11:08 sortie

https://github.com/ValveSoftware/source-sdk-2013/issues/85

ltodoto avatar Aug 16 '13 19:08 ltodoto

Why not add the makfiles you generated or port the builds to cmake or scons?

ericwomer avatar Aug 18 '13 17:08 ericwomer

There are no make files in the SDK, only VPC scripts.

saul avatar Aug 18 '13 18:08 saul

the vpc scripts generate makefiles the everything.mak and the games.mak are makefiles the problem is getting the a way to build the tools to test on linux, if nothing involving the build system is modified thats hard to do.

ericwomer avatar Aug 18 '13 18:08 ericwomer

I simply hand-wrote my own makefiles as a simpler alternative to running all the gcc invocations manually - they are not supposed to be upstreamed. Nonetheless, you can maybe make VPC generate something for you. Note how my work here is unfinished (I need fgdlib and raytrace to finish the VBSP and VRAD ports). For reference, these are the commands I used to compile the tools on Linux system: https://gist.github.com/sortie/6263132 (if you can't make a makefile from that yourself, this stuff isn't ready for you yet)

sortie avatar Aug 18 '13 18:08 sortie

You sir are a Boss.

ericwomer avatar Aug 18 '13 18:08 ericwomer

One thing I noticed is these types of errors

mp/src/public/tier0/dbg.h:626:29: fatal error: tier0/valve_off.h: No such file or directory
 #include "tier0/valve_off.h"

the file and the header are in the same folder so I don't know why #include "tier0/header.h" is nessary. I started removing the tier0 in the offending files but I don't like that fix since it will probably break somthing else. how did you get past this error?

ericwomer avatar Aug 18 '13 20:08 ericwomer

This pull request is probably not the ideal place to discuss this matter, but you need to give the correct include directories to gcc. You do not need to fix the header's includes, unless there is a problem with the case in the #include statements. See my reference command lines above for which -I relative paths I provide. It's probably more appropriate to send me a personal message on github (if it has that) than continuing the matter here. Additionally, if you do compile the tools, you need to install them correctly in the Source SDK 2013 Runtime base for them to work. This is not ready for the unskilled GNU/Linux developer to use.

sortie avatar Aug 18 '13 20:08 sortie

No communication between devs in github as far as I can tell.

ericwomer avatar Aug 18 '13 22:08 ericwomer

Whoa, nice work

yaakov-h avatar Aug 20 '13 01:08 yaakov-h

sorry for posting here but it does work, only problem I had , not including what sortie mentined above, was with vrad, it could not find the gameinfo.txt file for Half-Life 2 Deathmatch.

!!EDIT!! Just tested the map that comes with the source sdk in wine/windows and it works. just overlit since vrad did not work. sill good work though.

ericwomer avatar Aug 20 '13 01:08 ericwomer

@sortie, we appreciate this contribution. I'll hopefully be reviewing and merging this when I have time.

If you have time, I'd greatly appreciate it if you could split this into a "required to port tools" pull request, and a "fix optional build warnings" pull request. That'll make it a lot easier to review.

Regarding your seventh point: Any reason you can't use minmax.h instead of modifying valve_minmax_on.h? https://github.com/ValveSoftware/source-sdk-2013/blob/master/mp/src/public/minmax.h

jorgenpt avatar Aug 20 '13 02:08 jorgenpt

@sortie, regarding your twelfth point: Would it make sense to use CThreadMutex from https://github.com/ValveSoftware/source-sdk-2013/blob/master/mp/src/public/tier0/threadtools.h#L533 instead of pthreads / critical sections? That way you can have less #ifdefs.

jorgenpt avatar Aug 20 '13 02:08 jorgenpt

Hi @jorgenpt

I'll be sure to address your requests. As for valve_minmax_{on,off}.h - their purpose seem to be turning on and off the macros, it would then seem odd that valve_minmax_on.h doesn't do what it is supposed to on POSIX. (Unless of course, you are replacing that header.) I'll look more closely into this. I'll modify the tools to use CThreadMutex. I can try to make this OS X compatible, but I do not own such a platform, so I can't verify whether it works.

The first and second set of commits in this pull request attempt to fix real bugs and other non-portable constructs. If possible, you should merge these changes now as the other changes sort-of depends on them, which would make upstreaming the 1) warning fixes and 2) actual tool ports easier. Note that the tool ports may indirectly depend on the warning fixes. I will then proceed to create new pull requests that'll be easier to review.

For now, if you could release the source code for fgdlib and raytrace I would have all the needed information to make this a proper port that could be released with the SDK. (Well, except some VPC guy would have to make it build on POSIX.) I'll also go over the VMPI code again, I'm not totally satisfied with commenting it out.

Thanks!

@salamanderrake You should be using strace and other tools to see where the vrad executable is attempting to look for gameinfo,txt. Remember, you built the tool from source code that isn't official supported, it's your responsibility to debug it. The source SDK is known to be very sensitive to having the correct current directory with just the right contents and being provided with a proper -game option and the tools may look for files with the wrong case.

sortie avatar Aug 20 '13 10:08 sortie

@sortie it ignores the -game or -vproject command line options, which if its present upstream then I will file a bug, until that time I'll just leave it alone since I can get it to work by copying the gameinfo.txt file to the directory holding the .vmf/.bsp files.

ericwomer avatar Aug 20 '13 15:08 ericwomer

@sortie, that sounds reasonable. Would you mind splitting out the "required initial parts" (1 and 2) into a separate pull request? That way we can merge that before you're able to finish porting.

jorgenpt avatar Aug 20 '13 20:08 jorgenpt

@jorgenpt Alright, although the initial required part I would like merged is simply these commits that I put at the start of the changeset:

6d419a245f8eb266c1a3521ad0540a7b6e3ae6f6 Fix vbsp missing skybox error case when building cubemaps. 53ac2580ac9a515f8cb4ee6a1213ef253e8941f4 Fix SortEntry operator< not being const. 324b4f93dcdda1d4e50497d8ec69a6372a6817bd Fix bad class declaration in utils/vbsp/ivp. c5fb3851bb7650e3d83857f89555134acc36283f Fix use of temporary vector in utils/vrad/vraddetailprops.cpp. b64d2e878c93977ae5c155ec574c00c20cf821f9 Avoid conflict with gamma(3) in utils/vrad/vrad.h. 63325a7174ebae9766c1ede2d961208b07554641 Use Q_strncpy in utils/vbsp/detailobjects.cpp. 82fe37f0e65202c46a4bfb786ee7b679885f4df5 Fix bsp lump names being declared non-portably.

These commits can currently be fast-forwarded onto the master branch - it'd be useful to me if you would merge them (82fe37f) and then close this pull request. I'll then submit a new pull requests for the other parts. If you don't want all of the commits or wish to improve them yourself, feel free to use git cherry-pick and git commit --amend to customize it as you see fit. I'll be sure to rebase my own development branches on top of the upstream master so it can be fast-forwarded cleanly. I'm not super happy with the BSP lump name situation as it is currently, using an enum is really endian-unfriendly. The flie format should have used a char[4] array, but changing it will likely cause some incompatibilities here and there - my commit only works on little-endian processors, but it beats the non-portable construct previously used.

sortie avatar Aug 20 '13 22:08 sortie

@sortie, fgdlib and raytrace are now in the repository. Looking forward to seeing your work! I'll look into merging the first changes now.

jorgenpt avatar Sep 03 '13 20:09 jorgenpt

@jorgenpt Thanks, I just noticed the repository was updated with the fgdlib and raytrace source code. I will look into finishing the port shortly. :-)

sortie avatar Sep 03 '13 21:09 sortie

These first commits have now been merged into the main branch and you can rebase. I'm considering properly fixing line endings so that they're native line-endings and not crlf, but that would probably make your rebasing very painful. Maybe I'll wait until we've merged all of your work?

jorgenpt avatar Sep 03 '13 21:09 jorgenpt

I'd prefer to work with Unix line endings - I know git well enough to handle rebasing even in this case. I'm not sure whether the SDK codebase currently uses Windows line endings exclusively, but it'd probably be a better solution to make git automatically convert the line endings upon checkout and changing them to something neutral upon commit. This way users can use what they prefer working with without imposing problems onto others.

sortie avatar Sep 03 '13 21:09 sortie

@sortie, is there any reason to keep this pull request open?

jorgenpt avatar Dec 03 '13 04:12 jorgenpt