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

Soft fullscreen removes <body> element so document.body.style causes null deref #49

Closed
satoshinm opened this issue Apr 9, 2017 · 2 comments · Fixed by #52
Closed

Soft fullscreen removes <body> element so document.body.style causes null deref #49

satoshinm opened this issue Apr 9, 2017 · 2 comments · Fixed by #52

Comments

@satoshinm
Copy link
Owner

Testing on Chrome, press F11 to enter fullscreen, F11 again to exit, then this exception is thrown:

Uncaught TypeError: Cannot read property 'style' of null
at HTMLDocument.restoreOldStyle (craft.js:9026)

because emscripten's soft fullscreen seems to remove all other elements in the html document including <body>, so document.body is null!

screen shot 2017-04-09 at 9 42 10 am

Attempted in #48 to workaround this by doing my own "windowed full" mode, but it stretches the canvas undesirably, so using soft fullscreen in emscripten is desirable but this is probably a bug in its implementation (who removes body from html? bad idea?).

@satoshinm
Copy link
Owner Author

Didn't see this before, but could this be useful? https://github.com/kripken/emscripten/blob/master/tests/test_glfw_fullscreen.c - not using glfwSetWindowMonitor added in #9 since emscripten doesn't have it (glfw 3.2+), but rather emscripten's fullscreen and a glfw window size callback

@satoshinm
Copy link
Owner Author

satoshinm commented Apr 10, 2017

Starting from test_glfw_fullscreen, writing a minimal example, am able to successfully toggle between fullscreen and "fullwindow" (emscripten soft fullscreen), whipped up a demo here: test_fullscreen_fullwindow_toggle (source) - press F11 to toggle. So there may be some error on my end causing this body tag problem.

  • test_fullscreen_fullwindow_toggle has at least one important difference: listens for window size changes with glfwSetWindowSizeCallback, not .canvasResizedCallback. NetCraft is using the latter, as on_canvassize_changed(), to set glfwSetWindowSize(). test_fullscreen_fullwindow_toggle only is using a solid gray background currently, so I can't see the stretching/scaling effects, but it does correctly fill the entire screen and window.

  • test_fullscreen_fullwindow_toggle also is able to toggle fullscreen using F11 through glfwSetKeyCallback, unlike in NetCraft where I thought I had to register a separate fullscreen_key_callback from emscripten_set_keydown_callback to be considered a "user action" for the purposes of calling the fullscreen APIs as a result of user action, but it works either way. This means it ought to be possible to simplify and remove the emscripten_set_keydown_callback special case for F11 from NetCraft.


In the working test_fullscreen_fullwindow_toggle example, using EM_ASM(Module.requestFullscreen(1, 1)); which is the same as pressing the "Enter Fullscreen" button in the default shell.html. But to set a canvas resized callback, and other parameters like whether to stretch emscripten_request_fullscreen_strategy() is used, but if it is replaced then fullscreen fails:

void on_key(GLFWwindow *window, int key, int scancode, int action, int mods) {
    if (action && GLFW_PRESS && key == GLFW_KEY_F11) {
        printf("glfwSetKeyCallback: F11 pressed\n");
        // F11 toggles between fullscreen and fullwindow ("soft fullscreen") mode. The app starts
        // in fullwindow, a solid gray page. When fullscreen is entered with F11, the entire
        // screen should be solid gray. Returning to fullwindow with F11 should also show the same.

        EmscriptenFullscreenChangeEvent fsce;
        EMSCRIPTEN_RESULT ret = emscripten_get_fullscreen_status(&fsce);

        if (!fsce.isFullscreen) {
            emscripten_exit_soft_fullscreen();

            // Enter fullscreen
            EmscriptenFullscreenStrategy strategy = {
                .scaleMode = EMSCRIPTEN_FULLSCREEN_SCALE_STRETCH,
                .canvasResolutionScaleMode = EMSCRIPTEN_FULLSCREEN_CANVAS_SCALE_STDDEF,
                .filteringMode = EMSCRIPTEN_FULLSCREEN_FILTERING_DEFAULT,
                .canvasResizedCallback = on_canvassize_changed,
                .canvasResizedCallbackUserData = NULL
            };
            EMSCRIPTEN_RESULT ret = emscripten_request_fullscreen_strategy(NULL, EM_TRUE, &strategy);
            printf("emscripten_request_fullscreen_strategy = %d\n", ret);
            //EM_ASM(Module.requestFullscreen(1, 1))

ret=1 is #define EMSCRIPTEN_RESULT_DEFERRED 1, meaning "The requested operation cannot be completed now for web security reasons, and has been deferred for completion in the next event handler."... but how did EM_ASM(Module.requestFullscreen(1, 1)) succeed in its place?


test_fullscreen_fullwindow_toggle had a bug: enter fullscreen with F11, then exit with Escape (instead of F11). This takes you back to "presentation mode", instead of fullwindow. update: fixed in satoshinm/emglfwbugs@235fc0c with emscripten_set_fullscreenchange_callback()


test_fullscreen_fullwindow_toggle works well as a test, but in porting it into NetCraft, F11 toggle now works well, but a new problem arises: resizing the window to the size of the canvas, as the browser window changes, instead of leaving it fixed dimensions:

screen shot 2017-04-09 at 6 56 14 pm

glfwSetWindowSizeCallback() is used in my test_fullscreen_fullwindow_toggle (and the official emscripten test_glfw_fullscreen), but to show whether fullscreen was successfully entered. It detects the glfw window size change, this includes fullscreen transitions:

diff --git a/src/main.c b/src/main.c
index 6989eb8..be85f2e 100644
--- a/src/main.c
+++ b/src/main.c
@@ -2419,6 +2419,24 @@ void on_mouse_button(GLFWwindow *window, int button, int action, int mods) {
     }
 }
 
+static int inFullscreen = 0;
+static int wasFullscreen = 0;
+void windowSizeCallback(GLFWwindow* window, int width, int height) {
+    int isInFullscreen = EM_ASM_INT_V(return !!(document.fullscreenElement || document.mozFullScreenElement || document.webkitFullscreenElement || document.msFullscreenElement));
+    if (isInFullscreen && !wasFullscreen) {
+        printf("Successfully transitioned to fullscreen mode!\n");
+        wasFullscreen = isInFullscreen;
+    }
+
+    if (wasFullscreen && !isInFullscreen) {
+        printf("Exited fullscreen. Test succeeded.\n");
+        wasFullscreen = isInFullscreen;
+        //emscripten_cancel_main_loop();
+        return;
+    }
+	printf("windowSizeCallback %d x %d\n", width, height);
+}
+
 #ifdef __EMSCRIPTEN__
 // Emscripten's "soft fullscreen" = maximizes the canvas in the browser client area, wanted to toggle soft/hard fullscreen
 void maximize_canvas() {
@@ -2808,6 +2826,7 @@ int main(int argc, char **argv) {
 #else // web pointer lock requires user action to activate, start off disabled
     glfwSetInputMode(g->window, GLFW_CURSOR, GLFW_CURSOR_DISABLED);
 #endif
+	glfwSetWindowSizeCallback(g->window, windowSizeCallback);
     glfwSetKeyCallback(g->window, on_key);
     glfwSetCharCallback(g->window, on_char);
     glfwSetMouseButtonCallback(g->window, on_mouse_button);

, but not windowed browser resize. For that, there is canvasResizedCallback but I can only find to set it in EmscriptenFullscreenStrategy, which I'm not using (instead, EM_ASM(Module.requestFullscreen(1,1)). Time for another EM_ASM to get resize events?


emscripten has emscripten_set_canvas_size(), so could check that on each iteration, if changes, then glfwSetWindowSize. continue in #52

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