mosra / magnum-examples

Examples for the Magnum C++11 graphics engine
https://magnum.graphics/
The Unlicense
282 stars 95 forks source link

Examples don't pass cmake for Emscripten #29

Closed decltype-auto closed 8 years ago

decltype-auto commented 8 years ago

On Debian 8 x86_64 I've successfully built and installed corrade, magnum and magnum-plugins¹ for both the system's ELF clang-3.5 chain and for the emscripten (emcc 1.36.0) chain.

I sudo-installed corrade, magnum and magnum-plugins¹ underneath default /usr/local; "emsdk_portable" resides in my home directory.

Given that environment, the magnum viewer example builds flawlessly with the clang for ELF. and emcc and em++ compile HelloWorld programs to valid js.

On the example tree, however, after git'ing the toolchains module and patching toolchains/generic/Emscripten.cmake to point to my emsdk location, cmake (3.0.2), invoked from a build dir underneath the magnum-examples - root dir like

cmake -DCMAKE_TOOLCHAIN_FILE="../toolchains/generic/Emscripten.cmake" ..

first detects valid emcc and em++ but then fails with

CMake Error at /usr/share/cmake-3.0/Modules/FindPackageHandleStandardArgs.cmake:136 (message):
  Could NOT find Corrade (missing: _CORRADE_MODULE_DIR)
Call Stack (most recent call first):
  /usr/share/cmake-3.0/Modules/FindPackageHandleStandardArgs.cmake:343 (_FPHSA_FAILURE_MESSAGE)
  modules/FindCorrade.cmake:457 (find_package_handle_standard_args)
  modules/FindMagnum.cmake:186 (find_package)
  CMakeLists.txt:43 (find_package)

For my purposes, not all of the examples would need to be compiled to js, if it'd be just the viewer example itself - like it apparently was for mosra's showcases - I'd be happy enough.

Due to the complexity of the build infrastructure involved I cannot be totally sure that this is an issue - it is a perfectly plausible alternative hypothesis that I just messed sth up. But I've tried several approaches to alter cmake-module paths, setting missing variables on the command line or in the cmake-gui etc, but to no avail yet.

Could you look into that, please?

Yours sincerely, decltype-auto

P.S. @mosra: Your Magnum is a BRILLIANT piece of software! Likely the cleanest real-life C++11/14 I've yet seen; I'm really enthusiastic about it.

¹ For emscripten just with -DWITH_ANYAUDIOIMPORTER=ON \ -DWITH_ANYIMAGECONVERTER=ON \ -DWITH_ANYIMAGEIMPORTER=ON \ -DWITH_ANYSCENEIMPORTER=ON \ -DWITH_OPENGEXIMPORTER=ON \ due to lack of libpng, libfontwhatever in my emsdk environment.

decltype-auto commented 8 years ago

OK, as I had already expected, I had messed up the path settings for cmake.

This

cmake  -DCMAKE_TOOLCHAIN_FILE="../toolchains/generic/Emscripten.cmake"  -DCMAKE_MODULE_PATH=`pwd`/../toolchains/modules -DCMAKE_PREFIX_PATH=$HOME/emsdk_portable/emscripten/master/system  -DWITH_VIEWER_EXAMPLE=ON ..

successfully generates js make infrastructure for triangle and viewer.


triangle now also builds fine, but for viewer I get

[...]ViewerExample.cpp:201:43: error: no member named 'RGB8' in 'Magnum::TextureFormat'
            .setStorage(1, TextureFormat::RGB8, imageData->size())

which I assume has to do with the WITH_XXXflags passed to the Magnum cmake call for the emscripten library build.

Which feature must be enabled to get RGB support into the emscripten Magnum build?

mosra commented 8 years ago

Hey, thanks for the nice words! It made my day much brighter :) Really.

Some examples not building correctly on Emscripten -- to make the code simple and fun to read without overwhelming platform specifics and ugly boilerplate for various mobile platforms, the master branch compiles only on desktop and on desktop (non-ES) OpenGL. There's a ports branch that aims to port as many examples as possible to other platforms such as Emscripten, NaCl and Android, but it is currently in kinda sad state and wasn't updated for some time, because originally it required backwards compatibility with old compilers and that was pain to maintain.

I'm planning to revive it after I drop the backwards compatibility and then I try to port as many examples as possible to Emscripten and elsewhere, making use of web audio, WebGL 2 etc., but currently can't say any date as I'm quite overwhelmed with my job.

Regarding the viewer example: fortunately, just to get it to compile on WebGL 1, just change RGB8 to RGB. However, in order to actually be able to view it in a web browser like here you need some more boilerplate, like a HTML wrapper page and then some way to make the file available to the web browser. If you feel like digging up the diffs, you can see the differences here: https://github.com/mosra/magnum-examples/compare/master...ports

Sorry if this feels too much like DYIDIY -- once I get to cleaning up the ports branch, it would be just a matter of building and installing all of the stuff. But for now... :)

decltype-auto commented 8 years ago

For the ports branch cmake

cmake \
    -DCMAKE_TOOLCHAIN_FILE=$MAGNUM_EXAMPLE_ROOT/toolchains/generic/Emscripten.cmake  \
-DCMAKE_MODULE_PATH=$MAGNUM_EXAMPLE_ROOT/toolchains/modules \
-DCMAKE_PREFIX_PATH=$HOME/emsdk_portable/emscripten/master/system  \
-DWITH_VIEWER_EXAMPLE=ON \
-DCMAKE_CXX_FLAGS="-std=c++14" \
-DCMAKE_INSTALL_PREFIX=$MAGNUM_EXAMPLE_ROOT/site  \
$MAGNUM_EXAMPLE_ROOT

generates without ado, but the (make-) build fails with

Scanning dependencies of target magnum-triangle
[ 12%] Building CXX object src/triangle/CMakeFiles/magnum-triangle.dir/TriangleExample.cpp.o
[ 25%] Linking CXX executable magnum-triangle.js
[ 37%] Built target magnum-triangle
Scanning dependencies of target ViewerData-dependencies
[ 37%] Built target ViewerData-dependencies
src/viewer/CMakeFiles/magnum-viewer.dir/build.make:60: *** target pattern contains no '%'.  Stop.
CMakeFiles/Makefile2:172: recipe for target 'src/viewer/CMakeFiles/magnum-viewer.dir/all' failed
make[1]: *** [src/viewer/CMakeFiles/magnum-viewer.dir/all] Error 2
Makefile:116: recipe for target 'all' failed
make: *** [all] Error 2

due to make's double colon plague¹, the culprit is the Corrade::rc entity in
src/viewer/CMakeFiles/magnum-viewer.dir/build.make

60 src/viewer/resource_ViewerData.cpp: ../src/viewer/Corrade::rc
61 src/viewer/resource_ViewerData.cpp: src/viewer/resource_ViewerData.depends
62 src/viewer/resource_ViewerData.cpp: ../src/viewer/scene.ogex
63        @$(CMAKE_COMMAND) -E cmake_echo_color --switch=$(COLOR) --blue --bold --progress-dir=[...]/magnum-examples/build/CMakeFiles --progress-num=$(CMAKE_PROGRESS_1) "Compiling data resource file [...]/magnum-examples/build/src/viewer/resource_ViewerData.cpp"
64        cd [...]/magnum-examples/src/viewer && Corrade::rc ViewerData resources.conf [...]/magnum-examples/build/src/viewer/resource_ViewerData.cpp

Commenting out line #60 and replacing the string Corrade::rc with the result of which corrade-rc fixes this, and the viewer gets builds and and installed-

But if I publish the installed site page with e.g. python3 -m http.server 8765 --bind 127.0.0.1 run in $MAGNUM_EXAMPLE_ROOT/site and step on it in a browser, blender's Suzanne does not show up, because e.g. FF fails with²

exception thrown: ReferenceError: process is not defined,ASM_CONSTS<@http://localhost:8765/magnum-viewer.js:1473:65
_emscripten_asm_const_ii@http://localhost:8765/magnum-viewer.js:1476:9
__ZN7Corrade7Utility9Arguments8tryParseEiPPKc@http://localhost:8765/magnum-viewer.js:64289:14
__ZN7Corrade7Utility9Arguments5parseEiPPKc@http://localhost:8765/magnum-viewer.js:63880:8
dynCall_viii@http://localhost:8765/magnum-viewer.js:209961:3
invoke_viii@http://localhost:8765/magnum-viewer.js:9390:5
__ZN6Magnum7ContextC2ENS_9NoCreateTEiPPKcPFvvE@http://localhost:8765/magnum-viewer.js:47733:31
dynCall_viiiii@http://localhost:8765/magnum-viewer.js:209800:3
invoke_viiiii@http://localhost:8765/magnum-viewer.js:9183:5
__ZN6Magnum8Platform15Sdl2ApplicationC2ERKNS1_9ArgumentsEDn@http://localhost:8765/magnum-viewer.js:96237:2
__ZN6Magnum8Platform15Sdl2ApplicationC2ERKNS1_9ArgumentsERKNS1_13ConfigurationE@http://localhost:8765/magnum-viewer.js:96581:2
dynCall_viii@http://localhost:8765/magnum-viewer.js:209961:3
invoke_viii@http://localhost:8765/magnum-viewer.js:9390:5
__ZN6Magnum8Examples13ViewerExampleC2ERKNS_8Platform15Sdl2Application9ArgumentsE@http://localhost:8765/magnum-viewer.js:10024:3
_main@http://localhost:8765/magnum-viewer.js:17828:2
asm._main@http://localhost:8765/magnum-viewer.js:211938:8
callMain@http://localhost:8765/magnum-viewer.js:212184:15
doRun@http://localhost:8765/magnum-viewer.js:212242:42
run/<@http://localhost:8765/magnum-viewer.js:212253:7
"

Sorry if this feels too much like DIY

It doensn't. The build system is already very good for the native side - that it gets a little itchy on cross-compiling generic C++ for the Web (!) is quite natural given the complexity that is dealt with in this chain.

¹ Switching to cmake's Ninja or Eclipse generators didn't better things. ² Taken from FF's browser console; (Hotkey <CTRL> + <SHIFT> + J on my Debian Box)

mosra commented 8 years ago

Yes, sorry, this is exactly what I meant by saying that the ports branch is in a sad state -- the master wasn't merged in for a while and during that time the whole CMake buildsystem got a big overhaul, going from old-style "everything is global and/or variable" to much cleaner workflow using imported targets. This explains the colon cancer :)

Curious, if you run the example from the link above, does it show Suzanne or also fails with similar JavaScript error?

I might have some time during the weekend, so I'll try to clean it up and fix the issues -- the build system should be similarly easy to use as with the desktop builds, definitely not this hard and broken.

decltype-auto commented 8 years ago

Yes, sorry, this is exactly what I meant by saying that the ports branch is in a sad state

Could it be that one would better use the compatibility branches of corrade, magnum and magnum-plugins for the emscripten infrastructure for the ports branch of magnum-examples? I used the master branches for the infrastructure.

Curious, if you run the example from the link above, does it show Suzanne or also fails with similar JavaScript error?

Your showcase's Suzanne is all nice and well with current Iceweasel, Chrome and Epiphany on my Debian 8 box. And cool - literally: On both the CPU and the GPU there's very little payload from that container, thus you apparetnly implemented the update mimics very well. :)

Indeed, exactly the fact that that showcase is working that fine initially put me on the emscripten trek.

mosra commented 8 years ago

Indeed, exactly the fact that that showcase is working that fine initially put me on the emscripten trek.

ah! :)

Yes, you might get some more progress by using the compatibility branches, because their state might be at similar level of crappy and also their age should be similar, but then you have massively different API from what's now and I'm about to drop them, so I don't recommend it.

Your encouragement is making me want to give up everything else now and just focus on having the ports branch and Emscripten examples working again ... but I'm at work now. Can you please wait until tomorrow? :) I'll try to have this fixed ASAP.

mosra commented 8 years ago

Okay, I went all on it and:

If you check out the ports branch now, you should be able to build and run the triangle, textured-triangle, viewer and text examples without errors or annoying buildsystem issues. I encountered some weird error in the Suzanne material (randomly losing color), will investigate it later.

decltype-auto commented 8 years ago

@mosra: Heureka! That did it! Thank you very much for the fast bug fix!

I encountered some weird error in the Suzanne material (randomly losing color), will investigate it later.

Well - Suzi painted black? Call it a feature - that's an homage to the Rolling Stones. Period. :)

That's because the "lightcolor" Uniform is not set in Magnum's "phong.frag" if GL_ES is defined - try e.g.:

void main() {
    mediump vec4 WHITE = vec4(1.0);
    // [...]
    //color += finalDiffuseColor*lightColor*intensity;
    color += finalDiffuseColor*WHITE*intensity;
   // [...]
}

and no black anymore we want her to turn blue.

That's of course just a debug hack, don't know whether you want to fix that for WebGL in the C++ source or in the GLSL.

mosra commented 8 years ago

that's an homage to the Rolling Stones.

Hah :D

The default value is not set in GL_ES because GLSL ES doesn't support that, instead I have to set that explicitly from the client side as done here:

    ...
    /* Set defaults in OpenGL ES (for desktop they are set in shader code itself) */
    #ifdef MAGNUM_TARGET_GLES
    /* Default to fully opaque white so we can see the textures */
    if(flags & Flag::AmbientTexture) setAmbientColor(Color4{1.0f});
    else setAmbientColor(Color4{0.0f, 1.0f});
    ...

So I still have to investigate. But couldn't reliably reproduce today, so it was maybe just some random error (library mismatch?). Or does it look all black with just white speculars for you too?

decltype-auto commented 8 years ago

... /* Set defaults in OpenGL ES (for desktop they are set in shader code itself) _/

ifdef MAGNUM_TARGET_GLES

/_ Default to fully opaque white so we can see the textures */
if(flags & Flag::AmbientTexture) setAmbientColor(Color4{1.0f});
else setAmbientColor(Color4{0.0f, 1.0f});
...

Why "ambient", please? Your showcase's Suzi is certainly colored diffuse + specular (Lambert + Phong to be more precise.)

suzi_magnum

NW : screenshot of your showcase's Suzi NE: screenshot of Suzi here without lightColor set SW: screenshot of Suzi here without lightColor set, but ambientColor set to diffuseColor (aka "Toon shading") SE: screenshot of Suzi here with hardcoded white lightColor (pls see snippet above)

As one can see, btw, specular highlights are always visible.

And that's totally conclusive; here

https://github.com/mosra/magnum-examples/blob/master/src/viewer/ViewerExample.cpp#L394

void ColoredObject::draw(const Matrix4& transformationMatrix, SceneGraph::Camera3D& camera) {
    _shader->setAmbientColor(_ambientColor)
        .setDiffuseColor(_diffuseColor)
        .setSpecularColor(_specularColor)
        .setShininess(_shininess)
        .setLightPosition(camera.cameraMatrix().transformPoint({-3.0f, 10.0f, 10.0f}))
        .setTransformationMatrix(transformationMatrix)
        .setNormalMatrix(transformationMatrix.rotation())
        .setProjectionMatrix(camera.projectionMatrix());

    _mesh->draw(*_shader);
}

there are all colors set except lightColor, which is used and used only for the Lambert part of the Phong fragment shader.

mosra commented 8 years ago

Thanks a lot for the debugging, you pointed me to the right location! Just a few lines below the setup for default ambient color I posted above there was also default light color and that worked flawlessly for years so I assumed the problem just couldn't be there. But in fact it was! The problem originates in mosra/magnum@da2ac0047867d4a3fa9383236d8c61e1e05b256b, more precisely here (can you spot it?):

diff --git a/src/Magnum/Shaders/Phong.cpp b/src/Magnum/Shaders/Phong.cpp
index 4f76a9e..61aa8a3 100644
--- a/src/Magnum/Shaders/Phong.cpp
+++ b/src/Magnum/Shaders/Phong.cpp
@@ -113,13 +113,13 @@ Phong::Phong(const Flags flags): transformationMatrixUniform(0), projectionMatri
     /* Set defaults in OpenGL ES (for desktop they are set in shader code itself) */
     #ifdef MAGNUM_TARGET_GLES
     /* Default to fully opaque white so we can see the textures */
-    if(flags & Flag::AmbientTexture) setAmbientColor(Vector3{1.0f});
-    else setAmbientColor({});
+    if(flags & Flag::AmbientTexture) setAmbientColor(Color4{1.0f});
+    else setAmbientColor(Color4{0.0f, 1.0f});

-    if(flags & Flag::DiffuseTexture) setDiffuseColor(Vector3{1.0f});
+    if(flags & Flag::DiffuseTexture) setDiffuseColor(Color4{1.0f});

-    setSpecularColor(Vector3(1.0f));
-    setLightColor(Vector3(1.0f));
+    setSpecularColor(Color4{1.0f});
+    setLightColor(Color4{.0f});
     setShininess(80.0f);
     #endif
 }

It's fixed now in mosra/magnum@71f57b5378856de4c1c815cb415c6c840c23d725. Thanks again.

decltype-auto commented 8 years ago

It's fixed now in mosra/magnum@71f57b5 .

mosra/magnum@71f57b5

Shaders: fixed default Phong light color in ES builds.

The cause was careless coding in da2ac00. I desperately need those shader output tests :/

@mosra It's anyway not required to set unchanged uniforms again in every draw cycle. Why not generally encapsulate uniforms in a family of classes that push their value into the GL if and only if it was changed on the client side since the last cycle? Those classes could offer a runtime-configurable logging interface and handle race conditions in multi-threaded environments..

?

mosra commented 8 years ago

It's anyway not required to set unchanged uniforms again in every draw cycle.

How does that relate to the bugfix above? I'm not sure I understand.

Why not generally encapsulate uniforms in a family of classes that push their value into the GL if and only if it was changed on the client side since the last cycle?

Funnily enough I already tried this optimization in my previous project in order to reduce the number of GL calls per frame -- every shader class had a cache of the uniform values and every setter was first comparing the value to the cached one and only if it differed it was calling glUniform(). There was no measurable difference on desktop (NV GeForce) and slightly slower execution on mobile (NV Tegra 3), due to all the additional comparisons and memory jumps. So I reverted back to not caching anything.

The engine is doing state tracking of almost everything except uniforms, because I don't see any visible gain in doing so and the amount of code and testing needed for that would be huge.

If this proves to be a bottleneck in your application, you can always do one of these things:

Those classes could offer a runtime-configurable logging interface and handle race conditions in multi-threaded environments..

This is way beyond scope and KISS philosophy of the engine, sorry ;) Since OpenGL is inherently single-threaded, I don't see point in having thread-safe access to uniforms. If you need this, you can always wrap the shader class on application side, if you just need to see what uniforms are being set, you might be better off with some GL debugger/tracer such as Apitrace or Renderdoc.

decltype-auto commented 8 years ago

Funnily enough I already tried this optimization in my previous project in order to reduce the number of GL calls per frame -- every shader class had a cache of the uniform values and every setter was first comparing the value to the cached one and only if it differed it was calling glUniform().

Such comparisons are inherently expensive, better pimpl the setter to a no-op functor if the value is clean and to a glUniform() calling functor if it got dirtied.

Since OpenGL is inherently single-threaded, I don't see point in having thread-safe access to uniforms.

The point is that there's no limitation of what the rest of an app does to the data set by the glUniform calls and when.

mosra commented 8 years ago

Sorry, that still seems like a lot of coding and runtime overhead for questionable benefits. As I said above, if profiling shows that this functionality is needed, it can be always implemented on top of current simple shader API.

decltype-auto commented 8 years ago

It doesn't have to be all incorporated by Magnum right away, but to incorporate it efficiently in an application it at least needs an access point deep enough in the Magnum API stack. That's why I raise the notion of a uniform manager class.

mosra commented 8 years ago

I have a feeling that we are talking about vastly different things here. Maybe I misunderstood your original question or maybe you think Magnum works differently. Let me clarify:

In case this still doesn't answer your question, could you maybe explain your point with a small code snippet?