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

Emscripten full screen and antialiasing of 2d-shapes do not work anymore #6758

Closed
Jonathhhan opened this issue Jul 21, 2021 · 20 comments
Closed

Comments

@Jonathhhan
Copy link
Contributor

Jonathhhan commented Jul 21, 2021

I get this message when i press full screen:

Uncaught TypeError: Module.requestFullScreen is not a function
    at HTMLInputElement.onclick ((index):1)

And the circle isnt antialiased, while it is when I compile for desktop. And both were working with Emscripten before (I guess until OF 0.11.0). Here for example works both: https://arturocastro.net/files/of-emscripten/polygonExample/
And here not (my example - but it seems the same for the other examples): http://puredatagui.handmadeproductions.de/
Antialiasing fonts does work now, after I made this change: #6745
Tested with OF 0.11.2 / Emscripten 1.40.1 / Ubuntu 20.04

@Jonathhhan Jonathhhan changed the title Emscripten full screen and antialiasing does not work anymore Emscripten full screen and antialiasing do not work anymore Jul 21, 2021
@Jonathhhan
Copy link
Contributor Author

Jonathhhan commented Jul 21, 2021

Maybe the full screen issue is related to that:
emscripten-ports/SDL2#8
OGRECave/ogre#1698
OGRECave/ogre#1289

@Jonathhhan
Copy link
Contributor Author

Jonathhhan commented Sep 7, 2021

Regarding the fullscreen issue (not sure if it is helpful): If I use Module.requestFullscreen with a lower case "s" like in my example above, I can enter a black fullscreen without an error message, but if I switch back the webassembly canvas is gone (or minimized to the size 0x0).
emscripten-core/emscripten#4318

@ofTheo ofTheo added this to the 0.12.0 milestone Sep 8, 2021
@ofTheo ofTheo mentioned this issue Sep 8, 2021
77 tasks
@ofTheo
Copy link
Member

ofTheo commented Sep 8, 2021

Thank you for digging into this.

I added this to our 0.12.0 tracking issue.
If you find solutions to either feel free to submit a PR. :)

@Jonathhhan Jonathhhan changed the title Emscripten full screen and antialiasing do not work anymore Emscripten full screen and antialiasing of 2d-shapes do not work anymore Sep 9, 2021
@ofTheo
Copy link
Member

ofTheo commented Oct 6, 2021

From the forum this works for me for the fullscreen issue:
<input type="button" value="Fullscreen" onclick="Module['canvas'].requestFullscreen()">

I can toggle back and forth and things work as expected.

@Jonathhhan
Copy link
Contributor Author

Jonathhhan commented Oct 6, 2021

You can also use pointerlock with "Module['canvas'].requestPointerLock()". What does not work with this method is recalculating the mouse position and resizing the canvas. For that I had to change set_canvas_element_size to set_canvas_size in ofxAppEmscriptenWindow.cpp (and use Module.requestFullscreen()). set_canvas_size is deprecated, but it is still used for fullscreen in https://github.com/emscripten-core/emscripten/blob/main/src/library_browser.js:

void ofxAppEmscriptenWindow::setWindowShape(int w, int h){
    emscripten_set_canvas_size(w,h);
}

glm::vec2 ofxAppEmscriptenWindow::getWindowPosition(){
	return glm::vec2(0,0);
}

glm::vec2 ofxAppEmscriptenWindow::getWindowSize(){
	int width;
	int height;
	int isFullscreen;
    emscripten_get_canvas_size(&width, &height, &isFullscreen);
	return glm::vec3(width,height,isFullscreen);
}

And in the Emscripten library library_html5.js I changed this, so the mouse is also recalculated if the canvas is not resized:


#if !DISABLE_DEPRECATED_FIND_EVENT_TARGET_BEHAVIOR
    if (Module['canvas']) {
      var rect = __getBoundingClientRect(Module['canvas']);
      HEAP32[idx + {{{ C_STRUCTS.EmscriptenMouseEvent.canvasX / 4 }}}] = (e.clientX - rect.left) * (Module['canvas'].width / rect.width);
      HEAP32[idx + {{{ C_STRUCTS.EmscriptenMouseEvent.canvasY / 4 }}}] = (e.clientY - rect.top) * (Module['canvas'].height / rect.height);
    } else { // Canvas is not initialized, return 0.
      HEAP32[idx + {{{ C_STRUCTS.EmscriptenMouseEvent.canvasX / 4 }}}] = 0;
      HEAP32[idx + {{{ C_STRUCTS.EmscriptenMouseEvent.canvasY / 4 }}}] = 0;
    }
#endif
    var rect = __getBoundingClientRect(target);
    HEAP32[idx + {{{ C_STRUCTS.EmscriptenMouseEvent.targetX / 4 }}}] = (e.clientX - rect.left) * (target.width / rect.width);
    HEAP32[idx + {{{ C_STRUCTS.EmscriptenMouseEvent.targetY / 4 }}}] = (e.clientY - rect.top) * (target.height / rect.height);

@ofTheo
Copy link
Member

ofTheo commented Oct 6, 2021

wow thanks @Jonathhhan !
will put this all together in a PR.

@Jonathhhan
Copy link
Contributor Author

Jonathhhan commented Oct 6, 2021

@ofTheo thanks. Here you can test how fullscreen works with the changes from above: https://simplesequencer.handmadeproductions.de/

Not really related, but I also found a way to use DISABLE_DEPRECATED_FIND_EVENT_TARGET_BEHAVIOR=1.
For that I had to replace target with "#canvas" in this line var domElement = specialHTMLTargets[target] || (typeof document !== 'undefined' ? document.querySelector(target) : undefined); in https://github.com/emscripten-core/emscripten/blob/main/src/library_html5.js and replace canvasX / canvasY with targetX / targetY in https://github.com/openframeworks/openFrameworks/blob/master/addons/ofxEmscripten/src/ofxAppEmscriptenWindow.cpp But since it is easier to use the flag DISABLE_DEPRECATED_FIND_EVENT_TARGET_BEHAVIOR=0 than to edit an external file, I am not sure if it makes sense.

@Jonathhhan
Copy link
Contributor Author

Jonathhhan commented Oct 7, 2021

And antialiasing works again if I add antialias = true to webGLContextAttributes in https://github.com/emscripten-core/emscripten/blob/main/src/library_webgl.js (I also needed to change webgl to webgl2, because I use webgl2):

    // Returns the context handle to the new context.
    createContext: function(canvas, webGLContextAttributes) {
    webGLContextAttributes.antialias = true;

Only maybe not very practical to change a lot of Emscripten files...

@Jonathhhan
Copy link
Contributor Author

Jonathhhan commented Oct 7, 2021

Maybe it would make sense to change a few additional webGLContextAttributes parameters (not sure, but I have the feeling it improves the performance slightly)?
I tried:

        webGLContextAttributes.alpha = true;
        webGLContextAttributes.antialias = true;
        webGLContextAttributes.powerPreference = "high-performance";
        webGLContextAttributes.majorVersion = 2;

console.log(webGLContextAttributes):

{alpha: true, depth: true, stencil: false, antialias: true, majorVersion: 2, …}
alpha: true
antialias: true
depth: true
majorVersion: 2
minorVersion: 0
powerPreference: "high-performance"
stencil: false

About alpha I found:
"Avoid alpha:false, which can be expensive
Specifying alpha:false during context creation causes the browser to composite the WebGL-rendered canvas as though it were opaque, ignoring any alpha values the application writes in its fragment shader. On some platforms, this capability unfortunately comes at a significant performance cost. The RGB back buffer may have to be emulated on top of an RGBA surface, and there are relatively few techniques available in the OpenGL API for making it appear to the application that an RGBA surface has no alpha channel. It has been found that all of these techniques have approximately equal performance impact on affected platforms.

Most applications, even those requiring alpha blending, can be structured to produce 1.0 for the alpha channel. The primary exception is any application requiring destination alpha in the blending function. If feasible, it is recommended to do this rather than using alpha:false."
https://developer.mozilla.org/en-US/docs/Web/API/WebGL_API/WebGL_best_practices

And about powerPreference:
emscripten-core/emscripten#10000

@ofTheo
Copy link
Member

ofTheo commented Oct 7, 2021

@Jonathhhan Yeah, I wonder if it is possible to pass through these things from OF so we don't have to set them inside of emscripten.

In the .js file generate by emmake in bin/ ( ie: polgonExample.js ) I see:
if(useWebGL){var contextAttributes={antialias:false,alpha:false,majorVersion:1};

I tried setting it to true there, but it didn't seem to make a difference.

The window / context creation is done in here:
https://github.com/openframeworks/openFrameworks/blob/master/addons/ofxEmscripten/src/ofxAppEmscriptenWindow.cpp

I think whatever we did would have to account for if antialias is supported on the client side - so it might need to be something at the javascript level or maybe using a similar approach to how we handled the depth resolution here:
06a9366#diff-f3ce187f3e25e12b72b32629cd308f41075d821206fa7fedd69401ae03f91944

@Jonathhhan
Copy link
Contributor Author

Jonathhhan commented Oct 8, 2021

@ofTheo Your suggestion works great. This is the result:

alpha: true
antialias: true
depth: true
majorVersion: 1
minorVersion: 0
stencil: false

Here is the updated ofxAppEmscriptenWindow.cpp (it also works if I just set 8 for EGL_ALPHA_SIZE and 1 for EGL_SAMPLE_BUFFERS but like you said, that could be problematic):

https://github.com/Jonathhhan/openFrameworks/blob/master/addons/ofxEmscripten/src/ofxAppEmscriptenWindow.cpp

Only not sure if it is possible to set the majorVersion to 2 in ofxAppEmscriptenWindow.cpp and if it is even necessary (seems to work like it is, only that I have to select webgl2 manually - if used). And I wonder how to define the webgl context id name in ofxAppEmscriptenWindow.cpp (for changing the argument "target" in https://github.com/emscripten-core/emscripten/blob/main/src/library_html5.js from 0 to "#canvas"), so that DISABLE_DEPRECATED_FIND_EVENT_TARGET_BEHAVIOR=1 works without editing the emscripten file.

@ofTheo
Copy link
Member

ofTheo commented Oct 8, 2021

Thanks so much @Jonathhhan for figuring this out! 🤘

@Jonathhhan
Copy link
Contributor Author

Jonathhhan commented Oct 9, 2021

Now it works also without -s DISABLE_DEPRECATED_FIND_EVENT_TARGET_BEHAVIOR=0
Changed this in https://github.com/Jonathhhan/openFrameworks/blob/master/addons/ofxEmscripten/src/ofxAppEmscriptenWindow.cpp:

    emscripten_set_keydown_callback("#canvas",this,1,&keydown_cb);
    emscripten_set_keyup_callback("#canvas",this,1,&keyup_cb);
    emscripten_set_mousedown_callback("#canvas",this,1,&mousedown_cb);
    emscripten_set_mouseup_callback("#canvas",this,1,&mouseup_cb);
    emscripten_set_mousemove_callback("#canvas",this,1,&mousemoved_cb);

    emscripten_set_touchstart_callback("#canvas",this,1,&touch_cb);
    emscripten_set_touchend_callback("#canvas",this,1,&touch_cb);
    emscripten_set_touchmove_callback("#canvas",this,1,&touch_cb);
    emscripten_set_touchcancel_callback("#canvas",this,1,&touch_cb);

And and replaced canvasX / canvasY with targetX / targetY.

Only thing for now that the recalculation of the mouse value in fullscreen mode still happens (with my edit) in https://github.com/emscripten-core/emscripten/blob/main/src/library_html5.js not sure how I can get the css canvas size into Open Frameworks.

The other big issue (maybe more an improvement) is, that it would be really great, if audio could run in a seperate JS thread for reducing latency and audio artifacts. Like with audioworkletprocessor https://developer.mozilla.org/en-US/docs/Web/API/AudioWorkletProcessor and not with ScriptProcessorNode https://developer.mozilla.org/en-US/docs/Web/API/ScriptProcessorNode. Maybe also pthreads https://emscripten.org/docs/porting/pthreads.html could be a solution? I think I understand the basic concept of audioworkletprocessor, but I dont know how to pass the wasm module (already tried a few things, without much success). Also not sure, if the bottleneck is somewhere else, but I guess its the fact that everything happens in the main JS thread. Anyhow, with a buffersize of 4096 audio is quite stable right now, but with a lower buffer size it depends on the browser and the CPU/GPU.

@Jonathhhan
Copy link
Contributor Author

Jonathhhan commented Oct 11, 2021

changing this:

EGLint contextAttribs[] = { EGL_CONTEXT_MAJOR_VERSION, 3, EGL_NONE, EGL_NONE };
	std::vector <EGLint> attribList =
	   {
		   EGL_RED_SIZE, EGL_DONT_CARE,
		   EGL_GREEN_SIZE, EGL_DONT_CARE,
		   EGL_BLUE_SIZE, EGL_DONT_CARE,
		   EGL_ALPHA_SIZE, EGL_DONT_CARE,
		   EGL_DEPTH_SIZE, EGL_DONT_CARE,
		   EGL_STENCIL_SIZE, EGL_DONT_CARE,
		   EGL_SAMPLE_BUFFERS, EGL_DONT_CARE,
		   EGL_RENDERABLE_TYPE, EGL_OPENGL_ES3_BIT_KHR,
		   EGL_NONE
	   };

in https://github.com/Jonathhhan/openFrameworks/blob/master/addons/ofxEmscripten/src/ofxAppEmscriptenWindow.cpp and adding the flag -s USE_WEBGL2=1 to config.emscripten.default.mk enables webgl2 without editing any Emscripten file (in addition I use -s WEBGL2_BACKWARDS_COMPATIBILITY_EMULATION=1 for backwards compatibility with webgl1 https://emscripten.org/docs/optimizing/Optimizing-WebGL.html).

@ofTheo
Copy link
Member

ofTheo commented Oct 12, 2021

Thanks @Jonathhhan !

I think ideally you would be able to set the GL version via the ofGLESWindowSettings::glesVersion in the settings that is passed through to the ofxAppEmscriptenWindow.cpp::setup function here: https://github.com/openframeworks/openFrameworks/blob/master/addons/ofxEmscripten/src/ofxAppEmscriptenWindow.cpp#L56

@Jonathhhan
Copy link
Contributor Author

Jonathhhan commented Oct 12, 2021

@ofTheo Yes, that would be better. Just dont know how to set ofGLESWindowSettings::glesVersion (or can I just use

ofGLESWindowSettings settings;
settings.setGLESVersion(3.0);
ofCreateWindow(settings);

?) and pass it to the EGL context (I guess, I still need the -s USE_WEBGL2=1 and -s WEBGL2_BACKWARDS_COMPATIBILITY_EMULATION=1 flags).

Edit: Tried to get the glesVersion without success (I think I set it right).

On the other hand: Maybe I can always use webgl2 with the -s WEBGL2_BACKWARDS_COMPATIBILITY_EMULATION=1 flag, but the device / browser needs to support webgl2 then (which would also be the case if ofGLESWindowSettings::glesVersion is set to GLES 3).

@Jonathhhan
Copy link
Contributor Author

Jonathhhan commented Oct 13, 2021

@ofTheo thank you very much for your help. I would say, that basically this issues (and some others) are solved. Only the question, how to implement the solutions in the best way for the pull requests (hard for me to judge, because I am really not a experienced c++ programmer).
Last two Emscripten related issues (that I have in mind), that are both not very urgent: Update OF for the current Emscripten version (I use 2.0.6 at the moment) and make audio run in a seperate thread (or reduce latency / artifacts in another way).

One small thing, that isnt really an issue: The mouse events do not work, if the mouse is outside of the Emscripten canvas (I guess it works with the normal OF apps).

@ofTheo
Copy link
Member

ofTheo commented Oct 14, 2021

Thanks @Jonathhhan !
This is all immensely helpful.

I will probably pull in everything from your PR and maybe do a bit of cleanup and small fixes to make it more modular as you suggested.

I can change the workflows now to use 2.0.6 and we can change the site docs to suggest that version too.
Does 2.0.6 work just as well with the current release of OF?

Thanks!!

@Jonathhhan
Copy link
Contributor Author

Jonathhhan commented Oct 14, 2021

@ofTheo just testet it with the current OF nightly built for Linux (I always use Linux / Ubuntu for the Emscripten stuff) and it works well.

And I tried to use pthreads with this in mind:
"-s PROXY_TO_PTHREAD: In this mode your original main() is replaced by a new one that creates a pthread and runs the original main() on it. As a result, your application’s main() is run off the browser main (UI) thread, which is good for responsiveness. The browser main thread does still run code when things are proxied to it, for example to handle events, rendering, etc. The main thread also does things like create pthreads for you, so that you can depend on them synchronously."
https://emscripten.org/docs/porting/pthreads.html
I changed this in config.emscripten default:

PLATFORM_CFLAGS = -s USE_PTHREADS=1
PLATFORM_CXXFLAGS = -Wall -std=c++14 -Wno-warn-absolute-paths -s USE_PTHREADS=1

and:
PLATFORM_LDFLAGS = -Wl,--gc-sections --preload-file bin/data@data --emrun -s USE_WEBGL2=1 -s WEBGL2_BACKWARDS_COMPATIBILITY_EMULATION=1 -s USE_PTHREADS=1 -s PTHREAD_POOL_SIZE=2 -s PROXY_TO_PTHREAD=1

Almost everything compiles fine, but then i get the error message:
wasm-ld: error: --shared-memory is disallowed by TagLib.o because it was not compiled with 'atomics' or 'bulk-memory' features.
The strange thing is, that I cant find any TagLib file... Anyhow, not sure if my pthreads experiment makes sense at all.

One last thing: In https://github.com/openframeworks/openFrameworks/blob/master/addons/ofxEmscripten/libs/html5audio/lib/emscripten/library_html5audio.js I define the samplerate for the audioContext like:

        var context = new AudioContext({
            latencyHint: "interactive",
            sampleRate: 44100
        });

Otherwise the samplerate is set to the samplerate of the sound device (which results in different pitch and tempo). In the best case it could also be set as an argument.

@Jonathhhan
Copy link
Contributor Author

Jonathhhan commented Oct 28, 2021

Some new findings: It is possible to compile OF patches with Emscripten with c++17, and to use std::filesystem instead of boost::filesystem (so boost isnt necessary anymore?).

For that I had to set #define OF_USING_STD_FS in ofConstants.h to 1,
change PLATFORM_CXXFLAGS = -Wall -std=c++17 -Wno-warn-absolute-paths -s USE_PTHREADS=1 in config.emscripten.default.mk (pthreads is only needed for threads or sharedArrayBuffer).
-s USE_PTHREADS=1 only works, if everything is compiled with 'atomics' or 'bulk-memory' features.
For that I had to get rid of boost and exclude FreeImage lib for the moment, because they aren't.

PLATFORM_CORE_EXCLUSIONS += $(OF_LIBS_PATH)/FreeImage/lib/emscripten/%
PLATFORM_CORE_EXCLUSIONS += $(OF_LIBS_PATH)/boost/%

In ofFileUtils.h I added #include <unistd.h>.

Here is a temporary example (the graphicsExample) compiled with pthreads, std::filesystem and c++17, but not sure how to use the worker / sharedArrayBuffer: https://gameoflife3d.handmadeproductions.de/

Edit: Actually my changes broke the filesystem: missing function: _Z12ofToDataPathRKNSt10filesystem4pathEb, but it works for patches that do not load files with threads and without boost now...
Edit2: haha, the biggest .wasm file size reduction so far came from excluding the freeimage lib(from 6MB to 1MB approximately)...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants