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

Can't scale canvas with CSS without it changing the canvas Width and Height properties #22847

Closed
michaelfiber opened this issue Nov 4, 2024 · 32 comments

Comments

@michaelfiber
Copy link

Version of emscripten/emsdk:

emcc (Emscripten gcc/clang-like replacement + linker emulating GNU ld) 3.1.70 (b53978ee3f540dc74761eba127aa7f1b8761a125)
clang version 20.0.0git (https:/github.com/llvm/llvm-project f52b89561f2d929c0c6f37fd818229fbcad3b26c)
Target: wasm32-unknown-emscripten
Thread model: posix
InstalledDir: /home/michael/dev/raylib-test/emsdk/emsdk-main/upstream/bin

Starting with version 3.1.51 the width and height properties of a canvas as overridden if you use CSS to change the size of the canvas. How do we maintain the original way of doing it where you can use CSS to change the canvas.style.width and canvas.style.height without it overriding the canvas.width and canvas.height?

There is an issue open on the raylib project related to this: raysan5/raylib#4464 - but it doesn't seem like an issue with raylib itself.

The goal is to continue to allow emscripten programs compiled with GLFW=3 to be scaled up to fill a containing element with object-fit: cover or something similar so that you could, for instance, use CSS to allow an itch.io game to go fullscreen while the game itself remains a fixed resolution.

@sbc100
Copy link
Collaborator

sbc100 commented Nov 5, 2024

Can you confirm this was broken between 3.1.50 and 3.1.51? Could you perhaps bisect to see exactly which emscripten change broke this: https://emscripten.org/docs/contributing/developers_guide.html#bisecting

@sbc100
Copy link
Collaborator

sbc100 commented Nov 5, 2024

I see only 3 changes to the glfw between 3.1.50 and 3.1.51 so I suppose the culprit is likely one these:

$ git l 3.1.50..3.1.51  src/library_glfw.js
05821a3ed Leverage coalescing operators. NFC (#20531) 11 months ago [Ingvar Stepanyan]
c26fc388e Add Hi DPI Support for GLFW (#20584) 11 months ago [Yan Pujante]
95591bcfb Use `let` for loop variable instead of IIFEs. NFC (#20823) 11 months ago [Sam Clegg]
7d639f7a6 Fix glfw3 default hints being modified (#20770) 11 months ago [Yan Pujante]
b3b0f2794 Fix touch input handling for glfw. (#20805) 11 months ago [Ruben]

My guess is that it would be #20584. @ypujante WDYT?

@curiousdannii
Copy link
Contributor

I had a similar issue and had to overwrite emscripten_set_canvas_element_size to fix things in my project, though I'm not using (I think) GLFW, only SDL2. It doesn't look like it's been changed recently though.

@deathbeam
Copy link

I was testing this as well and yes the issue is between 3.1.50 and 3.1.51 (not reproducable on 3.1.50 and lower, reproducable after 3.1.51). I havent tried specific commits, was just using emsdk and checkouting tags + using emsdk install/activate for tags. When debugging in raylib disc we suspect that the issue is indeed #20584, specifically this section:

https://github.com/emscripten-core/emscripten/pull/20584/files#diff-efefa0c433583bf964817ede50dac6afac9e3ff5686e3af838993fb5f1717117R1244

@ypujante
Copy link
Contributor

ypujante commented Nov 5, 2024

The changes in PR #20584 were made to allow support for 4k/Hi DPI. The code in library_glfw.js that @deathbeam links to is the following:

      const wNativeScaled = Math.floor(wNative * scale);
      const hNativeScaled = Math.floor(hNative * scale);
      if (canvas.width  != wNativeScaled) canvas.width  = wNativeScaled;
      if (canvas.height != hNativeScaled) canvas.height = hNativeScaled;
      if (typeof canvas.style != 'undefined') {
        if (wNativeScaled != wNative || hNativeScaled != hNative) {
          canvas.style.setProperty( "width", wNative + "px", "important");
          canvas.style.setProperty("height", hNative + "px", "important");
        } else {
          canvas.style.removeProperty( "width");
          canvas.style.removeProperty("height");
        }
      }

and it overrides the code in library_browser.js which is:

        if (canvas.width  != wNative) canvas.width  = wNative;
        if (canvas.height != hNative) canvas.height = hNative;
        if (typeof canvas.style != 'undefined') {
          if (w != wNative || h != hNative) {
            canvas.style.setProperty( "width", w + "px", "important");
            canvas.style.setProperty("height", h + "px", "important");
          } else {
            canvas.style.removeProperty( "width");
            canvas.style.removeProperty("height");
          }
        }

and which was used prior to this PR.

As you can see, if the scale is 1.0, then the 2 code paths are identical.

So my question is, are you trying to make raylib work with 4k/Hi DPI?

@deathbeam
Copy link

deathbeam commented Nov 5, 2024

HDPI is disabled, as mentioned in the main post and linked issue, the issue is that it breaks when you set for example width: 100% css style on canvas.emscripten to scale it and the canvas dimensions end up being incorrect, and this worked before 3.1.51. Example of styling:

      canvas.emscripten {
        border: 0px none;
        background: black;
        width: 100%;
      }

@ypujante
Copy link
Contributor

ypujante commented Nov 5, 2024

@deathbeam can you provide a working minimal example so that I can try to reproduce the issue, ideally an example using glfw directly and not raylib?

Thank you

@deathbeam
Copy link

deathbeam commented Nov 5, 2024

@deathbeam can you provide a working minimal example so that I can try to reproduce the issue, ideally an example using glfw directly and not raylib?

Thank you

Sadly only thing I can provide is reproduction step for raylib, maybe @michaelfiber or someone else could provide glfw one, but here is raylib one anyway just in case:

emsdk setup:

git clone https://github.com/emscripten-core/emsdk
cd emsdk
git checkout 3.1.51
./emsdk install 3.1.51 && ./emsdk activate 3.1.51
source emsdk_env.sh
git clone https://github.com/raysan5/raylib
cd raylib/src
make PLATFORM=PLATFORM_WEB -B
cd ../examples
# Because of change in 3.1.51 add -sGL_ENABLE_GET_PROC_ADDRESS on line 275 in Makefile.web
# This is not required on newer releases so i guess it was made no required sometime after
# This also fails on one of the examples but the testing one should build before that
make PLATFORM=PLATFORM_WEB -B
emrun --port 8080 core/core_2d_camera_mouse_zoom.html

the shell being used in raylib example is in src/shell.html, the testing example is in examples/core/core_2d_camera_mouse_zoom.c.

If you retry this same steps on 3.1.50 the canvas is properly scaled to full screen width, on 3.1.51 and later its stuck in bottom-left corner at first and also input is not matching the canvas either. Works fine when width: 100% is removed from the shell.html (line 82) but with undesired behaviour (scaling no longer possible).

@ypujante
Copy link
Contributor

ypujante commented Nov 5, 2024

Given my prior experience with raylib, I must admit I am not super motivated in helping debugging raylib.

I will gladly take a look at the issue you refer to (which might be raylib not using GLFW properly) if somebody provides me with a self contained, pure GLFW minimal example.

For your convenience, here is a very minimal GFLW piece of cpp code that I have written (adapted from an example for my emscripten-glfw library):

#include <emscripten/emscripten.h>
#include <emscripten/html5.h>
#include <GLFW/GLFW3.h>
#include <cstdio>
#include <cmath>

void consoleErrorHandler(int iErrorCode, char const *iErrorMessage)
{
  printf("glfwError: %d | %s\n", iErrorCode, iErrorMessage);
}

//! jsRenderFrame: for the sake of this example, uses the canvas2D api to change the color of the screen / display a message
EM_JS(void, jsRenderFrame, (GLFWwindow *glfwWindow, int w, int h, int fw, int fh, double mx, double my, int color, bool isFullscreen), {
  const ctx = Module.canvas.getContext('2d');
  ctx.fillStyle = `rgb(${color}, ${color}, ${color})`;
  ctx.fillRect(0, 0, fw, fh); // using framebuffer width/height
  const text = `${w}x${h} | ${mx}x${my}`;
  ctx.font = '15px monospace';
  ctx.fillStyle = `rgb(${255 - color}, 0, 0)`;
  ctx.fillText(text, 10 + color, 20 + color);
})

//! Called for each frame
void renderFrame(GLFWwindow *iWindow)
{
  static int frameCount = 0;

  // poll events
  glfwPollEvents();

  int w,h; glfwGetWindowSize(iWindow, &w, &h);
  int fw,fh; glfwGetFramebufferSize(iWindow, &fw, &fh);
  double mx,my; glfwGetCursorPos(iWindow, &mx, &my);
  auto color = 127.0f + 127.0f * std::sin((float) frameCount++ / 120.f);
  jsRenderFrame(iWindow, w, h, fw, fh, mx, my, (int) color, false);
}

//! The main loop (called by emscripten for each frame)
void main_loop(void *iUserData)
{
  if(auto window = reinterpret_cast<GLFWwindow *>(iUserData); !glfwWindowShouldClose(window))
  {
    // not done => renderFrame
    renderFrame(window);
  }
  else
  {
    // done => terminating
    glfwTerminate();
    emscripten_cancel_main_loop();
  }
}

//! main
int main()
{
  // set a callback for errors otherwise if there is a problem, we won't know
  glfwSetErrorCallback(consoleErrorHandler);

  // print the version on the console
  printf("%s\n", glfwGetVersionString());

  // initialize the library
  if(!glfwInit())
    return -1;

  // no OpenGL (use canvas2D)
  glfwWindowHint(GLFW_CLIENT_API, GLFW_NO_API);

  // create the only window
  auto window = glfwCreateWindow(600, 400, "Emscripten #22847", nullptr, nullptr);
  if(!window)
    return -1;

  // tell emscripten to use "main_loop" as the main loop (window is user data)
  emscripten_set_main_loop_arg(main_loop, window, 0, GLFW_FALSE);
}

What I need is the shell.html file (and/or changes to the cpp code) that would demonstrate the issue and what your expectations are.

@raysan5
Copy link

raysan5 commented Nov 5, 2024

Hi! I created two videos illustrating the issue:

Environment:

  • MSI Laptop, with a 1920x1080 display
  • Windows 10 64bit (22H2)
  • Google Chrome Version 130.0.6723.92 (Official Build) (64-bit)
  • raylib 5.5-dev (master-branch)

Example compiled with emsdk 3.1.50 (expected behaviour)

Framebuffer is 800x450, canvas has CSS property width: 100%. The canvas fit the 100% of browser width space when browser resized but the canvas framebuffer it's 800x450 and the input is reported considering that framebuffer size, independent of the browser window size.

canvas_scale100_emsdk3.1.50.mp4

Example compiled with emsdk 3.1.51

First time the page is loaded, the canvas is scaled to 100% the browser size but the framebuffer (800x450) is placed in the lower-left corner of the canvas. Mouse position is relative to canvas position/size, not framebuffer. When browser is resized, the canvas is proportionally resized, including the contained framebuffer and also its relative mouse input.

canvas_scale100_emsdk3.1.51.mp4

NOTE 1: If width:100% is disabled for the canvas, the canvas (and its content) does not resize with browser, the framebuffer get just cut.

NOTE 2: It has been tested on a 1920x1080 monitor (no High-DPI) and the HighDPI flag is not enabled on GLFW.

NOTE 3: All web platform code is contained in raylib/src/platforms/platform_web.c.

Please, let me know if you need further information.

@ypujante
Copy link
Contributor

ypujante commented Nov 5, 2024

@raysan5 Thank you for posting the videos / showing the problem you are seeing with raylib.

As I mentioned, personally, I need a reproducible example that is not relying on raylib and using GLFW directly. I already gave you 99% of the code...

If somebody else from the Emscripten team wants to debug raylib, of course they can do that, but I won't.

@raysan5
Copy link

raysan5 commented Nov 5, 2024

@ypujante I don't understand your adversion to raylib project, but please note that this is a breaking change that happened after 10 years of a expected working behaviour and affects thousands of raylib users, potentially way more users than plain GLFW-on-web users. Not to mention the hundreds of raylib web projects that you can find for example on itchio.

I think it deserves some consideration.

@michaelfiber
Copy link
Author

Given my prior experience with raylib, I must admit I am not super motivated in helping debugging raylib.

I will gladly take a look at the issue you refer to (which might be raylib not using GLFW properly) if somebody provides me with a self contained, pure GLFW minimal example.

For your convenience, here is a very minimal GFLW piece of cpp code that I have written (adapted from an example for my emscripten-glfw library):

#include <emscripten/emscripten.h>
#include <emscripten/html5.h>
#include <GLFW/GLFW3.h>
#include <cstdio>
#include <cmath>

void consoleErrorHandler(int iErrorCode, char const *iErrorMessage)
{
  printf("glfwError: %d | %s\n", iErrorCode, iErrorMessage);
}

//! jsRenderFrame: for the sake of this example, uses the canvas2D api to change the color of the screen / display a message
EM_JS(void, jsRenderFrame, (GLFWwindow *glfwWindow, int w, int h, int fw, int fh, double mx, double my, int color, bool isFullscreen), {
  const ctx = Module.canvas.getContext('2d');
  ctx.fillStyle = `rgb(${color}, ${color}, ${color})`;
  ctx.fillRect(0, 0, fw, fh); // using framebuffer width/height
  const text = `${w}x${h} | ${mx}x${my}`;
  ctx.font = '15px monospace';
  ctx.fillStyle = `rgb(${255 - color}, 0, 0)`;
  ctx.fillText(text, 10 + color, 20 + color);
})

//! Called for each frame
void renderFrame(GLFWwindow *iWindow)
{
  static int frameCount = 0;

  // poll events
  glfwPollEvents();

  int w,h; glfwGetWindowSize(iWindow, &w, &h);
  int fw,fh; glfwGetFramebufferSize(iWindow, &fw, &fh);
  double mx,my; glfwGetCursorPos(iWindow, &mx, &my);
  auto color = 127.0f + 127.0f * std::sin((float) frameCount++ / 120.f);
  jsRenderFrame(iWindow, w, h, fw, fh, mx, my, (int) color, false);
}

//! The main loop (called by emscripten for each frame)
void main_loop(void *iUserData)
{
  if(auto window = reinterpret_cast<GLFWwindow *>(iUserData); !glfwWindowShouldClose(window))
  {
    // not done => renderFrame
    renderFrame(window);
  }
  else
  {
    // done => terminating
    glfwTerminate();
    emscripten_cancel_main_loop();
  }
}

//! main
int main()
{
  // set a callback for errors otherwise if there is a problem, we won't know
  glfwSetErrorCallback(consoleErrorHandler);

  // print the version on the console
  printf("%s\n", glfwGetVersionString());

  // initialize the library
  if(!glfwInit())
    return -1;

  // no OpenGL (use canvas2D)
  glfwWindowHint(GLFW_CLIENT_API, GLFW_NO_API);

  // create the only window
  auto window = glfwCreateWindow(600, 400, "Emscripten #22847", nullptr, nullptr);
  if(!window)
    return -1;

  // tell emscripten to use "main_loop" as the main loop (window is user data)
  emscripten_set_main_loop_arg(main_loop, window, 0, GLFW_FALSE);
}

What I need is the shell.html file (and/or changes to the cpp code) that would demonstrate the issue and what your expectations are.

The issue at hand is demonstrated by this code. Thanks for providing it.

How would you adjust this code so that the result can be resized using CSS without changing the framebuffer dimensions of the program?

@ypujante
Copy link
Contributor

ypujante commented Nov 5, 2024

@michaelfiber

The issue at hand is demonstrated by this code. Thanks for providing it.

Can you provide me with the shell.html file you use that demonstrates the issue? Then I should be able to answer your question. Thank you.

@michaelfiber
Copy link
Author

michaelfiber commented Nov 5, 2024

@ypujante sure thing

<!doctype html>
<html lang="EN-us">
  <head>
    <meta charset="utf-8">
    <meta http-equiv="Content-Type" content="text/html; charset=utf-8">

    <title>hello raylib wasm</title>
    <style>
        body { margin: 0px; }
        canvas.emscripten { border: 0px none; background-color: black; width: 100vw;}
        #print { display: none; }
    </style>
    </head>
    <body>
        <canvas class=emscripten id=canvas oncontextmenu=event.preventDefault() tabindex=-1></canvas>
        <p id="output" />
        <script>
            var Module = {
                canvas: (function() {
                    var canvas = document.getElementById('canvas');
                    return canvas;
                })()
            };
        </script>
        {{{ SCRIPT }}}
    </body>
</html>

@ypujante
Copy link
Contributor

ypujante commented Nov 5, 2024

@michaelfiber As I explain in my own project emscripten-glfw, the trick is to use a different HTML element to handle CSS resizing and have the code propagate the changes to the canvas.

Mixing and matching the 2 is impossible to manage especially when you go Hi DPI.

Here is how I adjusted the html:

<!doctype html>
<html lang="EN-us">
  <head>
    <meta charset="utf-8">
    <meta http-equiv="Content-Type" content="text/html; charset=utf-8">

    <title>hello raylib wasm</title>
    <style>
        body { margin: 0px; }
        #canvas-container { width: 100vw; height: calc(100vh - 1em);}
        canvas.emscripten { border: 0px none; background-color: black; }
        #print { display: none; }
    </style>
    </head>
    <body>
        <div id="canvas-container"><canvas class=emscripten id=canvas oncontextmenu=event.preventDefault() tabindex=-1></canvas></div>
        <p id="output" />
        <script>
            var Module = {
                canvas: (function() {
                    var canvas = document.getElementById('canvas');
                    return canvas;
                })()
            };
        </script>
        {{{ SCRIPT }}}
    </body>
</html>

and the cpp code:

#include <emscripten/emscripten.h>
#include <emscripten/html5.h>
#include <GLFW/GLFW3.h>
#include <cstdio>
#include <cmath>

/**
 * The purpose of this example is to demonstrate how to make the canvas resizable with another container (a
 * surrounding div) driving its size. The container width is proportional to the size of the window and so as the
 * window gets resized so does the div and so does the canvas. */

//! Display error message in the Console
void consoleErrorHandler(int iErrorCode, char const *iErrorMessage)
{
  printf("glfwError: %d | %s\n", iErrorCode, iErrorMessage);
}

//! jsRenderFrame: for the sake of this example, uses the canvas2D api to change the color of the screen / display a message
EM_JS(void, jsRenderFrame, (GLFWwindow *glfwWindow, int w, int h, int fw, int fh, double mx, double my, int color, bool isFullscreen), {
  const ctx = Module.canvas.getContext('2d');
  ctx.fillStyle = `rgb(${color}, ${color}, ${color})`;
  ctx.fillRect(0, 0, fw, fh); // using framebuffer width/height
  const text = `${w}x${h} | ${mx}x${my}`;
  ctx.font = '15px monospace';
  ctx.fillStyle = `rgb(${255 - color}, 0, 0)`;
  ctx.fillText(text, 10 + color, 20 + color);
})

//! Called for each frame
void renderFrame(GLFWwindow *iWindow)
{
  static int frameCount = 0;

  // poll events
  glfwPollEvents();

  int w,h; glfwGetWindowSize(iWindow, &w, &h);
  int fw,fh; glfwGetFramebufferSize(iWindow, &fw, &fh);
  double mx,my; glfwGetCursorPos(iWindow, &mx, &my);
  auto color = 127.0f + 127.0f * std::sin((float) frameCount++ / 120.f);
  jsRenderFrame(iWindow, w, h, fw, fh, mx, my, (int) color, false);
}

//! The main loop (called by emscripten for each frame)
void main_loop(void *iUserData)
{
  if(auto window = reinterpret_cast<GLFWwindow *>(iUserData); !glfwWindowShouldClose(window))
  {
    // not done => renderFrame
    renderFrame(window);
  }
  else
  {
    // done => terminating
    glfwTerminate();
    emscripten_cancel_main_loop();
  }
}

EM_BOOL EmscriptenResizeCallback(int eventType, const EmscriptenUiEvent *event, void *iUserData)
{
  auto window = reinterpret_cast<GLFWwindow *>(iUserData);
  double canvasWidth = 0;
  double canvasHeight = 0;
  emscripten_get_element_css_size("#canvas-container", &canvasWidth, &canvasHeight);
  glfwSetWindowSize(window, static_cast<int>(canvasWidth), static_cast<int>(canvasHeight));
  return true;
}

//! main
int main()
{
  // set a callback for errors otherwise if there is a problem, we won't know
  glfwSetErrorCallback(consoleErrorHandler);

  // print the version on the console
  printf("%s\n", glfwGetVersionString());

  // initialize the library
  if(!glfwInit())
    return -1;

  // no OpenGL (use canvas2D)
  glfwWindowHint(GLFW_CLIENT_API, GLFW_NO_API);

  // create the only window
  auto window = glfwCreateWindow(600, 400, "Resizable Container | emscripten-glfw", nullptr, nullptr);
  if(!window)
    return -1;

  emscripten_set_resize_callback(EMSCRIPTEN_EVENT_TARGET_WINDOW, window, true, EmscriptenResizeCallback);
  EmscriptenResizeCallback(0, nullptr, window);

  // tell emscripten to use "main_loop" as the main loop (window is user data)
  emscripten_set_main_loop_arg(main_loop, window, 0, GLFW_FALSE);
}

Can you give it a try and confirm that this is the behavior you are after?

@ypujante
Copy link
Contributor

ypujante commented Nov 5, 2024

This live example is showing a similar behavior using emscripten-glfw, but the concepts are the same...

@raysan5
Copy link

raysan5 commented Nov 5, 2024

Sincerely, I don't think that adding that kind of extra friction and complexity for the user is the proper solution. Considering that previous implementation was working well.

How does SDL manage it? Does it also require a different HTML element to handle CSS resizing? Probably worth checking it...

@ypujante
Copy link
Contributor

ypujante commented Nov 5, 2024

@michaelfiber I am trying to figure out the delta with 3.1.50. In the example that you gave me that you said was broken, isn't doing this end up behaving like the old way?

  // create the only window
  auto window = glfwCreateWindow(600, 400, "Emscripten #22847", nullptr, nullptr);
  if(!window)
    return -1;
  glfwSetWindowSize(window, 600, 400); // add this line to your code...

I am not saying that is the solution I am proposing, I am trying to identify the delta and I am requiring help besides comments like "I think it deserves some consideration" and "extra friction and complexity"...

@michaelfiber
Copy link
Author

  glfwSetWindowSize(window, 600, 400); // add this line to your code...

With this added the framebuffer stays set to the original size but mouse coordinates are now broken. The original functionality of emscripten you could scale with CSS and still rely on mouse positions being correct. Halfway across the 600px wide canvas would report an x coord of 300. With that change halfway across the 600px wide canvas reports half the window width instead.

@ypujante
Copy link
Contributor

ypujante commented Nov 5, 2024

@michaelfiber Thanks for checking. Then I am not sure what is the right solution, since @raysan5 believes what I suggest is unacceptable.

For the record, ImGui, a far bigger project than raylib, uses Emscripten/GLFW without this kind of issue. I understand that the behavior has changed between the 2 versions. But I don't understand why it doesn't affect a project like ImGui and it does raylib. Note that ImGui uses a similar (unacceptable) solution I suggested.

There are shenanigans in the raylib code, like this one completely bypassing the GLFW api to change the size of the canvas... so when you have the library trying to do something and the calling code doing something else, then that is definitely a recipe for disaster down the line. Note that I did try to help improve the raylib code on multiple occasions and was rejected every single time.

I do understand that the behavior has changed but it was a necessary change to support Hi DPI / 4K. It is possible to revert this change entirely and remove support for Hi DPI / 4K from Emscripten/GLFW to restore the prior behavior.

At this time I have no clue how to change the Emscripten code so that it behaves the way raylib wants it to behave without affecting other libraries and projects. That would definitely require a lot of investigation and research and I do not have such a bandwidth at the moment.

That being said, I will gladly help review PR to this project if somebody wants to take a crack at it.

I guess I will leave it to @sbc100 to make a decision.

@sbc100
Copy link
Collaborator

sbc100 commented Nov 5, 2024

It sounds like making the old and new behaviour both work is not feasible?

@ypujante since imgui now uses your new GLFW port do you know of any other projects that depend on HiDPI support in the builtin implementation? Perhaps reverting could make sense if it doesn't break anyone else?

On the other hand it does also seem like the code in raylib could be updated in the longer run: https://github.com/raysan5/raylib/blob/b47fffb48bfeb6a73060c5fb8847d8db2879110c/src/platforms/rcore_web.c#L1659. Could we revert for now, then update raylib, then reland the change perhaps?

@sbc100
Copy link
Collaborator

sbc100 commented Nov 5, 2024

Note that I did try to help improve the raylib code on multiple occasions and was rejected every single time.

Can you say more about this? Did you specifically try to fix the code at https://github.com/raysan5/raylib/blob/b47fffb48bfeb6a73060c5fb8847d8db2879110c/src/platforms/rcore_web.c#L1659? Do you have a link to the PR?

@sbc100
Copy link
Collaborator

sbc100 commented Nov 5, 2024

@ypujante is there some way to request automatic resizing in GLFW that doesn't requre each project to write their own EmscriptenResizeCallback like this:

EM_BOOL EmscriptenResizeCallback(int eventType, const EmscriptenUiEvent *event, void *iUserData)
{
  auto window = reinterpret_cast<GLFWwindow *>(iUserData);
  double canvasWidth = 0;
  double canvasHeight = 0;
  emscripten_get_element_css_size("#canvas-container", &canvasWidth, &canvasHeight);
  glfwSetWindowSize(window, static_cast<int>(canvasWidth), static_cast<int>(canvasHeight));
  return true;
}

@ypujante
Copy link
Contributor

ypujante commented Nov 5, 2024

@sbc100

It sounds like making the old and new behaviour both work is not feasible?

I am not saying it is not feasible. I am saying it doesn't seem to be as trivial as I was hoping it would be. It does require more investigative work and I don't have the bandwidth at the moment (I already spent most of my morning on this...). I might have it in the future. But if somebody else wants to take a crack at it, I can probably take the time to review a PR that would implement it.


since imgui now uses your new GLFW port do you know of any other projects that depend on HiDPI support in the builtin implementation? Perhaps reverting could make sense if it doesn't break anyone else?

It is very hard to say. ImGui actually offers the choice between -sUSE_GLFW=3 and --use-port=contrib.glfw so users can decide to use whichever they want. Since turning on HiDPI support is very simply and easy (even with -sUSE_GLFW=3) it is impossible to gauge how many people are doing it. Note that SDL2 has HiDPI support as well.


On the other hand it does also seem like the code in raylib could be updated in the longer run: https://github.com/raysan5/raylib/blob/b47fffb48bfeb6a73060c5fb8847d8db2879110c/src/platforms/rcore_web.c#L1659. Could we revert for now, then update raylib, then reland the change perhaps?

When I was pointing out this example, I am not saying this is what causes the issue with raylib. I was just pointing out the fact that what raylib is doing right now is certainly iffy at best and certain to cause problems in the long run as they are messing up with the size of the canvas outside the library.


Note that I did try to help improve the raylib code on multiple occasions and was rejected every single time.

Can you say more about this? Did you specifically try to fix the code at https://github.com/raysan5/raylib/blob/b47fffb48bfeb6a73060c5fb8847d8db2879110c/src/platforms/rcore_web.c#L1659? Do you have a link to the PR?

I created a raylib issue to address the HiDPI support lacking in raylib. You can see it is a long thread spanning a couple months (which if you are interested describe the reason why I stopped working for the raylib project and my earlier comment "Given my prior experience with raylib, I must admit I am not super motivated in helping debugging raylib"). This was accompanied with the following PR. This PR was trying to address the fact that raylib is not using the framebuffer size at all which is a mistake when using the GLFW API (ImGui does it right). In the process I had indeed removed the iffy statement I mentioned earlier.


is there some way to request automatic resizing in GLFW that doesn't requre each project to write their own EmscriptenResizeCallback like this:

EM_BOOL EmscriptenResizeCallback(int eventType, const EmscriptenUiEvent *event, void *iUserData)
{
  auto window = reinterpret_cast<GLFWwindow *>(iUserData);
  double canvasWidth = 0;
  double canvasHeight = 0;
  emscripten_get_element_css_size("#canvas-container", &canvasWidth, &canvasHeight);
  glfwSetWindowSize(window, static_cast<int>(canvasWidth), static_cast<int>(canvasHeight));
  return true;
}

If you look at emscripten-glfw I actually created a convenient API that takes care of it (it is not a standard GLFW API since GLFW does not have this concept). The user simply needs to add this to their code:

emscripten_glfw_make_canvas_resizable(window, "#canvas-container");

and the implementation essentially setup the proper listener as described above. Note that you can use the string "window" as the container to make the canvas occupies the full browser window as a convenience (so no need to change the HTML at all in this instance). We can certainly add something similar (even with the same name) to -sUSE_GLFW=3 but unclear if that would be an appropriate solution for raylib...

@michaelfiber
Copy link
Author

This is getting severely off topic so I'm going to step away until I can spend some time fixing what's broken myself.

@ypujante You have a personal beef with raylib, that is obvious. If I have time to fix the issue you introduced to emscripten myself I will now that you've helped confirm it exists and that it has nothing to do with raylib. It would make more sense to revert it at this point since this was a major change to basic functionality introduced, seemingly accidentally, in a patch release. That is not how anybody would expect this software to progress. But what's happened is done and I'd rather focus on fixing things that are problematic than whatever this discussion has devolved into.

@sbc100
Copy link
Collaborator

sbc100 commented Nov 6, 2024

@michaelfiber I would say that this is not necessarily an emscripten issue, it could well be an issue with the way raylib is using GLFW combined with low level emscripten APIs. I think we should investigate and consider fixes in both raylib and emscripten. I imagine its certainly possible to use the emscripten APIs and direct web APIs in ways that conflict the GLFW usage so we should keep an open mind here.

@ypujante
Copy link
Contributor

ypujante commented Nov 6, 2024

@sbc100 trying to set the negativity aside, I believe that the issue described is about zooming the canvas (for example, the size of the canvas is set to 600x400, but it is rendered with a css property that streches it = zooms it (either in or out)).

For what it's worth, there is nothing in the GLFW API (and certainly not for the desktop official implementation) that is about zooming a window: you can make the window bigger or smaller but the size of the underlying GLFW window follows. There is never a case (desktop) where the window gets bigger but the GLFW size remains the same: if you want to implement zooming, you need to do it yourself.

That being said, it seems that the implementation prior to my changes was (I suppose accidentally) resulting in this behavior. I guess I was not aware of this undocumented behavior because it's not part of the behavior I am familiar with when dealing with GLFW and I certainly never tested for this use case.

In other words, the solution I suggested, whether hard-coded or via something like emscripten_glfw_make_canvas_resizable would not result in what raylib wants, since it's about resizing the canvas not about zooming it.

In the end, it is somewhat messing with the canvas size outside the GLFW library which is why it is not trivial to deal with and is IMO brittle. But since that behavior was the "de facto" standard (whether by accident or not) prior to my changes, I suppose it is worth looking into it further (for the non Hi DPI case only)

@ypujante
Copy link
Contributor

ypujante commented Nov 7, 2024

@sbc100 @michaelfiber FYI, I have a few spare cycles this morning, so I am going to take a quick look to see if I can come up with a quick way to solve the issue.

@ypujante
Copy link
Contributor

ypujante commented Nov 7, 2024

For the record, and a way to document what I am doing, this is how to compile only the single example demonstrating the issue (compiling all examples takes a long time and currently fails with raylib trunk a617e1e217fa3d78e1d3d4e7d0bc4266c88a6dd4):

make -f Makefile.Web core/core_2d_camera_mouse_zoom

@michaelfiber
Copy link
Author

michaelfiber commented Nov 9, 2024

@michaelfiber I would say that this is not necessarily an emscripten issue, it could well be an issue with the way raylib is using GLFW combined with low level emscripten APIs. I think we should investigate and consider fixes in both raylib and emscripten. I imagine its certainly possible to use the emscripten APIs and direct web APIs in ways that conflict the GLFW usage so we should keep an open mind here.

I think we have to accept that GLFW is not aware of the layer we are describing here and that is part of the problem with how the functionality has changed. If you want a GLFW based wasm project to be aware of what CSS is doing to the canvas you have to form some hefty opinions on how to implement the flow of information into the window to enable that. The old way seemed to not have as many opinions but the new way is more focused on doing things one way at the expense of other ways. So which is the preferred approach for the emscripten project?

Should the opinion be built into emscripten that CSS manipulation of the canvas is automatically propagated to the GLFW context regardless of what the developer wants by default? Or should it be the other way where if you have an opinion on how the GLFW context should react to how CSS is changing the canvas, you implement it yourself? The old way would be the latter and that makes the most sense to me personally since we don't know how people are leveraging CSS with their emscripten projects.

Just because that makes the most sense to me doesn't me I'm going to die on that hill or anything. If the opinionated approach fits better with the overall emscripten project then that should be the approach that is used. I'd leave that up to people more familiar with the project to decide.

Edit: And I'll hold off on suggesting any changes since the way we proceed is really a question of what the philosophy of the project is.

@ypujante
Copy link
Contributor

ypujante commented Nov 9, 2024

I have restored the behavior you wanted. See #22900.

@sbc100 sbc100 closed this as completed in 5203d08 Nov 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants