flightlessmango / MangoHud

A Vulkan and OpenGL overlay for monitoring FPS, temperatures, CPU/GPU load and more. Discord: https://discordapp.com/invite/Gj5YmBb
MIT License
6.39k stars 282 forks source link

the updgrade to imgui-1.89.9 crashes 32bit apps if "font_size" is set to some values #1142

Open aur-r opened 1 year ago

aur-r commented 1 year ago

Setting "font_size" option to some values in MangoHud.config when running a 32bit app like vkcube, glxspheres on a 64-bit system crashes the app. This never happened to 64bit apps, only to 32bit ones and only for some specific values, in my case, between 20 and 23. Some 32bit wine applications didn't crash thou, even with these values. Also, as a workaround, not setting "font_size" in config file seems to work without issues.

Steps to reproduce the behavior:

  1. Add/Set "font_size=22" in $HOME/.config/MangoHud/MangoHud.conf
  2. run 32bit versions of vkcube/glxspheres: mangohud glxspheres
  3. See errors like: glxspheres: ../subprojects/imgui-1.89.9/imstb_truetype.h:3160: void stbtt__fill_active_edges_new(float*, float*, int, stbtt__active_edge*, float): Assertiondy >= 0' failed. Aborted (core dumped)`

Expected behavior Successful run.

flightlessmango commented 1 year ago

unable to repro this issue Can you compile mangohud with debug and get a gdb backtrace of the crash?

aur-r commented 1 year ago

Here it is:

gdb_bt_vkcube_mangohud.txt

aur-r commented 1 year ago

Attached my conf file, maybe it has some particularities that trigger this. MangoHud.txt

flightlessmango commented 1 year ago

The config was helpful in debugging the issue, thanks! I found 2 different issues that I've attempted to fix here b32e6b299cec1cc2dc4498630902c755e4bef86c and here 8bda2a2a04d279cd02fc3c688b04d51eda67ece9 Does it resolve your issue?

aur-r commented 1 year ago

Unfortunately not, the crash still happens in the same way.

flightlessmango commented 12 months ago

Back to the start then, still unable to repro this crash

aur-r commented 12 months ago

If it is of any help, a regression test showed that the last version of imgui that behaves properly is 1.87 . I'm zooming in on the last working commit ...

flightlessmango commented 12 months ago

We can't revert imgui as we need new features in it

aur-r commented 12 months ago

I was thinking of having a clue why is behaving like this when running 32bit on 64bit systems .

flightlessmango commented 12 months ago

does removing the config prevent the crash? And if so, can you narrow down which option causes it?

aur-r commented 12 months ago

Setting "font_size" to some values, like 20 to 23 or 30 and some others, crashes the app, but works with others. If "font_size" is not specified, it works fine too.

flightlessmango commented 12 months ago

Just to be clear, you've tried with only font_size?

aur-r commented 12 months ago

Only when font_size is set in the config file the crash happens, otherwise no crash.

aur-r commented 12 months ago

OK, the first commit that trigger this is https://github.com/ocornut/imgui/commit/0f14933577a1de01d90f8e87622296c466146f21

The commit looks kinda cumbersome, but it's no surprise that specific font_size values triggers the crash.

flightlessmango commented 12 months ago

I wonder if this could be an imgui bug. I guess next step would be to build a simple imgui demo without any mangohud bits and see if we can repro it

aur-r commented 12 months ago

I'm inclined to believe it's an imgui bug or it may be some peculiar settings on my system as I use the default distro setting to build a rpm package for mangohud . Unfortunately, I have no idea how to start an imgui demo test. Will look into their examples but most likely this is way above my programming abilities.

flightlessmango commented 12 months ago

If you clone imgui there's ready made examples, using example_glfw_opengl3 as an example, you would just need to uncomment io.Fonts->AddFontFromFileTTF("../../misc/fonts/DroidSans.ttf", 16.0f); and change the font size 16.0f. Then you can compile and run

flightlessmango commented 12 months ago

Oh yeah and also make sure to compile 32bit CXX="g++ -m32" make

aur-r commented 12 months ago

Thanks for hints, however, I tested font sizes between 15 and 32 and no crashes happened. This may be a bit more complex than their basic examples as the commit deals with automatic resizing of interdependent instances of different heights.

aur-r commented 12 months ago

I'm sorry, I messed up the regression tests and wasted your time. Anyhow, the real culprit seems to be this commit: imgui_commit_eefc9035 That imgui memory optimization seems to cause all this trouble in my case. It makes no sense to me, the wrongly identified commit made more sense. Reverting, i.e. setting OversampleH = 3 back, seems to solve my issues completely. So, mangohud with patched imgui 1.89.9 runs fine with 32bit apps again.

flightlessmango commented 12 months ago

Does this resolve it? fae6035c200a587862cfbaf731deec08aaff86d2

aur-r commented 12 months ago

Well, spoke too soon, it still crashes, but less often and, for the more sane values( 12-30 ) for font size, it behaves better now.

Here is a list of font sizes(up to 75 only) that crashes the apps before/after the patch. B: 2,4,5,7,9,10,11,13,15,20-23,28-30,33, 40-48,50, 55-61, 65, 70,74,75 A: 10, 28,30, 36,38, 45,48-50,52,53,55,59,60, 69,70, 75

aur-r commented 11 months ago

The troubling font size issue seems to start with this commit: imgui-v1.87-27-g0cff5ac5 , i.e. the update of imstb_truetype.h from 1.20 to 1.26 . This is consistent now with the crash backtrace. So, as a dirty test, I've disabled assert lines( lines #3160, #3161 in imstb_truetype.h ) STBTT_assert(dy >= 0); STBTT_assert(dx >= 0); and the crashes don't happen anymore, also I haven't noticed any other side effects from these changes, though, my tests are limited.

BTW, the latest commits broke "Display Custom System information" section from my config file MangoHud.txt, Latest commit that I used and worked is commit dbdc295.

gort818 commented 11 months ago

@aur-r I think I broke your Display Custom System information sorry! i will get a fix out for that fixed it in latest.

aur-r commented 11 months ago

@aur-r I think I broke your Display Custom System information sorry! i will get a fix out for that fixed it in latest.

@gort818 commit 7d51113 fixed it, all good, thanks.