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

Fix for Glfw and Wayland crashing #392

Closed
wants to merge 2 commits into from

Conversation

@costashatz
Copy link
Contributor

costashatz commented Nov 13, 2019

Fix for Glfw and Wayland crashing when showing the window after being hidden..

@codecov-io

This comment has been minimized.

Copy link

codecov-io commented Nov 13, 2019

Codecov Report

Merging #392 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #392   +/-   ##
=======================================
  Coverage   72.55%   72.55%           
=======================================
  Files         354      354           
  Lines       18854    18854           
=======================================
  Hits        13679    13679           
  Misses       5175     5175

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 98232f3...37d41d0. Read the comment docs.

@mosra mosra added this to the 2019.1c milestone Nov 13, 2019
@mosra mosra added this to TODO in Platforms via automation Nov 13, 2019
@mosra

This comment has been minimized.

Copy link
Owner

mosra commented Nov 13, 2019

Argh! I just realized this would introduce a regression when you specify WindowFlag::Hidden in the configuration. Is it possible for you to test this patch on Wayland on top of the above (and WindowFlag::Hidden passed in the config)? The window should briefly show and then stay hidden if all is good.

diff --git a/src/Magnum/Platform/GlfwApplication.cpp b/src/Magnum/Platform/GlfwApplication.cpp
index f4be6fd75..4b40495b5 100644
--- a/src/Magnum/Platform/GlfwApplication.cpp
+++ b/src/Magnum/Platform/GlfwApplication.cpp
@@ -407,8 +407,8 @@ bool GlfwApplication::tryCreate(const Configuration& configuration, const GLConf
        If we are on Wayland, this is causing a segfault; a blinking window is
        acceptable in this case. */
     constexpr const char waylandString[] = "wayland";
-    if(std::strncmp(std::getenv("XDG_SESSION_TYPE"), waylandString, sizeof(waylandString)) != 0)
-        glfwWindowHint(GLFW_VISIBLE, false);
+    const bool onWayland = std::strncmp(std::getenv("XDG_SESSION_TYPE"), waylandString, sizeof(waylandString)) != 0;
+    if(!onWayland) glfwWindowHint(GLFW_VISIBLE, false);
     else if(_verboseLog)
         Warning{} << "Platform::GlfwApplication: Wayland detected, GL context has to be created with the window visible and may cause flicker on startup";
     if((_window = glfwCreateWindow(scaledWindowSize.x(), scaledWindowSize.y(), configuration.title().c_str(), monitor, nullptr)))
@@ -487,9 +487,13 @@ bool GlfwApplication::tryCreate(const Configuration& configuration, const GLConf
         _window = nullptr;
     }
 
-    /* Show the window once we are sure that everything is okay */
+    /* Show the window once we are sure that everything is okay (and it's not
+       requested to be hidden). If we didn't hide it initially due to applying
+       the Wayland workaround and it's requested to be, hide it now. */
     if(!(configuration.windowFlags() & Configuration::WindowFlag::Hidden))
         glfwShowWindow(_window);
+    else if(onWayland)
+        glfwHideWindow(_window);
 
     /* Return true if the initialization succeeds */
     return true;

Thanks a lot!

@costashatz

This comment has been minimized.

Copy link
Contributor Author

costashatz commented Nov 13, 2019

Argh! I just realized this would introduce a regression when you specify WindowFlag::Hidden in the configuration. Is it possible for you to test this patch on Wayland on top of the above (and WindowFlag::Hidden passed in the config)? The window should briefly show and then stay hidden if all is good.

If the visibility is set to hidden (i.e., glfwWindowHint(GLFW_VISIBLE, false);) and we never call glfwShowWindow() then we do not get a segfault. I just verified that if I select WindowFlag::Hidden without any additional change in the code (this PR as is), it runs with no problem (of course no window appears).

@mosra

This comment has been minimized.

Copy link
Owner

mosra commented Nov 14, 2019

Merged as c8d2c33 (and without my patch above, since it was needed). Thank you!

@mosra mosra closed this Nov 14, 2019
Platforms automation moved this from TODO to Done Nov 14, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Platforms
  
Done
3 participants
You can’t perform that action at this time.