emscripten-ports / SDL2

Other
166 stars 64 forks source link

Browser full screen keypress (F11) and other shortcut keys don't work #128

Open adam4235 opened 4 years ago

adam4235 commented 4 years ago

I find that with an emscripten SDL2 website, pressing F11 to go into full screen doesn't work on either Firefox or Chrome. For example, I tried the sdl2_canvas_palette.c example included with emscripten. Some other keypresses that the browser would normally intercept, including Ctrl+O for opening a file and Ctrl+S for saving the webpage, also don't work, but some shortcuts do work, such as Ctrl+N for New Window.

Note that I'm talking about the browser's full screen functionality, not the Full Screen button that Emscripten puts on the webpage.

I can still use the browser's full screen functionality through its menu instead of through the shortcut key. However, then F11 doesn't work to get out of full screen.

It does work with a non-SDL2 Emscripten example, such as hello_world.c, which is what makes me think the problem is related to the porting of SDL2 to Emscripten.

Is this a bug, or is there some reason for these keys not doing anything? I wouldn't have thought it should be possible for a website to prevent a browser's shortcut keys from doing what they normally do.

Daft-Freak commented 4 years ago

By default we're preventing the default action on any event that has it's SDL equivalent enabled (SDL_EventState). Also by default, the keyboard events are attached to the window, this can be changed with SDL_SetHint(SDL_HINT_EMSCRIPTEN_KEYBOARD_ELEMENT, "#canvas") (or any CSS selector instead of "#canvas").

adam4235 commented 4 years ago

OK. That SDL_SetHint call does make the browser shortcut keys work again, but my application handles keypresses, and those no longer work when I call it. Is it possible to have only the keypresses I want to handle handled by the my application, but have other keypresses handled by the browser?

Daft-Freak commented 4 years ago

Ah, for keyboard events to work with that, the canvas needs to be given focus in JS (.focus()).

adam4235 commented 4 years ago

That doesn't seem to work for me, unless I'm misunderstanding you. This is my code:

    SDL_SetHint(SDL_HINT_EMSCRIPTEN_KEYBOARD_ELEMENT, "#canvas");
    EM_ASM({document.getElementById("canvas").focus();});
    emscripten_set_main_loop(mainLoopFunction, -1, 1);

The keypress events that were happening without the SDL_SetHint call are still no longer happening.

adam4235 commented 4 years ago

I do notice that when I also add tabindex='1' to my canvas element, which seems necessary to make a canvas focusable, then my keypress events work again. But then Firefox's F11 shortcut key stops working.

Daft-Freak commented 4 years ago

Sorry, yeah you need either to make the canvas have a tab index, or set the focus manually in a mouse handler. Demo with document.getElementById('canvas').addEventListener("mouseup", e => e.target.focus());: https://cupboard.daftgames.net/em/testinput.html

adam4235 commented 4 years ago

The demo seems to have the same problem I'm seeing in my code: when the website loads, I can press F11 to make it fullscreen, but keypresses aren't caught to be handled by the app. If I click the canvas, keypresses are then caught and handled by the app (printed, in this case), but pressing F11 is also caught and handled by the app and gets printed. What I want is for keypresses to be handled by my app, but when I press F11, it should forward to the browser for fullscreen (or, more generally, any keypress I don't handle should be forwarded to the browser in case it has a keyboard shortcut for that keypress). Is that possible?

Daft-Freak commented 4 years ago

It's not currently possible to filter events like that. I think it might be possible to add though.

This change:

diff --git a/src/video/emscripten/SDL_emscriptenevents.c b/src/video/emscripten/SDL_emscriptenevents.c
index bfa0cbd47..bf0dfc1c3 100644
--- a/src/video/emscripten/SDL_emscriptenevents.c
+++ b/src/video/emscripten/SDL_emscriptenevents.c
@@ -495,7 +495,7 @@ static EM_BOOL
 Emscripten_HandleKey(int eventType, const EmscriptenKeyboardEvent *keyEvent, void *userData)
 {
     Uint32 scancode;
-    SDL_bool prevent_default;
+    SDL_bool prevent_default = SDL_FALSE;
     SDL_bool is_nav_key;

     /* .keyCode is deprecated, but still the most reliable way to get keys */
@@ -569,12 +569,10 @@ Emscripten_HandleKey(int eventType, const EmscriptenKeyboardEvent *keyEvent, voi
                         break;
                 }
             }
-            SDL_SendKeyboardKey(eventType == EMSCRIPTEN_EVENT_KEYDOWN ? SDL_PRESSED : SDL_RELEASED, scancode);
+            prevent_default = SDL_SendKeyboardKey(eventType == EMSCRIPTEN_EVENT_KEYDOWN ? SDL_PRESSED : SDL_RELEASED, scancode);
         }
     }

-    prevent_default = SDL_GetEventState(eventType == EMSCRIPTEN_EVENT_KEYDOWN ? SDL_KEYDOWN : SDL_KEYUP) == SDL_ENABLE;
-
     /* if TEXTINPUT events are enabled we can't prevent keydown or we won't get keypress
      * we need to ALWAYS prevent backspace and tab otherwise chrome takes action and does bad navigation UX
      */

should cause keyboard events filtered out through SDL_SetEventFilter to be handled by the browser. (Also unhandled keys.) I haven't tested if this actually works yet though.

adam4235 commented 4 years ago

Thanks, I'd be willing to try it (although my code currently doesn't use SDL_SetEventFilter) but I'm wondering how do I recompile the SDL2 emscripten port with a change? These instructions just say that ports get downloaded and built automatically:

https://emscripten.org/docs/compiling/Building-Projects.html#emscripten-ports

I tried modifying the file in the downloaded code and rebuilding my project, but SDL_emscriptenevents.c wasn't recompiled.

Daft-Freak commented 4 years ago

If you clone this repo somewhere, you can use EMCC_LOCAL_PORTS="sdl2=/path/to/sdl/repo".

adam4235 commented 4 years ago

OK I did that and got your change compiled in. But I doesn't seem to fix my problem, unless I'm doing something wrong (sorry I'm not familiar with using SDL_SetEventFilter, and I can't find much documentation for it online). This is what I did:

int FilterEvents(void* userdata, SDL_Event *event) {
    if (event->type == SDL_KEYDOWN && event->key.keysym.sym == SDLK_F11) {
        return 0;
    }
    return 1;
}
...
    SDL_SetEventFilter(FilterEvents, NULL);
    while (SDL_PollEvent(&input) > 0) {
        <handle input>
    }

The while loop was there before and is what works fine in a non-emscripten build. When I add the SDL_SetEventFilter call after compiling in your patch, it does cause F11 to now be handled by the web browser, but now none of my keypresses have any effect in the while loop. Did I use SDL_SetEventFilter correctly?

Daft-Freak commented 4 years ago

Hmm, looks right and seems to work here.

#ifdef __EMSCRIPTEN__
#include <emscripten.h>
#endif

#include "SDL.h"

#include <stdio.h>

static int run = 1;

SDL_Window *window;

void loop()
{
  SDL_Event event;
  while (SDL_PollEvent(&event)) {
    switch(event.type) {

    case SDL_TEXTINPUT:
      printf("input %s\n", event.text.text);
      break;

    case SDL_KEYDOWN:
      printf("keydown %s\n", SDL_GetKeyName(event.key.keysym.sym));
      break;

    case SDL_KEYUP:
      printf("keyup %s\n", SDL_GetKeyName(event.key.keysym.sym));
      break;

    case SDL_QUIT:
      run = 0;
      break;
    }
  }
}

int FilterEvents(void* userdata, SDL_Event *event) {
    if (event->type == SDL_KEYDOWN && event->key.keysym.sym == SDLK_F11) {
        return 0;
    }
    return 1;
}

int main(int argc, char *argv[])
{
  if (SDL_Init(SDL_INIT_VIDEO) < 0) {
    return (1);
  }

  //SDL_SetHint(SDL_HINT_EMSCRIPTEN_KEYBOARD_ELEMENT, "#canvas");

  window = SDL_CreateWindow("Test", SDL_WINDOWPOS_CENTERED, SDL_WINDOWPOS_CENTERED, 640, 480, 0);
  SDL_SetEventFilter(FilterEvents, NULL);

#ifdef __EMSCRIPTEN__
  emscripten_set_main_loop(loop, 0, 1);
#else
  while (run) loop();
#endif

  return 0;
}
adam4235 commented 4 years ago

Hmm, yeah that test also works for me, when I use the emscripten generated html file (For my app I'm generating a .js file and writing a bare-bones html file so I don't have the "powered by emscripten" etc. surrounding it). I'll have to debug to determine why it isn't working with my code.

adam4235 commented 4 years ago

It does seem to work for me now once I moved the call to SDL_SetEventFilter to the beginning of my program and only call it once. It wasn't working when I was calling it repeatedly every frame. I'm not sure why that would be.

Daft-Freak commented 4 years ago

I think SDL_SetEventFilter may flush the event queue, so possibly all the events were getting dropped... Now I just need to do some tests to make sure that patch doesn't break anything.

caiiiycuk commented 3 years ago

Another workaround that may help someone. I need to reload page by F5, Ctr+R I did it like this:

        window.addEventListener("keydown", (e) => {
            if (e.code === "F5" || e.code === "KeyR") {
                e.preventDefault = function() { console.log("ignore prevent default"); };
            }
        }, { capture: true });