-
-
Notifications
You must be signed in to change notification settings - Fork 21.3k
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
Crash in DynamicFontAtSize::_find_texture_pos_for_glyph #55394
Comments
CC @bruvzg |
It seems to be crashing on this line: https://github.com/godotengine/godot/blob/3.4/scene/resources/dynamic_font.cpp#L445 It's the first write after allocating new memory for an image, which is consistent with the error:
The host OS is running out of memory and couldn't allocate the image. Either the image is too big, or the game really exhausted all available memory for the process. What's weird though is that there's an error condition before writing which seems to aim at catching such memory resize failure: godot/scene/resources/dynamic_font.cpp Line 439 in 8f0208a
|
Failed It's probably some sort of memory corruption, stack trace might be damaged and inaccurate, or something is written over the cowdata size value. None of the DynamicFont changes seems to relevant and affect this part of code. |
We should make errors more verbose, print argument values if it's possible, timestamps, and maybe add an option to print stack trace for each error. |
See godotengine/godot-proposals#963 and #43826. It needs an OS-specific implementation. |
So, after investigating further, the 150 size font was triggering DynamicFontAtSize to create a 2048x2048 texture of 8mb. The font also had an outline which caused it to allocate another 2048x2048 texture. This might've been enough to push an older gpu over the edge. So to fix we just set a smaller font size, then scaled the label up. With filtering it looks almost as good as the 150 font. So not sure if godot could've caught the failed alloc and avoid crashing, or if this issue should just be closed. |
Font sizes used to be clamped in the inspector to avoid this, but I suppose we should probably add a hard clamp to a size of 128 or so. (If you need to draw even larger fonts while keeping them crisp, MSDFs are a good way to achieve this in |
I'm not sure about this, that seems fairly limiting. A better solution IMO would be for the rasterized fonts to be split in multiple textures if the total size exceed 2048px. So you can still write "Hi!" in size 256 which fits in a single texture, but a bigger sentence would be split. Does that make sense @bruvzg? |
That's how dynamic font is working right now. Currently, single texture size is limited to But I'm not sure if it is the real reason for the crash. Failure to allocate GPU memory should not cause a crash. And 16 MB is not much even for old GPUs. Also, godot/scene/resources/dynamic_font.cpp Lines 550 to 557 in c2470f5
|
Godot version
3.4 custom build
System information
Windows 10, GLES2
Issue description
We updated our games last week from godot 3.3.4 to 3.4. Since then we've had several crashes reported by users to sentry.io, so I believe this is a new crash in 3.4 that wasn't there before. I have not been able to reproduce the crash.
This crash may be related to: #55395
It is crashing adding a scene. The scene contains a full screen ViewportContainer that contains a Viewport with a 3d Scene. It also has a label on top of everything that has a size 150 font, use Mipmaps is off.
Here is the raw call stack from sentry.io
Here is the godot.log:
Steps to reproduce
I have not been able to reproduce. About 200 users have updated to the new version and several of them have crashed.
The game is Hardwood Hearts and can be downloaded here: https://www.hardwoodgames.com/beta/
The crash happens when you shoot the moon, which means to take all the point cards, it's kind of hard to do though.
Minimal reproduction project
I'm hoping the problem can be tracked down with the callstack. If needed I can create a small project with a similar scene setup
The text was updated successfully, but these errors were encountered: