floooh / fips-bgfx

fipsified version of bgfx (https://github.com/bkaradzic/bgfx)
17 stars 8 forks source link

Add tools build #4

Closed fungos closed 9 years ago

fungos commented 9 years ago

do not merge this

This will build:

And some more 3rdparty dependencies:

Only tested on linux for now, will update to fix anything related to windows soon. This is a starting point to get bgfx generators working. Can fips build tools before using them in dependency?

If not, as a test, I've started doing this as another project. Something like fips-bgfx-utils to import inside fips-bgfx. But this would add another submodule bgfx cloning (or is there another way?). This new project then build fips-3rdparty, fcpp, glsl-optimizer and all tools, so when building fips-bgfx we would have the deps already built so the generator may search the tools executables in fips-deploy(?) and use then for building shaders during bgfx and samples build step.

floooh commented 9 years ago

There's currently no tools building pass in fips, but some discussion in this ticket: https://github.com/floooh/fips/issues/48, I think the main point is that such tools should definitely not go into the main 'project' (e.g. they should not show up in the default project in Visual Studio), and I don't currently like the idea of having projects with multiple separate subprojects (cmake allows this, but it would make fips more complicated). But may be we'll have to bite the bullet and have separate projects (which could be selected with for instance './fips set project xxx' similar to './fips set target xxx'). To build tools, one would then do a './fips set project tools', followed by a './fips build'....

For oryol I'm planning to move tools into a separate oryol-tools project, plus all helper scripts (and fips verbs) to implement a basic asset build pipeline.

fungos commented 9 years ago

I agree. I do not think having multiple sub-projects inside one fips project is a good thing. But I do prefer having real and separated multiple (and tiny, fipsified) projects, again in the rust/cargo style.

The only problem here with bgfx for me is that, to get the tools in another project the user will do clone bgfx two (or more) times. Git sparse checkout still do a full repo clone and does not solve this the way svn would do. :P

Another way to avoid multiple clones but more hacky, would be to have a repository fips-bgfx-base that clones the bgfx and bx, and then fips-bgfx-tools and fips-bgfx wouldn't clone anything but would access the files in the fips-bgfx-base by refering files as "../fips-bgfx-base/bgfx/src/*.cpp" for example.

So the resulting workspace would be:

/fips /fips-bgfx-base /fips-bgfx-tools (which imports fips-bgfx-base) /fips-bgfx (which imports fips-bgfx-base and fips-bgfx-tools)

This way building bgfx will first build base and tools. Then when building bgfx, the tools are ready to be run by the generators.

bkaradzic commented 9 years ago

Reason why I keep those separate is to not build for platforms where those command line tools don't make sense.

fungos commented 9 years ago

Yeah, that is fine. :)

Btw, separating the project in two does not help as it seems that cmake/fips do not care with the building order for apps from imported projects. Is is building imported libs before, some tools too, but for example, shaderc is being built last.

Maybe this may be improved in fips with some way to add dependency on imported project executables.

fungos commented 9 years ago

This commit + the fips PR solves the issue and we can build everything on bgfx in only one step! Still haven't tried to use this on my bgfx extern sample project to see if the hackish generator code is working correctly.

To fix the python script it will require that fips pass some info to generators. As in this case the correct fips-deploy path for the project (to find the tools previously built). Or better, some util to run a fips target by name (and using the cwd settings as workdir for example).

Again, I've only tested on linux. Will try to test on windows tomorrow if possible.

fungos commented 9 years ago

I've got the tools building on windows, but now there is a problem trying to run the generator, an exception 'config' is thrown from imp.load_module (.fips-gen.py). Any idea why?

floooh commented 9 years ago

I can't check on Windows over the long Easter weekend, but it sounds like it is trying to load a python module during code generation but can't find it because it isn't in the python search path. The gen_path variable in there is this search path. For instance, if you want to load one of fips' internal modules, you need to use 'from mod import config' because the fips directory is in the search path, but not 'fips/mod'

fungos commented 9 years ago

Now it is working on windows. Ready to go if everything is fine on MacOS. I've build everything but the samples 13, 14 and 16 on emscripten (problem with CtrFileReader - BX_CONFIG_CRT_FILE_READER_WRITER), but no sample works when running (exception thrown: TypeError: Cannot read property 'getStreamFromPtr' of undefined,TypeError: Cannot read property 'getStreamFromPtr' of undefined). Will try PNaCL and Android soon.

floooh commented 9 years ago

Ok cool, I'll try to merge into a branch first and do tests for OSX and cross-compiling, might take a few days, I'm quite busy with work stuff and the turbobadger port :)

Update: aehrm, this comment should actually go to https://github.com/floooh/fips/pull/81 ...

fungos commented 9 years ago

Emscripten: the problem is that there is some GL extension that BGFX tries to load but is not supported, so when trying to print the name of the invalid extension, this fprintf https://github.com/kripken/emscripten/blob/incoming/system/lib/gl.c#L1731 crashes because FS is set to undefined.

pnacl: Some toolchain problem, as some symbols aren't declared (like, strstr, strcasecmp, setenv, etc.)

@bkaradzic is emscripten build working on HEAD? I've tried to run 17-drawstress here. fips is using emscripten 1.30.5 (branch incoming), but I do not think they've excluded any gl procs.

floooh commented 9 years ago

I remember what the problem is: fips has the emscripten FS (filesystem) wrapper disabled by default to reduce client size and I had to enable it for the bgfx drawstress sample. I made this into a cmake option, so projects which need this can turn it on via 'fips config' or through a custom config: https://github.com/floooh/fips/blob/master/cmake-toolchains/emscripten.toolchain.cmake#L35

That fprintf in emscripten_GetProcAddress looks like a bug to me though, the GL wrapper code shouldn't require the FS wrapper to be enabled, I'll check whether there's a ticket in emscripten for this already, and if not, write one.

floooh commented 9 years ago

Wrote a ticket: https://github.com/kripken/emscripten/issues/3327, workaround for bgfx would be to enable the cmake option as written above.

floooh commented 9 years ago

I'm currently getting an error on Windows on the cmake run:

  Cannot find source file:
    bgfx/tools/shaderc/shaderc_hlsl.cpp

Looks like this has recently been split into shaderc_dx9 and shaderc_dx11: λ ls shaderc.cpp shaderc.h shaderc_dx11.cpp shaderc_dx9.cpp shaderc_glsl.cpp

bkaradzic commented 9 years ago

It's not split, it's unified recently. There is only shaderc_hlsl.cpp now.

fungos commented 9 years ago

Updated submodules again, this should fix the missing shaderc_hlsl.cpp. If it doesn't, try a submodule update before building.

floooh commented 9 years ago

I'm getting an error in the top-level CMakeLists.txt file because of the GROUP param in fips_files:

file(GLOB_RECURSE BX_FILES bx/include/bx/*.h bx/include/compat/*.h bx/include/tinystl/*.h)
fips_files(${BX_FILES} GROUP "bx")

The GROUP is then interpreted as a filename.

Did you extend the fips_files() macro with a GROUP named param in your fork but didn't provide a pull-request for fips yet? (that looks actually like a nice feature).

fungos commented 9 years ago

Haha no, sorry, probably was a typo. The problem there was that using fips_dir with a multi directory file(GLOB...) is a little difficult to get right. I must have mixed fips_dir with fips_files trying to get that to work. But yeah, that would be an interesting feature. I may take a look at it soon. In the meantime I will fix this one.

floooh commented 9 years ago

Ok, works now on OSX. Commencing merge :)