egui_vulkano icon indicating copy to clipboard operation
egui_vulkano copied to clipboard

fixed scaling issues

Open Wicpar opened this issue 3 years ago • 8 comments

fixes #8

Wicpar avatar Aug 08 '22 02:08 Wicpar

Hey, thanks for the PR! :)

I want to merge this, but it doesn't seem to be a complete fix yet?

According to https://github.com/emilk/egui/discussions/172, it should be possible to use Context::set_pixels_per_point to scale the UI (independently of the display's scale factor), but this doesn't work properly. With a scale factor of 2, only the top left quadrant of the display is drawn to. I think the fix would be to multiply window_size_points by egui_ctx.pixels_per_point() in Painter::draw, but I'm not sure if/how this interacts with high-DPI displays. What do you think?

derivator avatar Aug 14 '22 12:08 derivator

i didn't see that. Having looked at it now there are two disctinct variables, one in egui and another in egui winit there is a conflict. Egui-Winit creates a desync between the screen rect in logical pixels in take_egui_input as it has an internal ppp that desyncs with the context ppp.

also according to documentation:

pub fn set_pixels_per_point
... 
Note that this may be overwritten by input from the integration via RawInput::pixels_per_point. For instance, when using eframe on web, the browsers native zoom level will always be used.

the correct way to implement this would be to fix egui-winit by making it use the system scale factor directly and having a user set multiplier. This PR will be correct once the bug is fixed as the error stems from a desync between ppp and screen rect set by imgui winit in take_egui_input

Wicpar avatar Aug 14 '22 14:08 Wicpar

I now think the correct fix on top of your changes is to let egui tell us the scale factor:

let scale_factor = egui_ctx.pixels_per_point() as f64; // egui_winit sets this to the window's native scale factor
let size = surface.window().inner_size().to_logical(scale_factor);

By default this is the window's scale factor from winit, based on your display settings. egui_winit::State::handle_platform_output does keep track of modifications to the scale factor:

self.current_pixels_per_point = egui_ctx.pixels_per_point(); // someone can have changed it to scale the UI

The problem is that surface.window().scale_factor() doesn't know if a different value has been set inside egui.

derivator avatar Aug 14 '22 19:08 derivator

indeed it was that, my bad, i was wrong.

Wicpar avatar Aug 15 '22 05:08 Wicpar

also we may want to change the functions to do those calculations within the draw function to prevent bugs in user code. Maybe implement a trait that queries the window size if we don't want to depend on winit.

Wicpar avatar Aug 15 '22 05:08 Wicpar

Maybe implement a trait that queries the window size if we don't want to depend on winit.

Not sure what you mean exactly. I do want to avoid a dependency on winit, that should be handled by egui_winit. We could rename the parameter window_size_points to window_size_pixels in draw and document that you should pass in the physical size.

By the way, I would prefer to merge this separately from the update to vulkano 0.30, to keep a clean git history. You could probably do an interactive rebase or use cherry picking to untangle the commits. I could also do it myself if you prefer (keeping you as commit author, of course).

derivator avatar Aug 15 '22 11:08 derivator

if you could do it it would be nice so you can have it your way, i don't care for authorship.

Wicpar avatar Aug 15 '22 14:08 Wicpar

Ok, I will take care of it, thanks! :)

derivator avatar Aug 15 '22 14:08 derivator