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

Sync with latest bgfx. #7

Merged
merged 5 commits into from
Feb 23, 2016
Merged

Sync with latest bgfx. #7

merged 5 commits into from
Feb 23, 2016

Conversation

fungos
Copy link
Contributor

@fungos fungos commented Feb 21, 2016

There is still an issue with compute shader compilation for dx11 (at least in my windows), so I temporarily removed the sample 24-nbody from compilation.

- D3DCompiler_47.dll problem when using win64;
- D3DCompiler_47.dll problem when using win32 not being able to compile
  DX11 shaders (D3DReflect error);
- Not tested on linux;
Note that sample 24-nbody does not work. There is an issue with compute
shader compilation for dx11 that need to be addressed.
@floooh
Copy link
Owner

floooh commented Feb 22, 2016

Ok, I'll do a thorough test on Windows and OSX this evening before merging. Thanks!

@floooh
Copy link
Owner

floooh commented Feb 22, 2016

I'm getting an error when running './fips gen' on OSX about an unknown target 'makedisttex', this is inside a 'if (FIPS_CLANG)', which probably explains why it's only happening on OSX, see here:

https://github.com/fungos/fips-bgfx/blob/master/CMakeLists.txt#L79

@bkaradzic
Copy link

makedisttex is deleted in bgfx so you can just nuke those...

@fungos
Copy link
Contributor Author

fungos commented Feb 23, 2016

Thanks, done!

@bkaradzic How am I supposed to compile the compute shaders? I've followed what is in the shader.mk but I get this (will install msys2 to be able to build bgfx here and see the generated makefiles soon):

 C:\fips-workspace\fips-deploy\fips-bgfx\win64-vs2013-debug\shaderc.exe -i C:\fips-workspace\fips-bgfx\bgfx\src --platform windows --type compute -p cs_5_0 -O 1 -f C:/fips-workspace/fips-bgfx/bgfx/examples/24-nbody/cs_init_instances.sc -o c:\temp\bgfx_dx11_shaderc_0n_2sm --bin2c cs_init_instances_dx11
cpp: "C:\fips-workspace\fips-bgfx\bgfx\src/bgfx_compute.sh", line 81: Fatal: Out of space in macro "__IMAGE_IMPL" arg expansion
 from file C:/fips-workspace/fips-bgfx/bgfx/examples/24-nbody/cs_init_instances.sc, line 67.
cpp: "C:\fips-workspace\fips-bgfx\bgfx\src/bgfx_compute.sh", line 81: Error: Inside #ifdef block at end of input, depth = 3
 from file C:/fips-workspace/fips-bgfx/bgfx/examples/24-nbody/cs_init_instances.sc, line 67.
Shader entry point 'void main()' is not found.
Failed to build shader.

UPDATE: Found. The problem was a typo in NBUFF being defined as NBUF and so fcpp was busting the default 512.

@floooh
Copy link
Owner

floooh commented Feb 23, 2016

Ok, quick test on Windows: when linking shaderc.exe, the d3dx9.lib cannot be found, I guess I need to install the latest D3D9 SDK for this? Is this a new dependency? I don't remember having that problem before (but it might be that I switched machines in the meantime and had the d3d9 sdk installed on the old machine).

On OSX it gets a bit further now, but there's now a general linking error against system frameworks, this is because I removed graphics and audio related frameworks as standard dependencies a while ago (this was here: floooh/fips@0dc7cdc#diff-2cc79509e7f3babf30c42ff226fc491fL18). We can fix this by adding the required dependencies to the bgfx lib definition, like I did it for the Oryol Gfx module here: https://github.com/floooh/oryol/blob/master/code/Modules/Gfx/CMakeLists.txt#L208, all executables linking against bgfx should then link automatically. It looks like the Metal framework is also needed: https://github.com/floooh/oryol/blob/master/code/Modules/Gfx/CMakeLists.txt#L172

Since you can't test on OSX, do you want me to resolve the the linker problems in my repo, and you rebase you branch against this? I could do it in the evening (about +10 hrs from now)

@bkaradzic
Copy link

shaderc is not using d3dx9 at all.

@floooh
Copy link
Owner

floooh commented Feb 23, 2016

Oh ok, then it's a bug in our CMakeLists.txt file here: https://github.com/fungos/fips-bgfx/blob/master/CMakeLists.txt#L48, removing the d3dx9 dependency does indeed work.

@floooh
Copy link
Owner

floooh commented Feb 23, 2016

Removing d3dx9 from the CMakeLists.txt locally worked, Windows looking good now, everything compiling. Most demos are running, but so far I found 2 that are crashing: 27-terrain and 26-occlusion (Windows7, GF GTX 760). Are you seeing those crashes too @fungos ?

@floooh
Copy link
Owner

floooh commented Feb 23, 2016

Ok, the crashes for demos 26 and 27 are because they need a cwd-entry in fips.yml so they run with the right working directory. Basically 2 entries missing here:

https://github.com/fungos/fips-bgfx/blob/master/fips.yml#L60

Fixes missing cwd paths for samples 26 and 27;
Fixes unecessary d3dx9 dependency;
Fixes missing OSX/iOS dependencies;
@fungos
Copy link
Contributor Author

fungos commented Feb 23, 2016

Latest fixes, I've added your findings to the fixes. But if it has something wrong you can commit your local fixes and I will merge afterwards. Thanks for all your help @floooh and @bkaradzic !

@floooh
Copy link
Owner

floooh commented Feb 23, 2016

Ok, nearly there :)

Windows is working perfectly now, all demos are running fine. On OSX, there's one more framework dependency missing to QuartzCore (see here: https://github.com/floooh/oryol/blob/master/code/Modules/Gfx/CMakeLists.txt#L170), without this, the CAMetalLayer class is missing in the linker step.

On OSX I have trouble running the samples which require the current working directory to be set, I'm getting errors like this:

2016-02-23 15:53:50.742 18-ibl[22343:2098883] /Users/floh/projects/fips-bgfx/bgfx/examples/common/bgfx_utils.cpp(74): Failed to load textures/grace_lod.dds.
2016-02-23 15:53:50.742 18-ibl[22343:2098883] /Users/floh/projects/fips-bgfx/bgfx/examples/common/bgfx_utils.cpp(180): Failed to load textures/grace_lod.dds.
2016-02-23 15:53:50.742 18-ibl[22343:2098883] /Users/floh/projects/fips-bgfx/bgfx/examples/common/bgfx_utils.cpp(74): Failed to load textures/grace_irr.dds.
2016-02-23 15:53:50.742 18-ibl[22343:2098883] /Users/floh/projects/fips-bgfx/bgfx/examples/common/bgfx_utils.cpp(180): Failed to load textures/grace_irr.dds.

I remember vaguely a current-working-directory problem on OSX from long ago, I'll try to investigate in the evening.

@bkaradzic
Copy link

You have to be inside examples/runtime in order to run.

@floooh
Copy link
Owner

floooh commented Feb 23, 2016

It works when I copy the executable to examples/runtime, but not when the executable is somewhere else, but the cwd is at examples/runtime (either manually in bash, or setting the cwd in the Xcode debugger).

...found it: https://github.com/bkaradzic/bgfx/blob/master/examples/common/entry/entry_osx.mm#L68

This sets the directory to the bundle directory (or basically, the directory where the executable resides in). GLFW does something similar. I guess that's not a bug but a feature, since in a proper OSX app bundle, the resource files would live inside the app bundle's directory.

@fungos
Copy link
Contributor Author

fungos commented Feb 23, 2016

I've saw that but wasn't sure. Fixed.
As we already have set the pwd working directory in fips.yml and this works on Windows, that means we may still have some issues with pwd on osx most probably due to bundling? Are the samples bundled on osx?

@floooh
Copy link
Owner

floooh commented Feb 23, 2016

I think we shouldn't try to fix the current directory behaviour since it's inside bgfx (unless @bkaradzic thinks it is a bug). It's just a minor inconvenience when compiling and running the samples, and the code which changes the directory isn't part of core-bgfx but in the examples app wrapper. I can live with that, may be we should add a note to README.md (currently this has example instructions for Xcode and OSX, so I guess this worked at some time).

@fungos
Copy link
Contributor Author

fungos commented Feb 23, 2016

Good catch, we had the exact same issue with turbobadger too.
This one is annoying and almost every time we are hit by it. :(
I have no idea on how to solve this without indicating how to optionally build a proper bundle in fips.yml?

UPDATE:
@bkaradzic or adding an ifdef to indicate that we want this behavior or not?
@floooh Yes, but it is very nice to have it simply working everywhere for anyone just curious about fips and/or bgfx.

@floooh
Copy link
Owner

floooh commented Feb 23, 2016

Yeah, I wouldn't want to fix that in fips. It's a fairly specific case IMHO, and fips isn't supposed to be a complete asset pipeline ;) For such specific things I usually write an additional fips command as python script and drop it into the fips-verbs directory of the project.

@fungos
Copy link
Contributor Author

fungos commented Feb 23, 2016

Right, I will blindly try a generic verb here, this may be a base to something.

@floooh
Copy link
Owner

floooh commented Feb 23, 2016

PS: one 'clean' way to fix it would be if bgfx had a define where the chdir() wouldn't be called, then we could set this define up in the CMakeLists.txt, but this needs a fix in bgfx.

@floooh
Copy link
Owner

floooh commented Feb 23, 2016

PPS: I wrote a little 'testrunner' verb recently which you could use as base, this compiles and runs all executable targets in Oryol, I use that to quickly check if everything is still running and looks right: https://github.com/floooh/oryol/blob/master/fips-verbs/testrunner.py

@floooh
Copy link
Owner

floooh commented Feb 23, 2016

...other than the 'cwd problem' everything looks good now on OSX. Just let me know when I should hit the merge button ;)

@fungos
Copy link
Contributor Author

fungos commented Feb 23, 2016

Cool, thanks! You can merge it. Btw, soon I will bring more changes to shader compilation to support binary files.

floooh added a commit that referenced this pull request Feb 23, 2016
Sync with latest bgfx.
@floooh floooh merged commit 97d9e2d into floooh:master Feb 23, 2016
@floooh
Copy link
Owner

floooh commented Feb 23, 2016

Ok merged, thanks a lot :)

@floooh
Copy link
Owner

floooh commented Feb 24, 2016

For reference, about that chdir() problem: https://gist.github.com/floooh/3b8a475d840b07d11402#file-gistfile1-md, I think the right solution for fips-bgfx is to build the samples as app-bundles, just like the bgfx build system does. But I think that chdir() in the OSX sample entry code should be cleaned up somehow, since it doesn't work as intended.

I'll try the 'build samples as app bundles' in a fips-bgfx branch.

@floooh
Copy link
Owner

floooh commented Feb 24, 2016

I've created a branch 'windowed-samples' where I converted all samples to windowed. This works fine on OSX, but I haven't tested on other platforms yet (I wonder why we created the samples as cmdline exes in the first place): https://github.com/floooh/fips-bgfx/tree/windowed-samples

@fungos
Copy link
Contributor Author

fungos commented Feb 25, 2016

Yeah, I remember that. It was something to do with /SUBSYSTEM:windows missing the entrypoint.
Now looking at bgfx generated projects I see that the /ENTRY is set to mainCRTStartup, I will do the same here to fix that.

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.

3 participants