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

Wrong GUI offsets on AMD Radeon R9 M295X #22

Closed
iceiix opened this issue Nov 18, 2018 · 11 comments
Closed

Wrong GUI offsets on AMD Radeon R9 M295X #22

iceiix opened this issue Nov 18, 2018 · 11 comments

Comments

@iceiix
Copy link
Owner

iceiix commented Nov 18, 2018

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
Copy link
Owner Author

iceiix commented Nov 19, 2018

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 iceiix changed the title Wrong GUI offsets Wrong GUI offsets on AMD Radeon R9 M295X Nov 19, 2018
@iceiix
Copy link
Owner Author

iceiix commented Nov 19, 2018

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
Copy link
Owner Author

iceiix commented Nov 19, 2018

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).

grimfang4/sdl-gpu#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
Copy link
Owner Author

iceiix commented Nov 19, 2018

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
Copy link
Owner Author

iceiix commented Nov 19, 2018

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
Copy link
Owner Author

iceiix commented Nov 19, 2018

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
Copy link
Owner Author

iceiix commented Nov 19, 2018

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
Copy link
Owner Author

iceiix commented Nov 19, 2018

drawable_size() was intentionally used instead of size(), changed in Thinkofname/steven@715b0cb ... hope this is the right fix

iceiix referenced this issue in iceiix/steven Nov 19, 2018
…aling/offset

Closes https://github.com/iceiix/steven/issues/22 Wrong GUI offsets on AMD Radeon R9 M295X

This reverts commit 715b0cb.
@iceiix
Copy link
Owner Author

iceiix commented Nov 19, 2018

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
Copy link
Owner Author

iceiix commented Nov 19, 2018

Committing only the revert, since it is a single commit, iceiix/steven#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
Copy link
Owner Author

iceiix commented Nov 19, 2018

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

iceiix referenced this issue in iceiix/steven Nov 30, 2018
* Add glutin dependency

* Create a glutin window

* Use the glutin window, basics work

* Store DPI factor on game object, update on Resized

* Use physical size for rendering only. Fixes UI scaled too small

Fixes #35 (comment)
See also https://github.com/iceiix/steven/issues/22

* Begin adding mouse input events

* Listen for DeviceEvents

* Call hover_at on mouse motion

* Listen for CursorMoved window event, hovering works

Glutin has separate WindowEvent::CursorMoved and
DeviceEvent::MouseMotion events, for absolute cursor and relative mouse
motion, respectively, instead of SDL's Event::MouseMotion for both.

* Use tuple pattern matching instead of nested if for MouseInput

* Implement left clicking

* Use grab_cursor() to capture the cursor

* Hide the cursor when grabbing

* Implement MouseWheel event

* Listen for keyboard input, escape key release

* Keyboard input: console toggling, glutin calls backquote 'grave'

* Implement fullscreen in glutin, updates #31

* Update settings for glutin VirtualKeyCode

* Keyboard controls (note: must clear conf.cfg to use correct bindings)

* Move DeviceEvent match arm up higher for clarity

* Remove SDL

* Pass physical dimensions to renderer tick so blit_framebuffer can use full size but the ui is still sized logically.

* Listen for DeviceEvent::Text

* Implement text input using ReceivedCharacter window event, works

#35 (comment)

* Request specific version of OpenGL, version 3.2

* Request OpenGL 3.2 but fallback to OpenGL ES 2.0 if available (not tested)

* Set core profile and depth 24-bits, stencil 0-bits

* Allow changing vsync, but require restarting (until rust-windowing/glutin#693)

* Clarify specific Rust version requirement

* Import glutin::* in handle_window_event() to avoid overly repetitive code

* Linux in VM fix: manually calculate delta in MouseMotion

For the third issue on #35 (comment)
https://github.com/tomaka/glutin/issues/1084 MouseMotion event returns absolute instead of relative values, when running Linux in a VM

* Heuristic to detect absolute/relative MouseMotion from delta:(xrel, yrel); use a higher scaling factor

* Add clipboard pasting with clipboard crate instead of sdl2

#35 (comment)
@iceiix iceiix transferred this issue from iceiix/steven Jan 12, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant