egui icon indicating copy to clipboard operation
egui copied to clipboard

ImageButton does not use y value of style.spacing.button_padding

Open JakeRabinowitz opened this issue 7 months ago • 2 comments

Describe the bug ImageButton only uses the x value of ui.style().spacing.button_padding when drawn. In practice this means that ImageButtons cannot have independent padding on X / Y, and the Y value doesn't impact the layout of the frame.

Link to code: egui/src/widgets/button.rs

// egui/src/widgets/button.rs

impl<'a> Widget for ImageButton<'a> {
    fn ui(self, ui: &mut Ui) -> Response {
        let padding = if self.frame {
            // so we can see that it is a button:
            Vec2::splat(ui.spacing().button_padding.x)    // <--- this line
        } else {
            Vec2::ZERO
        };
    ...

To Reproduce Steps to reproduce the behavior:

  1. Change the ui.style_mut().spacing.button_padding to a Vec2 with different X & Y values.
  2. Create an ImageButton.
  3. Your ImageButton's padding will only use the value in X.

Expected behavior ImageButton padding's Y value can be set independently of its X value.

Screenshots image A = ui.style_mut().spacing.button_padding = egui::Vec2::new(2.0, 2.0); B = ui.style_mut().spacing.button_padding = egui::Vec2::new(8.0, 2.0); C = ui.style_mut().spacing.button_padding = egui::Vec2::new(2.0, 8.0); D = ui.style_mut().spacing.button_padding = egui::Vec2::new(2.0, 2.0);

Desktop (please complete the following information):

  • OS: Windows 10
  • Platform: Bevy, using bevy_egui 0.23.0/0.24.0. Also tested on pure egui off master as of 18 Jan 2024

Additional context The fix seems as simple as changing this line:

Vec2::splat(ui.spacing().button_padding.x)

to

ui.spacing().button_padding

But I don't know if there was a specific reason it uses splat now. Looks like this change was made a few years ago.

JakeRabinowitz avatar Jan 05 '24 17:01 JakeRabinowitz

You should try bevy_egui v0.24.0

rustbasic avatar Jan 18 '24 05:01 rustbasic

Sorry, forgot to mention I've tried it with bevy_egui 0.24.0 and egui 0.25.0 as well.

Here's a version that demonstrates the issue using pure egui (no bevy included), using a modified version of the images example:

#![cfg_attr(not(debug_assertions), windows_subsystem = "windows")] // hide console window on Windows in release

use eframe::egui::{self, ImageButton};

fn main() -> Result<(), eframe::Error> {
    env_logger::init(); // Log to stderr (if you run with `RUST_LOG=debug`).
    let options = eframe::NativeOptions {
        viewport: egui::ViewportBuilder::default().with_inner_size([600.0, 600.0]),
        ..Default::default()
    };
    eframe::run_native(
        "Image Viewer",
        options,
        Box::new(|cc| {
            // This gives us image support:
            egui_extras::install_image_loaders(&cc.egui_ctx);
            Box::<MyApp>::default()
        }),
    )
}

#[derive(Default)]
struct MyApp {}

impl eframe::App for MyApp {
    fn update(&mut self, ctx: &egui::Context, _frame: &mut eframe::Frame) {
        egui::CentralPanel::default().show(ctx, |ui| {
            // x = 100, y = 10
            ui.style_mut().spacing.button_padding = egui::Vec2::new(100.0, 10.0);
            ui.add(ImageButton::new(
                egui::Image::new(egui::include_image!("close_16.png"))
                    .max_size(egui::Vec2::splat(16.0)),
            ));

            // x = 10, y = 100
            ui.style_mut().spacing.button_padding = egui::Vec2::new(10.0, 100.0);
            ui.add(ImageButton::new(
                egui::Image::new(egui::include_image!("close_16.png"))
                    .max_size(egui::Vec2::splat(16.0)),
            ));
        });
    }
}

And the output: image

JakeRabinowitz avatar Jan 18 '24 21:01 JakeRabinowitz