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

After using the Fullscreen button, HTML5 library full screen functions fail #3283

Closed
dreamlayers opened this issue Mar 23, 2015 · 9 comments
Closed
Labels

Comments

@dreamlayers
Copy link
Contributor

The "Fullscreen" button in the top right corner of Emscripten-generated web pages uses Module.requestFullScreen(), which calls Browser.requestFullScreen(). This adds the fullScreenChange() event listener.

Browser.requestFullScreen() creates a div where canvas used to be, moves canvas into that div, making the div the full screen element. Code in fullScreenChange() is meant to undo this change, by moving canvas to into the parent of its parent.

When SDL 2 calls emscripten_request_fullscreen_strategy() and makes canvas the full screen element, fullScreenChange() code for moving canvas up is inappropriately triggered. The function assumes full screen is being entered only if the parent of canvas is the full screen element, and otherwise assumes full screen being exited and canvas needs to be moved back up to its original location. That exits out of full screen, calling fullScreenChange() again. The end result is canvas replaced body and it's not in full screen.

@dreamlayers
Copy link
Contributor Author

The following code can demonstrate this issue. Press the "Fullscreen" button, tell the browser to allow full screen if necessary, and then press Escape to exit full screen. Then press 1 to enter full screen via SDL 2 and see what happens. To confirm claims above, you can use the inspector in developer tools or set a breakpoint in fullScreenChange() after pressing Escape.

#include <stdio.h>
#include <SDL.h>
#include <emscripten.h>

#define WIDTH 600
#define HEIGHT 450
#define BAR_WIDTH 25
#define BAR_MOVEMENT 20

static SDL_Window *window = 0;
static SDL_Renderer *renderer = 0;

static void render() {
  static int direction = BAR_MOVEMENT;
  static SDL_Rect r = { .x = 0, .y = 0, .h = HEIGHT, .w = BAR_WIDTH };
  int newx;

  SDL_SetRenderDrawColor(renderer, 0, 0, 0, 255);
  SDL_RenderClear(renderer);
  SDL_SetRenderDrawColor(renderer, 255, 255, 255, 255);
  SDL_RenderFillRect(renderer, &r);
  SDL_RenderPresent(renderer);

  newx = r.x;
  newx += direction;
  if (direction < 0) {
    if (newx < 0) {
      newx = 0;
      direction = BAR_MOVEMENT;
    }
  } else {
    if (newx + BAR_WIDTH >= WIDTH) {
      newx = WIDTH - BAR_WIDTH;
      direction = -BAR_MOVEMENT;
    }
  }
  r.x = newx;
}

static void mainloop() {
  render();
  SDL_Event event;
  int haveEvent = SDL_PollEvent(&event);
  if (haveEvent && event.type == SDL_KEYDOWN) {
    switch (event.key.keysym.sym) {
    case SDLK_1:
      SDL_SetWindowFullscreen(window, SDL_WINDOW_FULLSCREEN);
      break;
    case SDLK_0:      
      SDL_SetWindowFullscreen(window, 0);
      break;
    }
  }
}

int main() {
  SDL_Init(SDL_INIT_VIDEO);
  window = SDL_CreateWindow(NULL, 0, 0, WIDTH, HEIGHT, SDL_WINDOW_SHOWN);
  renderer = SDL_CreateRenderer(window, -1, 0);
  emscripten_set_main_loop(mainloop, 0, 0);
  return 0;
}

(It was originally meant to be a tearing (vsync) test. The moving bar animation is useful here to ensure that the page has been properly scaled and to confirm that user code is running and updates are being seen.)

@kripken
Copy link
Member

kripken commented Mar 27, 2015

@DerKoun , you wrote that div-adding code. Do you know what should be done here?

@DerKoun
Copy link
Contributor

DerKoun commented Mar 27, 2015

Have to write from my phone. My PC broke and I don't have a replacement yet.

Having full screen applied to the parent of the canvas is (was?) the only way to ensure the canvas can have the correct aspect ratio in full screen in all browsers.
I recommend using this code from any method or API that enables full screen.

If being able to make the canvas itself full screen is a must in some scenarios the method has to be fixed:

  • check for any full screen element, not just canvas parent
  • mark the container div, eg with a class
  • if the canvas parent is not marked do not pull the canvas up

@juj
Copy link
Collaborator

juj commented Apr 1, 2015

I'm refactoring the src/library_html5.js so that the fullscreen button in the default shell.html (the Module.requestFullscreen function) could call directly into the HTML5 API and reuse the functionality there. This would allow removing the code that needs to create a div to the DOM for fullscreen handling.

@stale
Copy link

stale bot commented Aug 31, 2019

This issue has been automatically marked as stale because there has been no activity in the past 2 years. It will be closed automatically if no further activity occurs in the next 7 days. Feel free to re-open at any time if this issue is still relevant.

@stale stale bot added the wontfix label Aug 31, 2019
@stale stale bot closed this as completed Sep 7, 2019
@kripken
Copy link
Member

kripken commented Sep 17, 2019

It sounds like this is still an issue (see emscripten-ports/SDL2#8). Should we finish the refactoring @juj mentions earlier and make the button call emscripten_request_fullscreen in the HTML5 API, and remove the Browser.* fullscreen stuff?

cc @Daft-Freak

@guster32
Copy link

guster32 commented Sep 7, 2020

@juj I am trying to update Ogre3d to the latest version of emscripten It is currently using 1.38.24 .
It seems like we are hitting this issue but this issue seems rather old. Is still valid? Also I have been looking at the code to try and create a pull request. But I am having a hard time seeing how it all come together. Is there some docs u could point me to to educate myself this topic.
Thank you for your time any help on this would be appreciated.

@guster32
Copy link

guster32 commented Sep 7, 2020

@juj As u described... What we are seeing is that after going to Fullscreen by clicking on the button we end up with a canvas with a width and height = 0. In the code in ogre3d has a callback registered for emscripten_set_fullscreenchange_callback where it attempts to set the canvas to the desired dimensions with call to emscripten_set_canvas_element_size but it seems as if that code is executed before the size is set to 0 by some other javascript code. As a workaround I added this code on the same call back after the

EM_ASM(setTimeout(function(){var canvas = document.getElementById('canvas'); canvas.width = $0; canvas.height = $1;}, 0), width, height);

With the intent of executing the resize on the next javascript eventloop. This seems to work but it would be much better to complete the refactor.

paroj pushed a commit to OGRECave/ogre that referenced this issue Sep 8, 2020
@stale
Copy link

stale bot commented Sep 21, 2021

This issue has been automatically marked as stale because there has been no activity in the past year. It will be closed automatically if no further activity occurs in the next 30 days. Feel free to re-open at any time if this issue is still relevant.

@stale stale bot added the wontfix label Sep 21, 2021
@stale stale bot closed this as completed Apr 17, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

5 participants