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

[Android Editor] Gridmap doesn't render and causes crash when renderer is set to compatibility #92910

Closed
m4gr3d opened this issue Jun 8, 2024 · 8 comments

Comments

@m4gr3d
Copy link
Contributor

m4gr3d commented Jun 8, 2024

Tested versions

  • Reproducible in Godot 4.3 beta 1 Android Editor
  • Not reproducible in Godot 4.2.2 stable Android Editor

System information

Android 12, Android 14

Issue description

When the renderer is set to compatibility, gridmap nodes in the Godot Android Editor v4.3 beta 1 don't render and cause the editor to crash.
From looking at profiling data, it looks like the editor keeps allocating memory in an attempt to render the gridmap until it crashes.

Here's a recorded heap profiler data: https://drive.google.com/file/d/1vC9a9WKFRVWiWP5EnnnuGK7O8sfUH8Ob/view?usp=sharing

The issue doesn't occur in the Godot Android Editor v4.2.2 stable.

Steps to reproduce

  • Download or build the Godot Android Editor v4.3 beta 1
  • Load the attached mrp project (based off Kenney's city builder starter kit with the renderer set to compatibility).
  • Once the project is opened, notice that the gridmap doesn't render. Zooming in-out will cause the editor to crash because of unbounded memory allocations.

Minimal reproduction project (MRP)

starter-kit-city-builder-mrp.zip

@m4gr3d
Copy link
Contributor Author

m4gr3d commented Jun 8, 2024

cc @clayjohn @BastiaanOlij

@rburing
Copy link
Member

rburing commented Jun 8, 2024

Can you check if a MultiMesh has the same issue?

@matheusmdx
Copy link
Contributor

matheusmdx commented Jun 9, 2024

Bisecting points to #88645 as the culprit:

image


Edit: I tried to reproduce the crash using emulators and wasn't able to reproduce this on bluestacks 5 or android studio emulator, only happened on my redmi 7 phone

@clayjohn
Copy link
Member

clayjohn commented Jun 19, 2024

CC @KoBeWi

I wonder if the only reason bisect points to #88645 is that it exposed the crash that was hidden before.

We should test the commit directly before #69032 was merged and see if we can reproduce the crash. If not, the crash may have been introduced between #69032 and #88645 and was just hidden

@matheusmdx
Copy link
Contributor

matheusmdx commented Jun 21, 2024

I tested the beta 2 and the issue was fixed there, so i did a bisect to found where was fixed and points to pr #93060.

Captura 2024-06-20 16-49-22-092913


Reading the pr details i saw a line talking about a msaa memory leak and i remembered here also talks about a memory leak, so i checked the mrp settings and i saw the mrp uses 3d msaa 2x, so i tested the mrp again in beta 1 but when i disabled the 3d msaa the bug stops, editor render everything again and zoom in-out doesn't crash the engine.

Also you can reproduce the bug with an empty 3d scene, just needs the 3d msaa actived, so maybe that was the hidden bug?

@clayjohn
Copy link
Member

@matheusmdx thank you for the excellent investigation. That indeed sounds like the hidden bug I was describing.

@m4gr3d can you test again to confirm that the crash is gone with beta 2? Just to confirm we aren't dealing with 2 different crashes here

@clayjohn
Copy link
Member

@m4gr3d I just tested locally using your MRP on a custom dev build of 4.3 beta 1 on a Pixel 4 and could not reproduce the crash.

When you have a minute, please test again with Beta 2 and confirm whether this has been fixed

@m4gr3d
Copy link
Contributor Author

m4gr3d commented Jun 26, 2024

@matheusmdx thank you for the excellent investigation. That indeed sounds like the hidden bug I was describing.

@m4gr3d can you test again to confirm that the crash is gone with beta 2? Just to confirm we aren't dealing with 2 different crashes here

Tested on beta 2 and the issue is indeed fixed! Closing this issue.

@m4gr3d I just tested locally using your MRP on a custom dev build of 4.3 beta 1 on a Pixel 4 and could not reproduce the crash.

@clayjohn In order to repro the issue, you also had to change the renderer to compatibility (it's mobile by default), and ensure that MSAA 3D is enabled.

@m4gr3d m4gr3d closed this as completed Jun 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Release Blocker
Development

No branches or pull requests

4 participants