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

Setting resolution really small in openttd.cfg crashes the game #8869

Open
TrueBrain opened this issue Mar 15, 2021 · 6 comments
Open

Setting resolution really small in openttd.cfg crashes the game #8869

TrueBrain opened this issue Mar 15, 2021 · 6 comments

Comments

@TrueBrain
Copy link
Member

@TrueBrain TrueBrain commented Mar 15, 2021

Version of OpenTTD

1.11.0-RC1

Expected result

No crash, even if a user is being silly.

Actual result

Crash.

Steps to reproduce

In your config, set:
resolution = 1,1
Start the game
Enjoy the crash!

Backtrace

__GI_raise (sig=sig@entry=6) at ../sysdeps/unix/sysv/linux/raise.c:50
50      ../sysdeps/unix/sysv/linux/raise.c: No such file or directory.
(gdb) bt
#0  __GI_raise (sig=sig@entry=6) at ../sysdeps/unix/sysv/linux/raise.c:50
#1  0x00007ffff70d1859 in __GI_abort () at abort.c:79
#2  0x00007ffff70d1729 in __assert_fail_base (fmt=0x7ffff7267588 "%s%s%s:%u: %s%sAssertion `%s' failed.\n%n",
    assertion=0x55555674503d "min <= max",
    file=0x555556745000 "OpenTTD/src/core/math_func.hpp", line=79,
    function=<optimized out>) at assert.c:92
#3  0x00007ffff70e2f36 in __GI___assert_fail (assertion=0x55555674503d "min <= max",
    file=0x555556745000 "OpenTTD/src/core/math_func.hpp", line=79,
    function=0x555556744fe0 "T Clamp(T, T, T) [with T = int]") at assert.c:101
#4  0x00005555560ba63e in Clamp<int> (a=0, min=0, max=-12)
    at OpenTTD/src/core/math_func.hpp:79
#5  0x00005555560b1371 in Clamp (a=0, min=0, max=-12)
    at OpenTTD/src/core/math_func.hpp:103
#6  0x00005555560b68ea in EnsureVisibleCaption (w=0x5555588a3220, nx=-12, ny=0)
    at OpenTTD/src/window.cpp:2127
#7  0x00005555560b6d26 in ResizeWindow (w=0x5555588a3220, delta_x=0, delta_y=0, clamp_to_screen=true)
    at OpenTTD/src/window.cpp:2177
#8  0x00005555560b3ea2 in Window::ReInit (this=0x5555588a3220, rx=0, ry=0)
    at OpenTTD/src/window.cpp:1022
#9  0x00005555560b9f55 in ReInitAllWindows () at OpenTTD/src/window.cpp:3466
#10 0x0000555555d9f606 in LoadStringWidthTable (monospace=false)
    at OpenTTD/src/gfx.cpp:1286
#11 0x0000555555fee021 in CheckForMissingGlyphs (base_font=true,
    searcher=0x5555583b4070 <CheckForMissingGlyphs(bool, MissingGlyphSearcher*)::pack_searcher>)
    at OpenTTD/src/strings.cpp:2143
#12 0x0000555555ee3e72 in LoadIntroGame (load_newgrfs=false)
    at OpenTTD/src/openttd.cpp:359
#13 0x0000555555ee5485 in openttd_main (argc=4, argv=0x7fffffffe038)
    at OpenTTD/src/openttd.cpp:830
#14 0x0000555555c1ad7b in main (argc=4, argv=0x7fffffffe038)
    at OpenTTD/src/os/unix/unix.cpp:265
@James103
Copy link
Contributor

@James103 James103 commented Mar 16, 2021

This may be related to #6220, which was closed because of inactivity.

  1. Do you have the full crash.log (and crash.dmp, crash.png if available)? If so, can you please post these files here?
  2. Do you get a similar crash if you run openttd -r 1x1?
  3. Do you get a similar crash if you run openttd -D with resolution = 1,1 in openttd.cfg?

Edit: I was able to reproduce this on Windows. A similar crash occurs if OpenTTD is started with the vertical resolution being 12 pixels or less (13 pixels and up is safe, horizontal resolution does not matter, and a 0 in either axis resets to default)
crash.zip

Loading

@TrueBrain
Copy link
Member Author

@TrueBrain TrueBrain commented Mar 16, 2021

1. Do you have the full `crash.log` (and `crash.dmp`, `crash.png` if available)? If so, can you please post these files here?

Nope; they are not really relevant here. The backtrace is all that is important. And a crash.png for sure won't help .. at best it would be a 1x1 pixel screenshot :D

2. Do you get a similar crash if you run `openttd -r 1x1`?

Nope, this is properly clamped. It is only a problem with the config.

3. Do you get a similar crash if you run `openttd -D` with `resolution = 1,1` in openttd.cfg?

Ironically, and this is often a misconception, running with -D does not prevent the whole window system to initialize. So yes, -D or not really doesn't matter, it will initialize the window system at the requested resolution, and crash if it is 1x1.

Additionally, 1.9 does not crash. So I guess this is a nice regression :)

Loading

@TrueBrain TrueBrain added the bug label Mar 16, 2021
@James103
Copy link
Contributor

@James103 James103 commented Mar 16, 2021

Likely cause, according to the backtrace:
Function EnsureVisibleCaption has the following line:

ny = Clamp(ny, 0, _screen.height - MIN_VISIBLE_TITLE_BAR);

This calls function Clamp(a, min, max), which has the following assertion that gets triggered:
assert(min <= max);

The assertion is triggered because Clamp is called with a min of 0 and a max of _screen.height - MIN_VISIBLE_TITLE_BAR, where _screen.height is 12 or less. MIN_VISIBLE_TITLE_BAR is 13 as per the following line, which makes max negative, and thus triggers the assertion because 0 can never be less than or equal to a negative number.
static const int MIN_VISIBLE_TITLE_BAR = 13;

Loading

@TrueBrain
Copy link
Member Author

@TrueBrain TrueBrain commented Mar 16, 2021

Nice analysis!

I can think off two solutions:

  1. fix that we can render the game on a resolution of a height lower than 13.
  2. enforce the resolution to always be at least 13 in height.

I was sure we did 2), but clearly we do not :D
Not sure if 1) is useful at all. It is really silly to have such resolutions of course.

Possibly there are other solutions.

Loading

@nielsmh
Copy link
Contributor

@nielsmh nielsmh commented Mar 16, 2021

The game UI is designed for a minimum resolution of 640x480 so I think it would be appropriate to clamp the settings to that on load.

Loading

@James103
Copy link
Contributor

@James103 James103 commented Oct 10, 2021

I still get the same crash and assertion failure on 8b157c9 with the same reproduction steps.
crash.zip

Loading

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
3 participants