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

Flash of white when changing scaling on Windows (root cause included) #2570

Closed
fgimian opened this issue Nov 26, 2022 · 3 comments · Fixed by #2571
Closed

Flash of white when changing scaling on Windows (root cause included) #2570

fgimian opened this issue Nov 26, 2022 · 3 comments · Fixed by #2571
Labels
B - bug Dang, that shouldn't have happened DS - windows

Comments

@fgimian
Copy link
Contributor

fgimian commented Nov 26, 2022

Hey there folks, hope you're doing well. I noticed a little burst of white every time I change display scaling on Windows and I'm painting a dark background.

Here's a sample application below (adapted from the egui glow example):

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

use std::{thread, time::Duration};

use glow::{Context, HasContext as _};
use glutin::{
    event::{Event, StartCause, WindowEvent},
    event_loop::{ControlFlow, EventLoop, EventLoopBuilder},
    ContextBuilder, PossiblyCurrent, WindowedContext,
};

fn main() {
    let clear_color = [0.1, 0.1, 0.1];

    let event_loop: EventLoop<()> = EventLoopBuilder::with_user_event().build();

    let window_builder = glutin::window::WindowBuilder::new()
        .with_title("egui_glow example")
        .with_visible(false); // Keep hidden until we've painted something. See https://github.com/emilk/egui/pull/2279

    let gl_window: WindowedContext<PossiblyCurrent> = unsafe {
        ContextBuilder::new()
            .with_depth_buffer(0)
            .with_stencil_buffer(0)
            .with_vsync(true)
            .build_windowed(window_builder, &event_loop)
            .unwrap()
            .make_current()
            .unwrap()
    };

    let gl = unsafe { Context::from_loader_function(|s| gl_window.get_proc_address(s)) };

    event_loop.run(move |event, _, control_flow| {
        let redraw = || {
            thread::sleep(Duration::from_millis(250));

            unsafe {
                gl.clear_color(clear_color[0], clear_color[1], clear_color[2], 1.0);
                gl.clear(glow::COLOR_BUFFER_BIT);
            }

            gl_window.swap_buffers().unwrap();
            gl_window.window().set_visible(true);
        };

        match event {
            // Platform-dependent event handlers to workaround a winit bug
            // See: https://github.com/rust-windowing/winit/issues/987
            // See: https://github.com/rust-windowing/winit/issues/1619
            Event::RedrawEventsCleared if cfg!(windows) => redraw(),
            Event::RedrawRequested(_) if !cfg!(windows) => redraw(),

            Event::WindowEvent { event, .. } => {
                if matches!(event, WindowEvent::CloseRequested | WindowEvent::Destroyed) {
                    println!("exiting");
                    *control_flow = ControlFlow::Exit;
                }

                if let WindowEvent::Resized(physical_size) = &event {
                    println!("resize");
                    gl_window.resize(*physical_size);
                } else if let WindowEvent::ScaleFactorChanged { new_inner_size, .. } = &event {
                    println!("scaling factor changed");
                    gl_window.resize(**new_inner_size);
                }
            }
            Event::NewEvents(StartCause::ResumeTimeReached { .. }) => {
                println!("resume time reached");
                gl_window.window().request_redraw();
            }

            _ => (),
        }
    });
}

I've intentionally added a small sleep during the redraws so it's more obvious.

And here's my Cargo.toml with a local copy of winit which is checked out on the v0.27.5 tag:

[package]
name = "egui_glow"
version = "0.19.0"
edition = "2021"

[dependencies]
glutin = "0.29.0"
glow = "0.11"

[patch.crates-io]
winit = { path = "../winit" }

When running the application, attempt to change the display scaling while viewing the window and you'll see a small burst of white each time you change scaling.

I've traced this back to the PR #2419 and more specifically the choice to set hbrBackground to COLOR_WINDOWFRAME when setting up the window using WNDCLASSEXW.

Reverting this to an empty HBRUSH of 0 as was the case prior to the PR resolves this for me:

diff --git a/src/platform_impl/windows/window.rs b/src/platform_impl/windows/window.rs
index 63e41dff..ed6cda59 100644
--- a/src/platform_impl/windows/window.rs
+++ b/src/platform_impl/windows/window.rs
@@ -1026,7 +1026,6 @@ unsafe fn register_window_class<T: 'static>(
         .map(|icon| icon.inner.as_raw_handle())
         .unwrap_or(0);

-    use windows_sys::Win32::UI::WindowsAndMessaging::COLOR_WINDOWFRAME;
     let class = WNDCLASSEXW {
         cbSize: mem::size_of::<WNDCLASSEXW>() as u32,
         style: CS_HREDRAW | CS_VREDRAW,
@@ -1036,7 +1035,7 @@ unsafe fn register_window_class<T: 'static>(
         hInstance: util::get_instance_handle(),
         hIcon: h_icon,
         hCursor: 0, // must be null in order for cursor state to work properly
-        hbrBackground: COLOR_WINDOWFRAME as _,
+        hbrBackground: 0,
         lpszMenuName: ptr::null(),
         lpszClassName: class_name.as_ptr(),
         hIconSm: h_icon_small,

I'd be more than happy to submit a PR to revert this but just wanted to open it up for discussion as there may have been some rationale in setting this background which should be evaluated.

ccing @msiglreith 😄

Thanks heaps in advance!
Fotis

@fgimian
Copy link
Contributor Author

fgimian commented Nov 26, 2022

Just a little further note that hbrBackground should be set to the required color + 1 as per the docs:

A color value must be one of the following standard system colors (the value 1 must be added to the chosen color).

So the effective color being used here is actually COLOR_WINDOW (pub const COLOR_WINDOW: SYS_COLOR_INDEX = 5u32;) since COLOR_WINDOW + 1 is 6 which is equivalent to the costant used pub const COLOR_WINDOWFRAME: SYS_COLOR_INDEX = 6u32;.

@msiglreith
Copy link
Member

Hello, thanks for reporting and investigating!
I could reproduce it with your OpenGL application but not with my own Vulkan GUI app interestingly.
Removing the hbrBackground is fine for me, doesn't really seem to do much - iirc I set it somewhere when testing different approaches and just kept it due to having a default was nice to have,

@fgimian
Copy link
Contributor Author

fgimian commented Nov 26, 2022

Hello, thanks for reporting and investigating! I could reproduce it with your OpenGL application but not with my own Vulkan GUI app interestingly. Removing the hbrBackground is fine for me, doesn't really seem to do much - iirc I set it somewhere when testing different approaches and just kept it due to having a default was nice to have,

Thanks so much for the reply and help. I've just submitted PR #2571 to revert this behaviour. 😄

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
B - bug Dang, that shouldn't have happened DS - windows
Development

Successfully merging a pull request may close this issue.

2 participants