GUI: Game Options crashes launcher at small application widths

[antoniou79]

Tested on Windows 10 x64 (Version 10.0.19044 Build 19044, all updates) built from latest master (HEAD).

Steps to reproduce:

• Launch ScummVM in windowed mode
• From the launcher select any game from the list
• Click on "Game Options..." (former "Edit Game...") button.
• When the dialogue opens, try to resize the ScummVM's window borders so as to reduce the window's width to a very small size. (see attached screenshot for example)

Result:
ScummVM crashes with the message:

Assertion failed: isValidRect(), file ./common/rect.h, line 160

Expected Result:
ScummVM should handle this without crashing.

The crash does not occur for the launcher's "Global Options...", so it seems to be specific to the "Game Options..." dialogue.

If the width of the launcher is already small when launching ScummVM, then the crash will occur immediately upon clicking on "Game Options..."

A quick debugging session with VS indicates that the fault lies somewhere in ScrollContainerWidget::reflowLayout() in gui\widgets\scrollcontainer.cpp during the loop of "reflow layout of inner widgets"

From the related discussion on Discord, it seems that the issue has been around for a while (quite possibly since 2.2.0, according to rootfather)
​https://discord.com/channels/581224060529148060/711242520415174666/949661861152960532

[antoniou79]

Further looking into this, it seems that the culprit (or one of them) is EditTextWidget::getEditRect() from edittext.cpp and ​in particular this line:

Common::Rect r(2 + _leftPadding, 1, _w - 1 - _rightPadding, _h);

which, for low widths, attempts to create a Rect for which the right x coordinate is lower than the left x coordinate -- and this triggers the assertion fault in the isValidRect() check.

EditTextWidget::getEditRect() is called by EditableWidget::reflowLayout() at ​this line:

	_editScrollOffset = g_gui.getStringWidth(_editString, _font) - getEditRect().width();

The Game Options "Game" tab has two editable fields, so they are the ones causing the crash.
In very low widths, maybe these fields should not show up at all (just to prevent the crash)?

[antoniou79]

I've implemented a small fix for this, based on similar code in our listWidget (list.cpp); see:
​https://github.com/scummvm/scummvm/blob/1be7de067bf7b77bcc7aa4cb6c8fd760e7448205/gui/widgets/list.cpp#L622

Pull Request here:
​https://github.com/scummvm/scummvm/pull/3739

[antoniou79]

The PR was merged, so this ticket can be closed.