Support for resizeable windows with adaptive UI. #23161

Dyrewulfe · 2018-03-09T17:33:52Z

Overlay UIs, including the main menu, are not resized properly if active. I'd say that's a seperate, resultant issue, and outside the scope of this PR.

Closes #933

namarrgon · 2018-03-10T12:25:26Z

This works reasonably(!) well in the tiles and ncurses versions but breaks down when overlay dialog windows, such as the inventory or crafting menus, are open while resizing the window/terminal.

Dyrewulfe · 2018-03-10T14:08:07Z

Yeah, it's the same issue as with the main menu screen. There's no mechanism for rescaling overlay UIs while they are active. There's never been a need for such a thing before.

Dyrewulfe · 2018-03-10T21:05:34Z

~~What's the best solution available for testing that we're currently "in game"?~~ Never mind, it's a not an issue to handle at this level.

Dyrewulfe · 2018-03-11T10:59:04Z

I hooked into the existing fullscreen toggle to allow toggling windowed mode in tiles. Any issues with that?

Leland · 2018-03-12T03:23:25Z

I hooked into the existing fullscreen toggle to allow toggling windowed mode in tiles. Any issues with that?

Oh hell yes, this adds standard macOS fullscreen support (clicking fullscreen "traffic light" button). Very nice

Dyrewulfe · 2018-03-12T03:39:49Z

@Leland Well, the fullscreen thing you quoted is actually attached to the keybinding in game. In curses, it toggles off the sidebar(dependent on term sizes for some reason.) It had no functionality in the tiles build. But, I did my best with this to get every platform to allow it's windowing system to work properly. Glad to hear it works fine on OSX! That's the one platform I have no means to test.

OzoneH3 · 2018-03-12T10:34:17Z

In curses if the size goes below 80x24 it should display intro().

For the main menu a timeout with a recalc/redraw similar to the ingame handle_mouseview() would have to be implemented.

See get_player_input();
which uses inp_mngr.set_timeout( 125 )
to break out of ctxt.handle_input();

After that recalc and redraw the window.

BevapDin · 2018-03-12T17:30:53Z

src/wincurse.cpp

+            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" );


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.

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.

BevapDin · 2018-03-12T17:32:54Z

src/sdltiles.cpp

+            return;
+        }
+        SDL_RestoreWindow( ::window.get() );
+        SDL_SetWindowSize( window.get(), restore_win_w, restore_win_h );


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

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?

BevapDin · 2018-03-12T17:41:27Z

src/wincurse.cpp

+{
+    if( WindowDC != NULL ) {
+        if( ReleaseDC(WindowHandle, WindowDC) == 0 ) {
+            WindowDC = 0;


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.

BevapDin · 2018-03-12T17:47:55Z

src/ncurses_def.cpp

@@ -18,6 +18,7 @@
 #include "catacharset.h"
 #include "color.h"

+#include "game.h"


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

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.

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

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

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.

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.

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?

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.

This reverts commit 362e265.

ZhilkinSerg · 2018-05-18T14:52:13Z

Tiles builds seems to be broken now:

http://ci.narc.ro/job/Cataclysm-Matrix/7409/Graphics=Tiles,Platform=Windows/console

Dyrewulfe added 2 commits March 9, 2018 07:46

Add support for resizing *nix terminal.

2bcdf8a

Add support for resizing window in Tiles.

26fa954

Dyrewulfe changed the title ~~[RDY][CR]Support for resizing windows in *nix terminal and SDL.~~ [WIP][CR]Support for resizing windows in *nix terminal and SDL. Mar 9, 2018

Add support for resizing in Windows console.

1de99f9

Dyrewulfe changed the title ~~[WIP][CR]Support for resizing windows in *nix terminal and SDL.~~ [WIP][CR]Support for resizeable windows with adaptive UI. Mar 9, 2018

Dyrewulfe changed the title ~~[WIP][CR]Support for resizeable windows with adaptive UI.~~ [RDY][CR]Support for resizeable windows with adaptive UI. Mar 10, 2018

Dyrewulfe changed the title ~~[RDY][CR]Support for resizeable windows with adaptive UI.~~ [WIP][CR]Support for resizeable windows with adaptive UI. Mar 10, 2018

Dyrewulfe added 3 commits March 10, 2018 15:45

Remove tiles redraw to avoid crashing.

a7d18d4

Add toggling to/from fullscreen, set min size.

f3a8ea4

Insure minimum size gets set.

8c415d1

Dyrewulfe changed the title ~~[WIP][CR]Support for resizeable windows with adaptive UI.~~ [RDY][CR]Support for resizeable windows with adaptive UI. Mar 11, 2018

BevapDin reviewed Mar 12, 2018

View reviewed changes

Dyrewulfe added 4 commits March 12, 2018 12:21

Move wincurse resize outside of message handler, other CR tweaks.

243659d

Prevent attempting to resize before initialization.

cb4bf35

Remove term size check in toggle_fullscreen to prevent sidebar lockout.

45c2890

Handle window resizing using a signal and slot approach.

362e265

Dyrewulfe changed the title ~~[RDY][CR]Support for resizeable windows with adaptive UI.~~ [WIP][CR]Support for resizeable windows with adaptive UI. Mar 14, 2018

Dyrewulfe added 2 commits March 14, 2018 10:28

Revert "Handle window resizing using a signal and slot approach."

8e04b4d

This reverts commit 362e265.

Remove duplicate resizing code.

70cb269

Dyrewulfe changed the title ~~[WIP][CR]Support for resizeable windows with adaptive UI.~~ Support for resizeable windows with adaptive UI. Mar 14, 2018

ZhilkinSerg added Info / User Interface Game - player communication, menus, etc. SDL: Tiles / Sound Tiles visual interface and sounds. [C++] Changes (can be) made in C++. Previously named `Code` labels Apr 2, 2018

PatrikStridvall mentioned this pull request Apr 8, 2018

Use italic for disabled subtab (with ncurses emulation for SDL) #23419

Merged

kevingranade merged commit 70cb269 into CleverRaven:master May 18, 2018

ZhilkinSerg mentioned this pull request May 18, 2018

Fix Tiles build regression #23760

Merged

OzoneH3 mentioned this pull request Jun 11, 2018

Window resize fixes #23995

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support for resizeable windows with adaptive UI. #23161

Support for resizeable windows with adaptive UI. #23161

Dyrewulfe commented Mar 9, 2018 •

edited

namarrgon commented Mar 10, 2018

Dyrewulfe commented Mar 10, 2018

Dyrewulfe commented Mar 10, 2018 •

edited

Dyrewulfe commented Mar 11, 2018

Leland commented Mar 12, 2018

Dyrewulfe commented Mar 12, 2018 •

edited

OzoneH3 commented Mar 12, 2018

BevapDin Mar 12, 2018 •

edited

Dyrewulfe Mar 12, 2018 •

edited

BevapDin Mar 12, 2018

Dyrewulfe Mar 12, 2018

BevapDin Mar 12, 2018

BevapDin Mar 12, 2018

Dyrewulfe Mar 12, 2018 •

edited

Dyrewulfe Mar 12, 2018

kevingranade Mar 13, 2018

Dyrewulfe Mar 14, 2018 •

edited

kevingranade Mar 14, 2018

Dyrewulfe Apr 8, 2018

kevingranade Apr 9, 2018

ZhilkinSerg commented May 18, 2018

Support for resizeable windows with adaptive UI. #23161

Support for resizeable windows with adaptive UI. #23161

Conversation

Dyrewulfe commented Mar 9, 2018 • edited

namarrgon commented Mar 10, 2018

Dyrewulfe commented Mar 10, 2018

Dyrewulfe commented Mar 10, 2018 • edited

Dyrewulfe commented Mar 11, 2018

Leland commented Mar 12, 2018

Dyrewulfe commented Mar 12, 2018 • edited

OzoneH3 commented Mar 12, 2018

BevapDin Mar 12, 2018 • edited

Choose a reason for hiding this comment

Dyrewulfe Mar 12, 2018 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Dyrewulfe Mar 12, 2018 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Dyrewulfe Mar 14, 2018 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ZhilkinSerg commented May 18, 2018

Dyrewulfe commented Mar 9, 2018 •

edited

Dyrewulfe commented Mar 10, 2018 •

edited

Dyrewulfe commented Mar 12, 2018 •

edited

BevapDin Mar 12, 2018 •

edited

Dyrewulfe Mar 12, 2018 •

edited

Dyrewulfe Mar 12, 2018 •

edited

Dyrewulfe Mar 14, 2018 •

edited