optozorax / egui-macroquad

egui bindings for macroquad
https://optozorax.github.io/egui-macroquad/
Apache License 2.0
91 stars 75 forks source link

Fix miniquad scaling compatibility #21

Closed darthdeus closed 2 years ago

darthdeus commented 2 years ago

It seems egui-macroquad is not currently compatible with the latest egui-miniquad after https://github.com/not-fl3/egui-miniquad/pull/40 was merged. I'm basing this off of https://github.com/optozorax/egui-macroquad/pull/20 as that seems to be related to the original macroquad issue.

I'm not entirely sure if that is required, but the PR doesn't compile with miniquad after the fix.

This PR makes egui-macroquad work with latest egui-miniquad again, and based on my quick testing it seems UI scaling works with these changes.

Maybe a question for @buxx or @emilk, does the egui_mq_cfg still make sense after those changes?

I'm also not sure if mq::Context should be exposed up via egui_macroquad::ui(...), but since this causes existing code to fail to compile I thought it probably shouldn't? It's not a big change, but I don't know the reason why it was added in the first place, so it's hard for me to judge if it should be ignored at the level of egui-macroquad or passed up as a breaking change.

edit: Removed the egui_mq_cfg from #20 since it's not needed anymore

optozorax commented 2 years ago

Hi! Thank you. Could you please add usage of the new feature to the demo? Also, I think I would merge this PR after new version of egui-miniquad will be released. Just not to forget about it.

buxx commented 2 years ago

@darthdeus Hello, about egui_mq_cfg : It seems required to permit access to Miniquad::EguiMq.egui_input to be able to change miniquad used pixels_per_point like in following (imagine this code in program using egui_macroquad) :

    // Set egui scale
    egui_macroquad::egui_mq_cfg(|equi_mq| {
        equi_mq.egui_input().pixels_per_point = Some(2.0)
    });
darthdeus commented 2 years ago

Hi! Thank you. Could you please add usage of the new feature to the demo? Also, I think I would merge this PR after new version of egui-miniquad will be released. Just not to forget about it.

Added a slider to the demo that adjusts UI scaling of egui.

@darthdeus Hello, about egui_mq_cfg : It seems required to permit access to Miniquad::EguiMq.egui_input to be able to change miniquad used pixels_per_point like in following (imagine this code in program using egui_macroquad) :

    // Set egui scale
    egui_macroquad::egui_mq_cfg(|equi_mq| {
        equi_mq.egui_input().pixels_per_point = Some(2.0)
    });

I don't think this is necessary, since you don't need to access egui_input() / egui::InputState, you can just set this on egui::Context via ctx.set_pixels_per_point(2.0)

I've addded an example of this to the demo where it doesn't use egui_mq_cfg but just the egui::Context passed in to egui_macroquad::ui(|ctx| { ... ctx.set_poixels_per_point(...); })

buxx commented 2 years ago

I don't think this is necessary, since you don't need to access egui_input() / egui::InputState, you can just set this on egui::Context via ctx.set_pixels_per_point(2.0)

I've addded an example of this to the demo where it doesn't use egui_mq_cfg but just the egui::Context passed in to egui_macroquad::ui(|ctx| { ... ctx.set_poixels_per_point(...); })

Okay thanks :) I will use this way to set pixels in my own program.

optozorax commented 2 years ago

I'll be able to review this PR on the weekend, has a lot of work to do.