godotengine / godot

Godot Engine – Multi-platform 2D and 3D game engine
https://godotengine.org
MIT License
90.44k stars 21.07k forks source link

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

Closed m4gr3d closed 3 months ago

m4gr3d commented 4 months ago

Tested versions

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

Minimal reproduction project (MRP)

starter-kit-city-builder-mrp.zip

m4gr3d commented 4 months ago

cc @clayjohn @BastiaanOlij

rburing commented 4 months ago

Can you check if a MultiMesh has the same issue?

matheusmdx commented 4 months ago

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 commented 4 months ago

CC @KoBeWi

I wonder if the only reason bisect points to https://github.com/godotengine/godot/pull/88645 is that it exposed the crash that was hidden before.

We should test the commit directly before https://github.com/godotengine/godot/pull/69032 was merged and see if we can reproduce the crash. If not, the crash may have been introduced between https://github.com/godotengine/godot/pull/69032 and https://github.com/godotengine/godot/pull/88645 and was just hidden

matheusmdx commented 4 months ago

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 commented 4 months ago

@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 commented 3 months ago

@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 commented 3 months ago

@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.