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

Support for resizeable windows with adaptive UI. #23161

Merged
merged 12 commits into from
May 18, 2018
Merged
2 changes: 1 addition & 1 deletion src/cursesdef.h
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,6 @@ void mvwvline( const window &win, int y, int x, chtype ch, int n );
void wrefresh( const window &win );
void refresh();
void wredrawln( const window &win, int beg_line, int num_lines );

void mvwprintw( const window &win, int y, int x, const std::string &text );
template<typename ...Args>
inline void mvwprintw( const window &win, const int y, const int x, const char *const fmt,
Expand All @@ -114,6 +113,7 @@ inline void wprintw( const window &win, const char *const fmt, Args &&... args )
return wprintw( win, string_format( fmt, std::forward<Args>( args )... ) );
}

void resizeterm();
void werase( const window &win );
void init_pair( short pair, base_color f, base_color b );
void wmove( const window &win, int y, int x );
Expand Down
7 changes: 7 additions & 0 deletions src/cursesport.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
#include "color.h"
#include "catacharset.h"
#include "animation.h"
#include "game.h"

#include <cstring> // strlen
#include <stdexcept>
Expand Down Expand Up @@ -407,6 +408,12 @@ void catacurses::mvwprintw(const window &win, int y, int x, const std::string &p
return printstring(win.get<cata_cursesport::WINDOW>(), printbuf);
}

//Resizes the underlying terminal after a Window's console resize(maybe?) Not used in TILES
void catacurses::resizeterm()
{
g->init_ui();
}

//erases a window of all text and attributes
void catacurses::werase(const window &win_)
{
Expand Down
4 changes: 4 additions & 0 deletions src/game.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -205,6 +205,7 @@ void intro();
game *g;
#ifdef TILES
extern std::unique_ptr<cata_tiles> tilecontext;
extern void toggle_fullscreen_window();
#endif // TILES

uistatedata uistate;
Expand Down Expand Up @@ -693,6 +694,9 @@ void game::toggle_fullscreen()
fullscreen = !fullscreen;
init_ui();
refresh_all();
#else
toggle_fullscreen_window();
refresh_all();
#endif
}

Expand Down
12 changes: 12 additions & 0 deletions src/ncurses_def.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
#include "catacharset.h"
#include "color.h"

#include "game.h"
Copy link
Contributor

Choose a reason for hiding this comment

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

The fact that so much pure output code now depends on "game.h" and class game worries me. This is not a good design.

Copy link
Contributor Author

@Dyrewulfe Dyrewulfe Mar 12, 2018

Choose a reason for hiding this comment

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

I agree. I only pulled this header to access game::init_ui. Perhaps there's a better location for it? The game class seems far too involved in handling output in general, in my opinion. Of course, it's also a behemoth that I'm sure no one really wants to suffer through refactoring.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We could also extend our available toolkit with signals. A signal would be an excellent way to handle this situation.

Copy link
Member

Choose a reason for hiding this comment

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

How so? This is a dependency issue, the primary uses of signals are IPC and asynchronicity, we arent dealing with either of those here.

Copy link
Contributor Author

@Dyrewulfe Dyrewulfe Mar 14, 2018

Choose a reason for hiding this comment

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

When it comes down to it, we are dealing with an asynchronous event. Window resizing is an external event generated by the platform window manager, and a signal and slot approach let's us handle it with ease. It also let's us work around the tight coupling issue and avoid adding in another header just to run a single function.

Edit: I can use this approach to tackle the issues with overlay UI resizing and screen redraws, as well.

Copy link
Member

Choose a reason for hiding this comment

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

Just because there is asynchronous behavior happening somewhere doesn't mean it's suitable to use an asynchronous solution for this particular problem.
In this particular case you are avoiding moving logic out of the game module (which we are in dire need of) and creating a new header by adding a new header and replacing a simple method call with indirection through the OS.
If you want to add support for handling external signals, cool, do that.
Even if we have signal support it is not ok to do communication between different parts of the program with signals.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is the merge blocker on this the ui code being moved out of game? As far as I can tell, that requires changing the ownership of the windows, which affects all of the ui code using g->w_whatever, as well as requiring some serious changes to game itself.

That'd turn a small feature PR into a massive refactor PR, wouldn't it?

Copy link
Member

Choose a reason for hiding this comment

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

No, the problem is just the new inclusion of game.h, you can just add a function that accepts no arguments and unconditionally invokes the g->init_ui(), and a later cleanup can move more code into that module.
I probably would have done it for you if I had time at the moment.

#include <stdexcept>

extern int VIEW_OFFSET_X; // X position of terrain window
Expand Down Expand Up @@ -197,6 +198,15 @@ void catacurses::init_pair( const short pair, const base_color f, const base_col

catacurses::window catacurses::stdscr;

void catacurses::resizeterm()
{
const int new_x = ::getmaxx( stdscr.get<::WINDOW>() );
const int new_y = ::getmaxy( stdscr.get<::WINDOW>() );
if( ::is_term_resized( new_x, new_y ) ) {
g->init_ui();
}
}

// init_interface is defined in another cpp file, depending on build type:
// wincurse.cpp for Windows builds without SDL and sdltiles.cpp for SDL builds.
void catacurses::init_interface()
Expand Down Expand Up @@ -245,6 +255,8 @@ input_event input_manager::get_input_event()
rval.type = CATA_INPUT_ERROR;
}
// ncurses mouse handling
} else if( key == KEY_RESIZE ) {
catacurses::resizeterm();
} else if( key == KEY_MOUSE ) {
MEVENT event;
if( getmouse( &event ) == OK ) {
Expand Down
3 changes: 3 additions & 0 deletions src/output.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2088,6 +2088,8 @@ std::string format_volume( const units::volume &volume, int width, bool *out_tru

// In non-SDL mode, width/height is just what's specified in the menu
#if !defined(TILES)
// We need to override these for Windows console resizing
#if !(defined _WIN32 || defined __WIN32__)
int get_terminal_width()
{
int width = get_option<int>( "TERMINAL_X" );
Expand All @@ -2098,6 +2100,7 @@ int get_terminal_height()
{
return get_option<int>( "TERMINAL_Y" );
}
#endif

bool is_draw_tiles_mode()
{
Expand Down
54 changes: 53 additions & 1 deletion src/sdltiles.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -230,6 +230,7 @@ int fontwidth; //the width of the font, background is always this size
int fontheight; //the height of the font, background is always this size
static int TERMINAL_WIDTH;
static int TERMINAL_HEIGHT;
bool fullscreen;

static SDL_Joystick *joystick; // Only one joystick for now.

Expand Down Expand Up @@ -323,16 +324,17 @@ bool WinCreate()
int window_flags = 0;
WindowWidth = TERMINAL_WIDTH * fontwidth;
WindowHeight = TERMINAL_HEIGHT * fontheight;
window_flags |= SDL_WINDOW_RESIZABLE;

if( get_option<std::string>( "SCALING_MODE" ) != "none" ) {
window_flags |= SDL_WINDOW_RESIZABLE;
SDL_SetHint( SDL_HINT_RENDER_SCALE_QUALITY, get_option<std::string>( "SCALING_MODE" ).c_str() );
}

if (get_option<std::string>( "FULLSCREEN" ) == "fullscreen") {
window_flags |= SDL_WINDOW_FULLSCREEN;
} else if (get_option<std::string>( "FULLSCREEN" ) == "windowedbl") {
window_flags |= SDL_WINDOW_FULLSCREEN_DESKTOP;
fullscreen = true;
SDL_SetHint(SDL_HINT_VIDEO_MINIMIZE_ON_FOCUS_LOSS, "0");
}

Expand Down Expand Up @@ -360,6 +362,8 @@ bool WinCreate()
TERMINAL_HEIGHT = WindowHeight / fontheight;
}

SDL_SetWindowMinimumSize( ::window.get(), fontwidth * 80, fontheight * 24 );

// Initialize framebuffer caches
terminal_framebuffer.resize(TERMINAL_HEIGHT);
for (int i = 0; i < TERMINAL_HEIGHT; i++) {
Expand Down Expand Up @@ -659,6 +663,8 @@ static void try_sdl_update()
}
}



//for resetting the render target after updating texture caches in cata_tiles.cpp
void set_displaybuffer_rendertarget()
{
Expand Down Expand Up @@ -1246,6 +1252,49 @@ long sdl_keysym_to_curses( SDL_Keysym keysym )
}
}

bool handle_resize(int w, int h)
{
if( ( w != WindowWidth ) || ( h != WindowHeight ) ) {
WindowWidth = w;
WindowHeight = h;
TERMINAL_WIDTH = WindowWidth / fontwidth;
TERMINAL_HEIGHT = WindowHeight / fontheight;
SetupRenderTarget();
g->init_ui();
tilecontext->reinit_minimap();

return true;
}
return false;
}

void toggle_fullscreen_window()
{
static int restore_win_w = get_option<int>( "TERMINAL_X" ) * fontwidth;
static int restore_win_h = get_option<int>( "TERMINAL_Y" ) * fontheight;

if ( fullscreen ) {
if( SDL_SetWindowFullscreen( ::window.get(), 0 ) != 0 ) {
dbg(D_ERROR) << "SDL_SetWinodwFullscreen failed: " << SDL_GetError();
return;
}
SDL_RestoreWindow( ::window.get() );
SDL_SetWindowSize( window.get(), restore_win_w, restore_win_h );
Copy link
Contributor

Choose a reason for hiding this comment

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

Why are you accessing the global window in an inconsistent way (with and without :: prefix)? The prefix is not needed in this function all.

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 honestly have no idea. I was jumping between this function and WinCreate to work things out, I guess I got my wires crossed. Why exactly is the prefix being used in WinCreate?

SDL_SetWindowMinimumSize( ::window.get(), fontwidth * 80, fontheight * 24 );
} else {
restore_win_w = WindowWidth;
restore_win_h = WindowHeight;
if( SDL_SetWindowFullscreen( ::window.get(), SDL_WINDOW_FULLSCREEN_DESKTOP ) != 0 ) {
dbg(D_ERROR) << "SDL_SetWinodwFullscreen failed: " << SDL_GetError();
return;
}
}
int nw, nh;
SDL_GetWindowSize( ::window.get(), &nw, &nh );
handle_resize( nw, nh );
fullscreen = !fullscreen;
}

//Check for any window messages (keypress, paint, mousemove, etc)
void CheckMessages()
{
Expand All @@ -1266,6 +1315,9 @@ void CheckMessages()
case SDL_WINDOWEVENT_RESTORED:
needupdate = true;
break;
case SDL_WINDOWEVENT_RESIZED:
needupdate = handle_resize( ev.window.data1, ev.window.data2 );
break;
default:
break;
}
Expand Down
87 changes: 68 additions & 19 deletions src/wincurse.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,9 @@ std::array<RGBQUAD, color_loader<RGBQUAD>::COLOR_NAMES_COUNT> windowsPalette;
unsigned char *dcbits; //the bits of the screen image, for direct access
bool CursorVisible = true; // Showcursor is a somewhat weird function

static int TERMINAL_WIDTH;
static int TERMINAL_HEIGHT;

//***********************************
//Non-curses, Window functions *
//***********************************
Expand Down Expand Up @@ -87,7 +90,7 @@ bool WinCreate()
return false;

// Adjust window size
uint32_t WndStyle = WS_CAPTION | WS_MINIMIZEBOX | WS_SYSMENU | WS_VISIBLE; // Basic window, show on creation
uint32_t WndStyle = WS_CAPTION | WS_MINIMIZEBOX | WS_SIZEBOX | WS_MAXIMIZEBOX | WS_SYSMENU | WS_VISIBLE; // Basic window, show on creation
RECT WndRect;
WndRect.left = WndRect.top = 0;
WndRect.right = WindowWidth;
Expand All @@ -108,7 +111,7 @@ bool WinCreate()
0, 0, WindowINST, NULL);
if (WindowHandle == 0)
return false;

return true;
};

Expand All @@ -126,6 +129,36 @@ void WinDestroy()
}
};

// creates a backbuffer to prevent flickering
void create_backbuffer()
{
if( WindowDC != NULL ) {
if( ReleaseDC(WindowHandle, WindowDC) == 0 ) {
WindowDC = 0;
Copy link
Contributor

Choose a reason for hiding this comment

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

Setting this to 0 (why not nullptr, it's not of a numeric type, it's a pointer) is quite pointless. It's overridden unconditionally a few lines below anyway. Same with backbuffer.

}
}
if( backbuffer != NULL ) {
if( ReleaseDC(WindowHandle, backbuffer) == 0 ) {
backbuffer = 0;
}
}
WindowDC = GetDC(WindowHandle);
backbuffer = CreateCompatibleDC(WindowDC);

BITMAPINFO bmi = BITMAPINFO();
bmi.bmiHeader.biSize = sizeof(BITMAPINFOHEADER);
bmi.bmiHeader.biWidth = WindowWidth;
bmi.bmiHeader.biHeight = -WindowHeight;
bmi.bmiHeader.biPlanes = 1;
bmi.bmiHeader.biBitCount = 8;
bmi.bmiHeader.biCompression = BI_RGB; // Raw RGB
bmi.bmiHeader.biSizeImage = WindowWidth * WindowHeight * 1;
bmi.bmiHeader.biClrUsed = color_loader<RGBQUAD>::COLOR_NAMES_COUNT; // Colors in the palette
bmi.bmiHeader.biClrImportant = color_loader<RGBQUAD>::COLOR_NAMES_COUNT; // Colors in the palette
backbit = CreateDIBSection(0, &bmi, DIB_RGB_COLORS, (void**)&dcbits, NULL, 0);
DeleteObject(SelectObject(backbuffer, backbit));//load the buffer into DC
}

// Copied from sdlcurses.cpp
#define ALT_BUFFER_SIZE 8
static char alt_buffer[ALT_BUFFER_SIZE] = {};
Expand Down Expand Up @@ -251,6 +284,26 @@ LRESULT CALLBACK ProcessMessages(HWND__ *hWnd,unsigned int Msg,
lastchar = code;
}
return 0;

case WM_SIZE:
case WM_SIZING:
RECT WndRect;
if( GetClientRect( WindowHandle, &WndRect ) ) {
TERMINAL_WIDTH = WndRect.right / fontwidth;
TERMINAL_HEIGHT = WndRect.bottom / fontheight;
WindowWidth = TERMINAL_WIDTH * fontwidth;
WindowHeight = TERMINAL_HEIGHT * fontheight;
catacurses::resizeterm();
create_backbuffer();
SetBkMode(backbuffer, TRANSPARENT);//Transparent font backgrounds
SelectObject(backbuffer, font);//Load our font into the DC
color_loader<RGBQUAD>().load( windowsPalette );
if( SetDIBColorTable(backbuffer, 0, windowsPalette.size(), windowsPalette.data() ) == 0 ) {
throw std::runtime_error( "SetDIBColorTable failed" );
Copy link
Contributor

@BevapDin BevapDin Mar 12, 2018

Choose a reason for hiding this comment

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

This is throwing from a callback function that is called from non-C++ code. And the exception will leave the C++ code and that basically invokes Undefined Behaviour.

Btw. the call to color_loader::load above can also throw.

Copy link
Contributor Author

@Dyrewulfe Dyrewulfe Mar 12, 2018

Choose a reason for hiding this comment

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

I was winging it with wincurse.cpp, honestly. Reading M$ Windows API references is like sticking a red hot poker in my eyes. I didn't really expect to get this platform to work at all.

}
catacurses::refresh();
}
return 0;

case WM_SYSCHAR:
add_alt_code((char)wParam);
Expand Down Expand Up @@ -458,6 +511,14 @@ int projected_window_height(int)
return get_option<int>( "TERMINAL_Y" ) * fontheight;
}

int get_terminal_width() {
return TERMINAL_WIDTH;
}

int get_terminal_height() {
return TERMINAL_HEIGHT;
}

//***********************************
//Pseudo-Curses Functions *
//***********************************
Expand All @@ -474,28 +535,16 @@ void catacurses::init_interface()
::fontheight = fl.fontheight;
halfwidth=fontwidth / 2;
halfheight=fontheight / 2;
WindowWidth= get_option<int>( "TERMINAL_X" ) * fontwidth;
WindowHeight = get_option<int>( "TERMINAL_Y" ) * fontheight;
TERMINAL_WIDTH = get_option<int>( "TERMINAL_X" );
TERMINAL_HEIGHT = get_option<int>( "TERMINAL_Y" );
WindowWidth = TERMINAL_WIDTH * fontwidth;
WindowHeight = TERMINAL_HEIGHT * fontheight;

WinCreate(); //Create the actual window, register it, etc
timeBeginPeriod(1); // Set Sleep resolution to 1ms
CheckMessages(); //Let the message queue handle setting up the window

WindowDC = GetDC(WindowHandle);
backbuffer = CreateCompatibleDC(WindowDC);

BITMAPINFO bmi = BITMAPINFO();
bmi.bmiHeader.biSize = sizeof(BITMAPINFOHEADER);
bmi.bmiHeader.biWidth = WindowWidth;
bmi.bmiHeader.biHeight = -WindowHeight;
bmi.bmiHeader.biPlanes = 1;
bmi.bmiHeader.biBitCount = 8;
bmi.bmiHeader.biCompression = BI_RGB; // Raw RGB
bmi.bmiHeader.biSizeImage = WindowWidth * WindowHeight * 1;
bmi.bmiHeader.biClrUsed = color_loader<RGBQUAD>::COLOR_NAMES_COUNT; // Colors in the palette
bmi.bmiHeader.biClrImportant = color_loader<RGBQUAD>::COLOR_NAMES_COUNT; // Colors in the palette
backbit = CreateDIBSection(0, &bmi, DIB_RGB_COLORS, (void**)&dcbits, NULL, 0);
DeleteObject(SelectObject(backbuffer, backbit));//load the buffer into DC
create_backbuffer();

// Load private fonts
if (SetCurrentDirectoryW(L"data\\font")){
Expand Down