emilk / egui

egui: an easy-to-use immediate mode GUI in Rust that runs on both web and native
https://www.egui.rs/
Apache License 2.0
20.61k stars 1.49k forks source link

Web canvas resize loop #4699

Open alexkirsz opened 4 days ago

alexkirsz commented 4 days ago

Describe the bug https://github.com/emilk/egui/pull/4536 introduced a ResizeObserver-based resizing for the canvas. Testing it with rerun, when the size of the canvas isn't explicitly bounded via CSS, the canvas quickly enters a resize loop where:

  1. A first resize entry is fired after the first call to .observe. In the callback, the canvas is resized to the measured device pixel size. For instance, if the device pixel ratio is 2 and the canvas size hasn't been explicitly set via CSS, the canvas will be resized from 300x150 (logical pixels) to 600x300 (physical pixels).
  2. The resize causes a new resize entry to be fired. This time, the canvas will be resized from 600x300 to 1200x600.
  3. etc.
  4. This quickly causes rerun/egui to crash from trying to instantiate too large a texture.
emilk commented 4 days ago

Seems like there is some place where we confuse units (logical vs physical pixels).

@jprochazk can you take a look?

jprochazk commented 4 days ago

Can you provide a repro, e.g. what your HTML/CSS was when embedding rerun in a page? egui doesn't resize the canvas.

Also note that the ResizeObserver changes haven't even been released yet, so make sure you're testing with the latest main of rerun.

alexkirsz commented 4 days ago

Can you provide a repro, e.g. what your HTML/CSS was when embedding rerun in a page?

Sure! Is there a working codesandbox template I could fork to try to repro the issue there? I currently have a custom setup with a local build of the rerun viewer running in a Next.js app, so it would be hard to extract, but I think I should be able to repro it in isolation.

egui doesn't resize the canvas.

These two lines are where the problem stems from in eframe.

jprochazk commented 4 days ago

These two lines are where the problem stems from in eframe.

That doesn't resize the canvas display size, which is the problem you're experiencing. It changes the number of pixels in the canvas drawing buffer. Here's a good writeup about the difference: https://stackoverflow.com/a/43364730

alexkirsz commented 4 days ago

In the absence of any CSS explicitly setting the size of the canvas, the width and height attributes of the canvas will also affect its display size: https://codesandbox.io/s/musing-mendeleev-342tww

jprochazk commented 4 days ago

Right, so that's a problem, because:

Is there a reason why you can't set width/height style on the canvas in your setup? I don't think there's anything we can do about this in egui/eframe. Settings those attributes is a requirement to make egui's rendering work.

alexkirsz commented 4 days ago

The original reason was that rerun's WebViewer API doesn't provide direct control over the underlying canvas element and I had not styled it properly. This behavior from eframe was surprising to me and I think it might make sense to add a circuit breaker here to at least print out an error when this loop is encountered. With proper styling, this isn't an issue.

jprochazk commented 4 days ago

I agree that it is surprising. We'll have to think about how best to detect this, and document that users are expected to set the canvas size through CSS.

As for the rerun viewer, just today we merged a PR that exposes the underlying canvas as a property on the WebViewer. You can access it that way if you update to latest main, so something like viewer.canvas.classList.add("custom-canvas-style") will work. We're committed to making sure that the only element the web viewer appends to the provided parent element is the canvas, so selectors like .your-parent-element > canvas should always work as well.

emilk commented 2 days ago

How do we document this better? In web_demo/index.html maybe, and link to that from the eframe docs?