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

Jump to bottom

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

Closed
TrueBrain opened this issue Mar 15, 2021 · 6 comments · Fixed by #10932
Closed

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

TrueBrain opened this issue Mar 15, 2021 · 6 comments · Fixed by #10932
Labels
bug Something isn't working priority: low This issue should be fixed but has either a low impact or is en edge-case

Comments

@TrueBrain
Copy link
Member

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 commented Mar 16, 2021 •
edited

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

@TrueBrain
Copy link
Member Author

TrueBrain commented Mar 16, 2021 •
edited 

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 :)

@TrueBrain TrueBrain added the bug Something isn't working label Mar 16, 2021
@James103
Copy link
Contributor

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

OpenTTD/src/window.cpp

Line 2127 in fe451b8

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:

OpenTTD/src/core/math_func.hpp

Line 79 in 9b800a9

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.

OpenTTD/src/window.cpp

Line 2058 in fe451b8

static const int MIN_VISIBLE_TITLE_BAR = 13;

@TrueBrain
Copy link
Member Author

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.

@nielsmh
Copy link
Contributor

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.

@James103
Copy link
Contributor

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

@TrueBrain TrueBrain added the priority: low This issue should be fixed but has either a low impact or is en edge-case label Oct 10, 2021
@James103 James103 mentioned this issue Oct 25, 2021
Closed
@LordAro LordAro mentioned this issue Mar 2, 2023
Closed
@TrueBrain TrueBrain mentioned this issue Jun 4, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug Something isn't working priority: low This issue should be fixed but has either a low impact or is en edge-case
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Fix: crash when window can't be placed on low resolution screens. TrueBrain/OpenTTD
3 participants
@nielsmh @TrueBrain @James103