iceiix / stevenarella

Multi-protocol Minecraft-compatible client written in Rust
Apache License 2.0
1.45k stars 60 forks source link

Wrong GUI offsets on AMD Radeon R9 M295X #22

Closed iceiix closed 5 years ago

iceiix commented 5 years ago

On an old iMac (Retina 5K, 27-inch, Late 2014) with AMD Radeon R9 M295X, Steven shows the GUI elements at bad offsets. The screen when starting up, showing the server list:

screen shot 2018-11-17 at 9 00 23 pm

The user login screen is also offset in a similar location. The problem persists when resizing the window, but can be fixed by full-screening and un-fullscreening the window, then the elements are centered:

screen shot 2018-11-17 at 9 02 20 pm

The GUI scale also seems wrong (text too small), but is that a missing feature?

Notably this did NOT occur on a 2017 model iMac with an AMD Radeon Pro 580. The AMD Radeon R9 M295X also reproduces https://github.com/iceiix/steven/issues/5 invisible players, may or may not be related.

iceiix commented 5 years ago

Possibly related, on this system (AMD Radeon R9 M295X), Steven starts out completely black. Moving the window causes it to render, albeit with the offsetted GUI elements. Toggling fullscreen re-centers the elements but the scaling seems wrong, everything is too small and hard to read. Compare to the same size window (default size) on a Intel HD Graphics 4000 system:

screen shot 2018-11-18 at 4 32 54 pm

iceiix commented 5 years ago

This codepath is executed in src/render/mod.rs update_camera() when the window is moved:

        if self.height != height || self.width != width {
            self.width = width;
            self.height = height;
            gl::viewport(0, 0, width as i32, height as i32);

            self.perspective_matrix = cgmath::Matrix4::from(
                cgmath::PerspectiveFov {
                    fovy: cgmath::Rad::from(cgmath::Deg(90f32)),
                    aspect: (width as f32 / height as f32),
                    near: 0.1f32,
                    far: 500.0f32,
                }
            );

            self.init_trans(width, height);
        }

The size changes from 480x854 (the default) to 960x1708, when the window is moved even if not physically resizing the window, then it renders (but offset since that isn't the real size). 960/480 = 2, 1708/854 = 2, so it is receiving exactly a 2x scaling. Changing the scaling factor in display prefs does not affect these values, it is always 480x854 to 960x1708 on this system.

On the Intel Graphics 5000 system, this code path is executed on startup, changing from 0x0 to 480x854, and doesn't change when the window is moved. The AMD Radeon R9 M295X system also initializes to 0x0 then resizes to 480x854 on startup, but for some reason doesn't render until the second resizing.

iceiix commented 5 years ago

Do I need SDL_RenderSetScale?

This looks like a similar problem: https://stackoverflow.com/questions/47979639/how-is-it-possible-to-determine-the-correct-drawable-size-of-a-window-on-windows - says using 125% scaling on Windows 10, create a 800x600 window, but it is actually 1000x750 due to 125% scaling, though SDL_GL_GetDrawableSize returns 800x600. They added the SDL_WINDOW_ALLOW_HIGHDPI flag on window creation (not used in Steven).

https://github.com/grimfang4/sdl-gpu/pull/5 mentions both SDL_GL_GetDrawableSize and SDL_GetWindowSize to support HDPI mode:

I ran into a display issue when creating a SDL_Windows with SDL_WINDOW_ALLOW_HIGHDPI flag set on HDPI screens. Such windows have different drawable and window sizes, which results in incomplete window viewport (basically only 1/4 bottom left area is covered). A solution is to use https://wiki.libsdl.org/SDL_GL_GetDrawableSize for the dimensions and it worked out of the box for our users.

iceiix commented 5 years ago

https://docs.rs/sdl2/0.32.0-beta.2/sdl2/video/struct.Window.html window.drawable_size() changes from 854x480 to 960x1708 (2x) when moved, but window.size() stays fixed at 854x480. What allows drawable_size() to change? Source in rust-sdl2/video.rs shows these rust-sdl2 functions are merely wrappers around SDL2's SDL_GetWindowSize and SDL_GL_GetDrawableSize:

    pub fn size(&self) -> (u32, u32) {
        let mut w: c_int = 0;
        let mut h: c_int = 0;
        unsafe { sys::SDL_GetWindowSize(self.context.raw, &mut w, &mut h) };
        (w as u32, h as u32)
    }

    pub fn drawable_size(&self) -> (u32, u32) {
        let mut w: c_int = 0;
        let mut h: c_int = 0;
        unsafe { sys::SDL_GL_GetDrawableSize(self.context.raw, &mut w, &mut h) };
        (w as u32, h as u32)
    }

https://wiki.libsdl.org/SDL_GL_GetDrawableSize

This may differ from SDL_GetWindowSize() if we're rendering to a high-DPI drawable, i.e. the window was created with SDL_WINDOW_ALLOW_HIGHDPI on a platform with high-DPI support (Apple calls this "Retina"), and not disabled by the SDL_HINT_VIDEO_HIGHDPI_DISABLED hint.

iceiix commented 5 years ago

rust-sdl2's allow_highdpi() in WindowBuilder sets SDL_WINDOW_ALLOW_HIGHDPI:

    /// Creates the window in high-DPI mode if supported (>= SDL 2.0.1)
    pub fn allow_highdpi(&mut self) -> &mut WindowBuilder {
        self.window_flags |= sys::SDL_WindowFlags::SDL_WINDOW_ALLOW_HIGHDPI as u32;
        self
    }

Passing this when creating the main window fixes the bad GUI offsets on M295X:

screen shot 2018-11-19 at 7 01 30 am
diff --git a/src/main.rs b/src/main.rs
index d3a6fbe..9891100 100644
--- a/src/main.rs
+++ b/src/main.rs
@@ -169,6 +169,7 @@ fn main() {
     let sdl = sdl2::init().unwrap();
     let sdl_video = sdl.video().unwrap();
     let window = sdl2::video::WindowBuilder::new(&sdl_video, "Steven", 854, 480)
+                            .allow_highdpi()
                             .opengl()
                             .resizable()
                             .build()

but the scaling is still wrong (2x too small). Get the window size with size() then divide drawable_size() by it to get the scale (2x)? For SDL_RenderSetScale (RenderTarget set_scale)? It is also possible to disable HDPI with the SDL_HINT_VIDEO_HIGHDPI_DISABLED window hint.

iceiix commented 5 years ago

SDL2 also has SDL_RenderSetLogicalSize, but like SDL_RenderSetScale it appears to be only for the 2D canvas rendering target.

Would like to try SDL_HINT_VIDEO_HIGHDPI_DISABLED, if it can avoid special-casing on different systems. I set this hint with sdl2::hint::set_with_priority("SDL_VIDEO_HIGHDPI_DISABLED", "1", &sdl2::hint::Hint::Override); but still get 2x drawable size. Offset without allow_highdpi(). https://www.gamedev.net/forums/topic/678042-how-to-disable-hidpi-scaling-problems// reports "I did SDL_SetHint(SDL_HINT_VIDEO_HIGHDPI_DISABLED,"1"), it returns SDL_TRUE. Now, it seems to be working on some Win 8.1 machines, but not all (I have access to such machine so I'm sure it's not working)...", but has no solution. https://forums.libsdl.org/viewtopic.php?p=40484 points to setting NSHighResolutionCapable in Info.plist, but there isn't any info plist in this Rust executable (otool -P target/release/steven shows nothing).

At least the bad offset is fixed with allow_highdpi()...

iceiix commented 5 years ago

Removing allow_highdpi() and using size() instead of drawable_size() fixes both the offset and scale on the M295X:

screen shot 2018-11-19 at 7 36 50 am
iceiix commented 5 years ago

drawable_size() was intentionally used instead of size(), changed in https://github.com/thinkofname/steven/commit/715b0cb1eee5f6cb80b7ba2a5adc09003a82ef06 ... hope this is the right fix

iceiix commented 5 years ago
diff --git a/src/main.rs b/src/main.rs
index d3a6fbe..ae92073 100644
--- a/src/main.rs
+++ b/src/main.rs
@@ -169,11 +169,13 @@ fn main() {
     let sdl = sdl2::init().unwrap();
     let sdl_video = sdl.video().unwrap();
     let window = sdl2::video::WindowBuilder::new(&sdl_video, "Steven", 854, 480)
+                            //.allow_highdpi()
                             .opengl()
                             .resizable()
                             .build()
                             .expect("Could not create sdl window.");
     sdl2::hint::set_with_priority("SDL_MOUSE_RELATIVE_MODE_WARP", "1", &sdl2::hint::Hint::Override);
+    //sdl2::hint::set_with_priority("SDL_VIDEO_HIGHDPI_DISABLED", "1", &sdl2::hint::Hint::Override);
     let gl_attr = sdl_video.gl_attr();
     gl_attr.set_stencil_size(0);
     gl_attr.set_depth_size(24);
@@ -221,7 +223,7 @@ fn main() {
         let diff = now.duration_since(last_frame);
         last_frame = now;
         let delta = (diff.subsec_nanos() as f64) / frame_time;
-        let (width, height) = window.drawable_size();
+        let (width, height) = window.size();

         let version = {
             let mut res = game.resource_manager.write().unwrap();
iceiix commented 5 years ago

Committing only the revert, since it is a single commit, https://github.com/iceiix/steven/pull/27 dpi branch has the other commented-out changes. Maybe could implement DPI support later (but how important is it really for a low-res pixelated game?)

iceiix commented 5 years ago

Fixes 2/3 of the issues in https://github.com/iceiix/steven/issues/25