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

Window manager for properly refreshing and resizing multiple layers of windows #37894

Merged
merged 1 commit into from
Mar 16, 2020

Conversation

Qrox
Copy link
Contributor

@Qrox Qrox commented Feb 10, 2020

Summary

SUMMARY: Infrastructure "Window manager for properly refreshing and resizing multiple layers of windows"

Purpose of change

Currently many game menus can be called from multiple places, such as the keybindings menu which can be opened from nearly every game menu. However, when closing some of these menus, keybindings menu included, only the previous menu is redrawn, leaving black space on the screen. Additionally, when resizing the game window, most of the menus aren't correctly resized and re-positioned.

Describe the solution

A new window manager that stores ui_adaptor objects, which UI code can use to encapsulate their initialization and redrawing code as callbacks. When closing a menu or resizing the game window, UIs are invalidated accordingly and redrawn, so all layers of windows are correctly displayed.

This PR only contains the code for the ui manager. Menus need to be migrated individually to be properly refreshed and resized. For example, #38235 migrates the main menu and several related menus using ui_manager.

Describe alternatives you've considered

If the UI code was to be designed from scratch I would use an object-oriented event-based system, but since most of the game's UI code was written to be sequentially executed, this is a solution I found least intrusive.

Testing

Compiled with #38235, triggered a popup in the main game UI, and opened its keybindings menu. Closed the keybindings menu and observed that there was not black space left behind. Resized the game window and observed that the popup was correctly placed in the center of the screen.

@Night-Pryanik
Copy link
Contributor

This should close #8514 once and for all?

@ZhilkinSerg ZhilkinSerg added [C++] Changes (can be) made in C++. Previously named `Code` Info / User Interface Game - player communication, menus, etc. labels Feb 10, 2020
@Qrox
Copy link
Contributor Author

Qrox commented Feb 11, 2020

It could, but the related menus should be first migrated to use this system. I'll migrate the menus mentioned there if it turns out to be doable in this PR.

@Qrox Qrox force-pushed the window-manager branch 2 times, most recently from fa1024e to 12ad380 Compare February 11, 2020 14:19
src/sdltiles.cpp Outdated
// Main menu redraw
reinitialize_framebuffer();
// TODO: redraw all game menus if they are open
ui_manager::redraw_all();
Copy link
Contributor

@8street 8street Feb 11, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please test it in fullscreen dx9 mode. I remember in fullscreen mode when the game window was maximized after alt-tabbing, an event appeared - SDL_WINDOWEVENT_FOCUS_GAINED but not SDL_WINDOWEVENT_EXPOSED.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've tested in fullscreen with direct3d and it was refreshing properly. I didn't see a dx9 mode though, so I'm not sure if that's what you mean.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok.

@Qrox Qrox force-pushed the window-manager branch 2 times, most recently from 304f5b4 to faf6881 Compare February 15, 2020 07:50
@Qrox
Copy link
Contributor Author

Qrox commented Feb 15, 2020

@Night-Pryanik the menus mentioned in that issue are more complex than I expected so I'll leave them to a later PR.

@Qrox Qrox marked this pull request as ready for review February 15, 2020 09:08
@Qrox
Copy link
Contributor Author

Qrox commented Feb 15, 2020

Also, can someone test if this is working on ncurses?

@anothersimulacrum
Copy link
Member

@Qrox Do you need anything specific for testing on curses? Just open the game play around with menus, make a new character, edit settings, make a new world, etc.?

@Qrox
Copy link
Contributor Author

Qrox commented Feb 26, 2020

@anothersimulacrum Thanks! There are two things I would like to test on curses:

  1. Refreshing when 3 or more windows are open and the last one is closed. For example, if I press ESC in the main menu, and open the keybindings menu in the exit confirmation popup, when closing the keybindings menu I expect the main menu to be refreshed instead of partly erased.
  2. Resizing when 2 or more windows are open and the terminal size is changed. I expect every open windows to be resized according to the new terminal size.

On tiles I also expect the window to be refreshed when the window content is invalidated by system, such as when recovering from a sleep, but I assume on curses this is done by the terminal software and requires no action from the game.

@anothersimulacrum
Copy link
Member

anothersimulacrum commented Feb 27, 2020

Refreshing for that first case does not seem to work
refresh

I also tested with xterm, got the same results.
Resizing with no windows open - it's a bit odd
resize
(Recording isn't the highest quality, sorry)
Resizing with windows open does not work (safe mode manager, and then editing the proximity in it are the windows)
resize_wind

Though the first one does work on tiles either
tiles
Nor does the second one, it just goes black.
tiles2
(resizing without having windows opens works)

@Qrox
Copy link
Contributor Author

Qrox commented Feb 27, 2020

Have you applied the changes in #38235? That's where the actual migration takes place.

@anothersimulacrum
Copy link
Member

No, that would probably help.

@Qrox Qrox force-pushed the window-manager branch from 7940eb8 to 71c5597 Compare March 2, 2020 07:46
@anothersimulacrum
Copy link
Member

Okay, with the changes to make it work applied, it works correctly.
1
2

@ZhilkinSerg ZhilkinSerg changed the base branch from master to dev March 16, 2020 11:24
@ZhilkinSerg ZhilkinSerg merged commit e91f938 into CleverRaven:dev Mar 16, 2020
@ZhilkinSerg ZhilkinSerg mentioned this pull request Mar 16, 2020
13 tasks
ZhilkinSerg pushed a commit that referenced this pull request Mar 17, 2020
For properly refreshing and resizing multiple layers of windows
ZhilkinSerg pushed a commit that referenced this pull request Mar 17, 2020
For properly refreshing and resizing multiple layers of windows
ZhilkinSerg pushed a commit that referenced this pull request Mar 18, 2020
For properly refreshing and resizing multiple layers of windows
@Qrox Qrox deleted the window-manager branch March 18, 2020 14:39
ZhilkinSerg pushed a commit that referenced this pull request Mar 29, 2020
For properly refreshing and resizing multiple layers of windows
ZhilkinSerg pushed a commit that referenced this pull request Apr 1, 2020
For properly refreshing and resizing multiple layers of windows
ZhilkinSerg pushed a commit that referenced this pull request Apr 1, 2020
For properly refreshing and resizing multiple layers of windows
ZhilkinSerg pushed a commit that referenced this pull request Apr 2, 2020
For properly refreshing and resizing multiple layers of windows
ZhilkinSerg pushed a commit that referenced this pull request Apr 2, 2020
For properly refreshing and resizing multiple layers of windows
// add_msg( m_info, "tile map fw %d fh %d", fw, fh);
} else if( map_font && capture_win == g->w_terrain ) {
window_dimensions dim;
if( use_tiles && win == g->w_terrain ) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Apparently sometimes g could be empty when there are errors (see #39258), so we need to check for non-emptiness prior to comparing win with members of g, e.g.:

    window_dimensions dim;
    dim.scaled_font_size.x = fontwidth;
    dim.scaled_font_size.y = fontheight;
    if( g ) {
        if( use_tiles && win == g->w_terrain ) {
            // tiles might have different dimensions than standard font
            dim.scaled_font_size.x = tilecontext->get_tile_width();
            dim.scaled_font_size.y = tilecontext->get_tile_height();
        } else if( map_font && win == g->w_terrain ) {
            // map font (if any) might differ from standard font
            dim.scaled_font_size.x = map_font->fontwidth;
            dim.scaled_font_size.y = map_font->fontheight;
        } else if( overmap_font && win == g->w_overmap ) {
            dim.scaled_font_size.x = overmap_font->fontwidth;
            dim.scaled_font_size.y = overmap_font->fontheight;
        }
    }

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[C++] Changes (can be) made in C++. Previously named `Code` Info / User Interface Game - player communication, menus, etc.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants