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

Significant main menu filckering on curses #37191

Closed
ifreund opened this issue Jan 18, 2020 · 13 comments
Closed

Significant main menu filckering on curses #37191

ifreund opened this issue Jan 18, 2020 · 13 comments
Labels
<Bug> This needs to be fixed Info / User Interface Game - player communication, menus, etc. (S2 - Confirmed) Bug that's been confirmed to exist

Comments

@ifreund
Copy link
Contributor

ifreund commented Jan 18, 2020

Describe the bug

Top layer elements on the main menu flicker in and out with the background on curses builds.

Steps To Reproduce

Steps to reproduce the behavior:

  1. Run the game from your terminal emulator
  2. Open the credits page of the main menu
  3. Observe the flickering

Expected behavior

There should be no flickering

Screenshots

https://streamable.com/9qm7m

Versions and configuration

  • OS: Linux
    • OS Version: LSB Version: 1.4; Distributor ID: Arch; Description: Arch Linux; Release: rolling; Codename: n/a;
  • Game Version: 0.D-11440-g82a2eb2ab5 [64-bit]
  • Graphics Version: Curses
  • Mods loaded: [
    Dark Days Ahead [dda],
    Disable NPC Needs [no_npc_food]
    ]
@ifreund ifreund added <Bug> This needs to be fixed Info / User Interface Game - player communication, menus, etc. (S2 - Confirmed) Bug that's been confirmed to exist labels Jan 18, 2020
@ifreund ifreund added this to the 0.E milestone Jan 18, 2020
@ZhilkinSerg ZhilkinSerg added the (P2 - High) High priority (for ex. important bugfixes) label Jan 18, 2020
@ifreund
Copy link
Contributor Author

ifreund commented Jan 18, 2020

Ok, I've done some more investigating and out of the terminal emulators I've tested this flicker is only apparent on Alacritty. This seems to be because Alacritty is really fast and is rendering frames that other terminal emulators are dropping. Slowing down alacritty by running in verbose mode outputting to a file with alacritty -vvv > out.log and maximizing the window reduces the observed flickering substantially.

I would still consider this a bug with cataclysm though, as we should not be redrawing these menus if the state is not changing.

The only popup on the main menu that does not flicker is the help menu, so we should be able to look at how that is drawn to the screen and apply that to the rest of the main menu popups.

@ifreund ifreund removed the (P2 - High) High priority (for ex. important bugfixes) label Jan 18, 2020
@ifreund
Copy link
Contributor Author

ifreund commented Jan 18, 2020

Looks like this issue is rooted in the fact that we are currently attempting to redraw the main menu every 125 milliseconds no matter what

std::string main_menu::handle_input_timeout( input_context &ctxt )
{
std::string action = ctxt.handle_input( 125 );
if( action == "TIMEOUT" ) {
init_windows();
}
return action;
}

The reason we are currently doing this is because that is how we make sure that the screen is redrawn on resizing the terminal. This, i should think, obviously not the right way to solve that problem and we probably should be either polling for terminal size change directly or using some type of event handling.

Edit: to set the record straight this was all @esotericist's idea

@ghost
Copy link

ghost commented Jan 19, 2020

This also manifests on *nix native tty's and wincurses port.

@8street
Copy link
Contributor

8street commented Jan 20, 2020

I somehow came across such a problem. Try changing init_windows(); to game_ui::init_ui(); or reinitialize_framebuffer();. Perhaps it will help. I can try it myself much later.

@ifreund
Copy link
Contributor Author

ifreund commented Jan 20, 2020

@8street If you want a quick fix switching the 125 to something like 1000 will do the job. Only downside is that there will then be a delay of up to 1 second when resizing the window before the UI adjusts to the new size.

Maybe that change would be sufficient to declassify this as a release blocker though? A proper fix is a good bit more in depth.

@8street
Copy link
Contributor

8street commented Jan 20, 2020

Maybe that change would be sufficient to declassify this as a release blocker though?

Can you build the game with your changes? And what will happen if the user performs many actions at intervals of less than 1 second?

@ifreund
Copy link
Contributor Author

ifreund commented Jan 20, 2020

Here's the patch. It builds just fine so feel free to compile and test it yourself

From b188e9950809a197257c416aec9293b81725cb5a Mon Sep 17 00:00:00 2001
From: Isaac Freund <[email protected]>
Date: Mon, 20 Jan 2020 12:34:03 +0100
Subject: [PATCH] Bump main menu input timeout to 1 second

---
 src/main_menu.cpp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/main_menu.cpp b/src/main_menu.cpp
index 15d4a85372..21ebf6bbdc 100644
--- a/src/main_menu.cpp
+++ b/src/main_menu.cpp
@@ -242,7 +242,7 @@ std::vector<std::string> main_menu::load_file( const std::string &path,
 
 std::string main_menu::handle_input_timeout( input_context &ctxt )
 {
-    std::string action = ctxt.handle_input( 125 );
+    std::string action = ctxt.handle_input( 1000 );
 
     if( action == "TIMEOUT" ) {
         init_windows();
--
2.25.0

This is only changing the input timeout. It doesn't affect responsiveness to input at all, only how long the game waits for input each loop before deciding to move on. If anything, this patch would marginally increase input responsiveness.

If we had a proper solution to handling resizing as outlined above the timeout could be made infinitely large without issue.

@Godridge
Copy link

some flickering in alacritty on main menu (0.E), background logo text flashing over stuff like developer credits for a frame

@ifreund
Copy link
Contributor Author

ifreund commented Apr 29, 2020

Not too surprised, the patch above was just a hack to make things work slightly better for 0.E. As mentioned, a proper solution would be a good deal more invasive.

@kevingranade kevingranade removed this from the 0.E milestone May 6, 2020
@Leland
Copy link
Contributor

Leland commented Jun 6, 2020

Someone wanna retest this? That refresh rate was removed entirely in ae68d3f in favor of Qrox's ui_adaptor. Not sure if that landed in 0.E, per Godridge's comment.

@anothersimulacrum
Copy link
Member

anothersimulacrum commented Jun 6, 2020

Cannot reproduce anymore in TTY, xterm, or alacritty - I think it's been fixed.

And yeah, that did not land in 0.E, AFAIK.

@ghost
Copy link

ghost commented Nov 9, 2020

0.E-2 is doing this for me with alacritty: https://boop.icu/QM.webm

@anothersimulacrum
Copy link
Member

Would you mind testing and seeing if that persists on an experimental build? If not, ae68d3f handled it, otherwise, we may need to be looking at it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
<Bug> This needs to be fixed Info / User Interface Game - player communication, menus, etc. (S2 - Confirmed) Bug that's been confirmed to exist
Projects
None yet
Development

No branches or pull requests

7 participants