[Suggestion] Limit stretching of Menu buttons

[Noseey]

Since I like to play in 21:9 and fullscreen, I found that the menu buttons are stretched a bit too much looking slightly unpleasant and confusing, here an example:

Would it be acceptable to limit them a bit, maybe like indicated by the red border? If it is, I could have a look into it.

[Flow86]

currently all elements are scaled "as is" since it was the simplest solution. If you have an idea, feel free to find a better way :-)

[Noseey]

Alright, then I will give it a shot and see if I can come up with something reasonable. 😀

[Flamefire]

Currently all elements have a generic scaling capability, so you only have to specify "normalized" sizes (IIRC for a 800x600 screen size) and the scaling will adjust them automatically so relative sizes are preserved.

I'm afraid avoiding "too stretched" will be a) opinionated b) lot's of work providing manual max limits and/or c) messy code to handle that.

Maybe try sketching up (before coding) an approach for a solution that works for many situations, which need to be gathered first: Where else do you see such "issues"? (in quotes because I personally don't mind that, we rather have an issue with too small elements/texts in HighDPI settings) And how would they be solved with that approach.

[Noseey]

Ofc, this is all very cosmetic and functionally this isn't an issue at all. I think my "problem" with it is really specific to ultrawide aspect ratios, as with 16:9, 16:10 the generic scaling still looks good to me.

Where else do you see such "issues"?

1. For me, it is most distracting in main menu and sub-menus "Singleplayer", "Multiplayer" and "Direct IP".
2. It is slightly distracting in "Options", "Campaign Selection", "Endless Game", "Lobby",

And how would they be solved with that approach.

After having a first look in the Code, a current rough idea could be as follows:

• Setting a scale limit, similar to the scale_
  --> can happen once in the constructor of the deskop?
• Adapting the Window::Scale and ScaleWindowPropUp::scale to consider an additional parameter limited as bool.
  --> The limiting itself is still a bit TBD, but should be relative to the incoming size values and probably not absolute?
• Introducing a separate Window::ScaleLimIf function
  --> that still checks if a scale_ should be applied at all
  --> additionally passes the limited true to Window::Scale
• exchange specific ScaleIf with ScaleLimIf in the code for Size parameters
  --> especially upon AddTextButton
  --> position scaling should remain untouched

This way, exiting ScaleIfs can largely remain untouched in the code and only a few would change to ScaleLimIf. Also, no new parameter is needed by the caller.

Some open questions:

• How can this interfer with the GUI Scaling option for the text? (could there be a scenario where text is cut off?)
• How to consider also windowed mode and rescaling correctly?

[Flamefire]

How to consider also windowed mode and rescaling correctly?

That is currently done here:

s25client/libs/s25main/Window.cpp

Lines 532 to 549 in 7a79bd2

	void Window::Msg_ScreenResize(const ScreenResizeEvent& sr)
	{
	// If the window elements don't get scaled there is nothing to do
	if(!scale_)
	return;
	RescaleWindowProp rescale(sr.oldSize, sr.newSize);
	for(Window* ctrl : childIdToWnd_ | boost::adaptors::map_values)
	{
	if(!ctrl)
	continue;
	// Save new size (could otherwise be changed(?) in Msg_ScreenResize)
	Extent newSize = rescale(ctrl->GetSize());
	ctrl->SetPos(rescale(ctrl->GetPos()));
	ctrl->Msg_ScreenResize(sr);
	ctrl->Resize(newSize);
	}
	animations_.onRescale(sr);
	}

It would need to be adjusted for taking the limitation into account

Setting a scale limit, similar to the scale_
--> can happen once in the constructor of the deskop?

When/Where would you do that? For all desktops?

• exchange specific ScaleIf with ScaleLimIf in the code for Size parameters
  --> especially upon AddTextButton
  --> position scaling should remain untouched

I'm wary that this make break some layouts as positions are often set with the expectation that they are scaled the same as sizes. If they are not elements might overlap or (I think) rather spaced out too much.

--> The limiting itself is still a bit TBD, but should be relative to the incoming size values and probably not absolute?

I think that is the most pressing question: What/How to limit in a generic way that works here for everything.

[Noseey]

Sorry for the late response! I thought at this point, some code and tests are necessary.
A first version can be seen here: master...Noseey:s25client:limit_window_scaling

It considers the limiting only for ctrlTextButton by simply capping the scale when over double the size of the base extent.
Here an example screenshot from the menu buttons in 21:9 (mind that the GUI scale was different in the first screenshot, I added a previous one here from comparison):

New:

Old:

When/Where would you do that? For all desktops?

In the end, I refrained from tying it to the desktops but instead tied it to scale_ and the call of ScaleLimIf. The reason for that: I wanted certain elements to be limited instead of whole desktops.

I'm wary that this make break some layouts as positions are often set with the expectation that they are scaled the same as sizes. If they are not elements might overlap or (I think) rather spaced out too much.

Yes, that is the case as seen in the screenshot, it will space out more. To avoid this, one could apply the cap only to the width, otherwise probably more sophisticated methods are required. On the other hand, the extremly stretched buttons also break the visual norm somehow. 😉

If you already see that this causes too much code change for too little effort, let me know and I can close the issue. Otherwise, I can open a draft PR to optimize this further.

[Flamefire]

That might just work. Maybe we can simplify it:

• The size and position are scaled by passed size and reference size (which gets "normalized" to 800x600)
• On resize we rescale by undoing that and redoing the scaling with the new reference size

If we need to store the original size anyway then we could as well store it in the (or a new) ScaleWindowProp and call that for resizing operations instead of constructing it again. Might then even optimize by storing the factor instead of recalculating on each call, but that likely doesn't matter.

And in your commit: The factor should be a factor not a shift value to make it easier to understand.

If you tested it to work in general for desktops, windows etc. also across multiple resizing operations (change resolution multiple times in fullscreen view, change window size multiple times randomly in windowed mode) and there are no issues I'd accept the change.

Asking because currently those make me nervous, as I'm not sure if that really works in all directions:

// hack: take origSize that was set by the last (and presumably only) ScaleLimIf(Extent) call to pass to children.

And in ScaleLimIf:

orig_size_ = pt;

[Noseey]

So after the first draft, here an update on what I would see required for the limiting:

1. ctrlTextButton via AddCtrl on all desktops except dskGameInterface and dskOptions
2. OR alternatively: 1) but include dskOptions and consider all elements there for limiting, not just ctrlTextButton. Exception is the "advanced" window ofc. --> visually not tested yet, but I imagine it could work.

The most flexible solution to achieve this, would be to pass limits for the adding of each element (e. g. AddTextButton in dskMainMenu), but this causes alot of changes in the code and might not be required for the request above.

The next still rather flexible solution is, to consider the ctrl type (when adding in Window.cpp) together with the corresponding Desktop, which could fulfill the request and even the alternative. But it requires a variable to track if a desktop in general allows limiting or not, similar to scale_.