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

XCB fixes #825

Merged
merged 8 commits into from Mar 29, 2015

Conversation

Projects
None yet
5 participants
@binary1248
Member

binary1248 commented Mar 14, 2015

Supersedes #780, #665 and #425.

  • Replaced Xlib event names by XCB equivalents.
  • Fixed mouse grabbing in fullscreen mode. Removed keyboard grabbing to allow the user to "alt+tab" out of the window.
  • Added EWMH support when a compliant WM is present. If a compliant WM is present, it will be in charge of setting the fullscreen state, forwarding any system key combinations (alt+whatever), and making sure the application stays responsive (it will prompt the user to kill the process if necessary)
  • If EWMH support is unavailable, the old method of setting fullscreen and removing decorations is used, in this case, system key combinations need to be explicitly handled by the user since OVERRIDE_REDIRECT is necessary
  • In both scenarios, a hard video mode switch is performed before the window enters the fullscreen state
  • Added support for handling rotated display configurations

This PR should fix

  • #274 - If EWMH is present, alt+f4 will always result in sf::Event::Closed regardless of window style
  • #771 - Rotated screen configurations are enumerated and accepted during window creation
  • #847 - Modified non-alphabetic keys return the unmodified sf::Keyboard values
@eXpl0it3r

This comment has been minimized.

Show comment
Hide comment
@eXpl0it3r

eXpl0it3r Mar 16, 2015

Member

Can some people on Linux test this? @danharibo, @Congelli501, @SuperV1234, @dabbertorres, @Robert42, @h0tw1r3 or @Mischa-Alff :bowtie:

Member

eXpl0it3r commented Mar 16, 2015

Can some people on Linux test this? @danharibo, @Congelli501, @SuperV1234, @dabbertorres, @Robert42, @h0tw1r3 or @Mischa-Alff :bowtie:

@TankOs

This comment has been minimized.

Show comment
Hide comment
@TankOs

TankOs Mar 17, 2015

Member
  1. When resolution is different than the requested one, it changes the resolution but does not show the window.
  2. After exiting the program, the original resolution isn't restored.
  3. When resolution is the same as the requested one (e.g. by just running it the 2nd time), the window is shown.

AwesomeWM, Arch Linux, Nvidia with original driver.

Member

TankOs commented Mar 17, 2015

  1. When resolution is different than the requested one, it changes the resolution but does not show the window.
  2. After exiting the program, the original resolution isn't restored.
  3. When resolution is the same as the requested one (e.g. by just running it the 2nd time), the window is shown.

AwesomeWM, Arch Linux, Nvidia with original driver.

@binary1248

This comment has been minimized.

Show comment
Hide comment
@binary1248

binary1248 Mar 18, 2015

Member

@TankOs Should be fixed now. Tested myself on AwesomeWM, Arch Linux, AMD with free driver.

Member

binary1248 commented Mar 18, 2015

@TankOs Should be fixed now. Tested myself on AwesomeWM, Arch Linux, AMD with free driver.

@Marukyu

This comment has been minimized.

Show comment
Hide comment
@Marukyu

Marukyu Mar 18, 2015

Using XFWM4 (EWMH supported and correctly detected as such by SFML), Debian sid (unstable), Nvidia 343.22 drivers.
Monitor setup: TwinView 1920x1080 (left, primary, "DFP-2") + 1280x1024 (right, secondary, "CRT-1").

  1. Alt-Tab does not work at all for me, though Alt-F4 does. Disabling pointer grabbing by making WindowImplX11::setPointerGrabbed a no-op function causes it to work. Perhaps automatic pointer grabbing in fullscreen could be disabled and made optional, if #614 ends up being implemented.
  2. The window shows up correctly for me in every case, both after a successful resolution change and a resolution change failure.
  3. The resolution does not reset after closing the window. It does correctly reset when recreating a fullscreen window with the native resolution.
  4. Resolution changing behaves incorrectly when using multiple monitors (ignores primary monitor setting; disables all other screens), though this is already known. However, this PR provides an (unintentional?) workaround: using the default desktop mode with sf::Style::Fullscreen creates a fullscreen window that fills only the screen the mouse pointer is on, even though the desktop mode refers to the virtual screen, which spans both monitors (3200x1080). This is performed by the WM, and is the desired behavior with some multiple monitor setups (such as mine), though in some other cases the expected behavior (stretching the window over both screens) may be preferred.

Marukyu commented Mar 18, 2015

Using XFWM4 (EWMH supported and correctly detected as such by SFML), Debian sid (unstable), Nvidia 343.22 drivers.
Monitor setup: TwinView 1920x1080 (left, primary, "DFP-2") + 1280x1024 (right, secondary, "CRT-1").

  1. Alt-Tab does not work at all for me, though Alt-F4 does. Disabling pointer grabbing by making WindowImplX11::setPointerGrabbed a no-op function causes it to work. Perhaps automatic pointer grabbing in fullscreen could be disabled and made optional, if #614 ends up being implemented.
  2. The window shows up correctly for me in every case, both after a successful resolution change and a resolution change failure.
  3. The resolution does not reset after closing the window. It does correctly reset when recreating a fullscreen window with the native resolution.
  4. Resolution changing behaves incorrectly when using multiple monitors (ignores primary monitor setting; disables all other screens), though this is already known. However, this PR provides an (unintentional?) workaround: using the default desktop mode with sf::Style::Fullscreen creates a fullscreen window that fills only the screen the mouse pointer is on, even though the desktop mode refers to the virtual screen, which spans both monitors (3200x1080). This is performed by the WM, and is the desired behavior with some multiple monitor setups (such as mine), though in some other cases the expected behavior (stretching the window over both screens) may be preferred.
@binary1248

This comment has been minimized.

Show comment
Hide comment
@binary1248

binary1248 Mar 18, 2015

Member

Man... I just pushed a fixed commit. 😄

Mind testing again? 😛

Oh and by the way, SFML doesn't support multi-monitor setups yet. It is true that this patch lets the WM handle fullscreen switching, so that might be enough for your case, but bear in mind that although workarounds might exist, until something is fully intentional and part of the documented behaviour, I wouldn't rely on it.

Member

binary1248 commented Mar 18, 2015

Man... I just pushed a fixed commit. 😄

Mind testing again? 😛

Oh and by the way, SFML doesn't support multi-monitor setups yet. It is true that this patch lets the WM handle fullscreen switching, so that might be enough for your case, but bear in mind that although workarounds might exist, until something is fully intentional and part of the documented behaviour, I wouldn't rely on it.

@Marukyu

This comment has been minimized.

Show comment
Hide comment
@Marukyu

Marukyu Mar 18, 2015

I'm probably missing something or doing things wrong (pretty new to git, sorry!), but it seems like bugfixes/altf4_new, the branch I compiled from earlier, already had commit e9907a0 included since 4 days, if that's the one you were referring to.

Marukyu commented Mar 18, 2015

I'm probably missing something or doing things wrong (pretty new to git, sorry!), but it seems like bugfixes/altf4_new, the branch I compiled from earlier, already had commit e9907a0 included since 4 days, if that's the one you were referring to.

@binary1248

This comment has been minimized.

Show comment
Hide comment
@binary1248

binary1248 Mar 18, 2015

Member

No... If I amend a commit, the authorship date won't change. To be extra sure, you can checkout the master branch and re-checkout this one. It should check the new version out in that case.

Member

binary1248 commented Mar 18, 2015

No... If I amend a commit, the authorship date won't change. To be extra sure, you can checkout the master branch and re-checkout this one. It should check the new version out in that case.

@Marukyu

This comment has been minimized.

Show comment
Hide comment
@Marukyu

Marukyu Mar 18, 2015

Ah, thank you for the hint! I got it working now.

I repeated all the previous tests, and I am getting the same results as before. It appears as if some window managers (such as XFWM) simply do not allow Alt-Tabbing out of a window that grabs the mouse pointer. Other WM key combinations do work, such as Alt-F11 to toggle fullscreen.

Marukyu commented Mar 18, 2015

Ah, thank you for the hint! I got it working now.

I repeated all the previous tests, and I am getting the same results as before. It appears as if some window managers (such as XFWM) simply do not allow Alt-Tabbing out of a window that grabs the mouse pointer. Other WM key combinations do work, such as Alt-F11 to toggle fullscreen.

@binary1248

This comment has been minimized.

Show comment
Hide comment
@binary1248

binary1248 Mar 18, 2015

Member

That's strange... No matter which WM I try, all of them let me alt-tab out of a fullscreen SFML window. Can you check if it's really not alt-tabbing or if it is, but the SFML window is basically "stuck on top" of all other windows, leading you to think that nothing happened? You can see if it lost focus by pressing keys that are supposed to do something in the SFML application.

Member

binary1248 commented Mar 18, 2015

That's strange... No matter which WM I try, all of them let me alt-tab out of a fullscreen SFML window. Can you check if it's really not alt-tabbing or if it is, but the SFML window is basically "stuck on top" of all other windows, leading you to think that nothing happened? You can see if it lost focus by pressing keys that are supposed to do something in the SFML application.

@Marukyu

This comment has been minimized.

Show comment
Hide comment
@Marukyu

Marukyu Mar 18, 2015

Turns out it was a misbehavior of XFWM itself, I checked the source code and it was very recently fixed. The XFWM version in the Debian unstable repository will hopefully receive the fix soon.
Sorry for having wasted your time with this, and keep up the great work!

Edit: Compiling XFWM4 from the latest source makes alt-tabbing work correctly. The mouse pointer is properly ungrabbed when unfocusing and re-grabbed when re-focusing.

Marukyu commented Mar 18, 2015

Turns out it was a misbehavior of XFWM itself, I checked the source code and it was very recently fixed. The XFWM version in the Debian unstable repository will hopefully receive the fix soon.
Sorry for having wasted your time with this, and keep up the great work!

Edit: Compiling XFWM4 from the latest source makes alt-tabbing work correctly. The mouse pointer is properly ungrabbed when unfocusing and re-grabbed when re-focusing.

@dabbertorres

This comment has been minimized.

Show comment
Hide comment
@dabbertorres

dabbertorres Mar 19, 2015

Contributor

Seems to be working correctly here.

Arch Linux x86_64
Cinnamon DE (Muffin WM) 2.4.6
Nvidia Proprietary 346.47
5760x1080 (triple 1920x1080)

Even though SFML doesn't officially support multiple monitors, I noticed behavior similar to @Marukyu 's #4.
Creating a fullscreen window with the size of one of the monitors (1920x1080) creates it at the left-most monitor (0, 0), and disables the other two monitors.
However, creating a fullscreen window with the size of the X screen, or current desktop mode (5760x1080), creates a window of size 1920x1080 on the left-most monitor (0, 0), BUT leaves the other two monitors enabled. I can then alt-tab and move the SFML window around with my WM. This is my preferred behavior.

Anyways seems to works great!

Contributor

dabbertorres commented Mar 19, 2015

Seems to be working correctly here.

Arch Linux x86_64
Cinnamon DE (Muffin WM) 2.4.6
Nvidia Proprietary 346.47
5760x1080 (triple 1920x1080)

Even though SFML doesn't officially support multiple monitors, I noticed behavior similar to @Marukyu 's #4.
Creating a fullscreen window with the size of one of the monitors (1920x1080) creates it at the left-most monitor (0, 0), and disables the other two monitors.
However, creating a fullscreen window with the size of the X screen, or current desktop mode (5760x1080), creates a window of size 1920x1080 on the left-most monitor (0, 0), BUT leaves the other two monitors enabled. I can then alt-tab and move the SFML window around with my WM. This is my preferred behavior.

Anyways seems to works great!

@binary1248

This comment has been minimized.

Show comment
Hide comment
@binary1248

binary1248 Mar 25, 2015

Member

New commit pushed. It should fix fullscreen not being set on window managers like awesome. I also got rid of the automatic pointer grabbing in fullscreen since it will be taken care of separately in #614.

@TankOs, @Marukyu, @dabbertorres Feel free to test the new version for any bugs.

Member

binary1248 commented Mar 25, 2015

New commit pushed. It should fix fullscreen not being set on window managers like awesome. I also got rid of the automatic pointer grabbing in fullscreen since it will be taken care of separately in #614.

@TankOs, @Marukyu, @dabbertorres Feel free to test the new version for any bugs.

@binary1248

This comment has been minimized.

Show comment
Hide comment
@binary1248

binary1248 Mar 26, 2015

Member

Pushed commits that fixed a small memory leak and fixed #843.

Member

binary1248 commented Mar 26, 2015

Pushed commits that fixed a small memory leak and fixed #843.

@eXpl0it3r

This comment has been minimized.

Show comment
Hide comment
@eXpl0it3r

eXpl0it3r Mar 28, 2015

Member

This PR has been added to my merge list, meaning it will be merged soon, unless someone raises any concerns.

Member

eXpl0it3r commented Mar 28, 2015

This PR has been added to my merge list, meaning it will be merged soon, unless someone raises any concerns.

TankOs and others added some commits Jan 9, 2015

Adjusted WindowImplX11.
* Replaced Xlib event names by XCB equivalents.
* Removed XCB_CW_OVERRIDE_REDIRECT in order to let the WM handle mapping
  the window to the full screen.
* Fixed mouse grabbing in fullscreen mode. Removed keyboard grabbing to
  allow the user to "alt+tab" out of the window.
* Completely revised fullscreen handling: The screen's resolution is not
  changed at all anymore. Instead the WM is asked for going fullscreen
  and the view is scaled.
Reverted back to hard switching the fullscreen video mode when necess…
…ary on Unix systems, added support for automatically choosing between EWMH fullscreen setting when supported and manual fullscreen setting as the fallback, added support for window responsiveness checks.
Fixed RandR extension not being loaded causing Unix screen mode switc…
…hing to fail, added several more error checks to RandR operations, added support for rotated resolutions on Unix (#771).

binary1248 added some commits Mar 25, 2015

Fixed numerous bugs/undefined behavior in the XCB implementation, add…
…ed a lot more XCB error handling and reporting, make use of xcb-ewmh to handle EWMH for us, refactored some code out of the Window constructor into their own methods, fixed fullscreen state transition not working on window managers that create temporary parent windows when the window is being mapped, removed automatic fullscreen pointer grabbing since that is the subject of #614 and might not be desired in some situations.
Add ScopedXcbPtr to CMakeLists.txt, replaced xcb_query_extension with…
… xcb_get_extension_data where possible, removed decorations from fullscreen windows, fixed DRI2 events not being forwarded as Xlib events leading to Mesa not functioning correctly in certain situations.
Replaced a few Xlib keyboard handling functions with XCB keyboard han…
…dling functions, fixed modified key events returning sf::Keyboard::Unknown on Unix (#847), fixed sf::Keyboard::Quote and sf::Keyboard::Tilde events not functioning properly on Unix, optimized keycode lookup when using sf::Keyboard::isKeyPressed() on Unix.

@eXpl0it3r eXpl0it3r merged commit 7287b77 into master Mar 29, 2015

@eXpl0it3r eXpl0it3r deleted the bugfix/altf4_new branch Mar 29, 2015

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment