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

Crash when setting a sky's radiance size to value > 512 #35552

Closed
RodZill4 opened this issue Jan 25, 2020 · 15 comments
Closed

Crash when setting a sky's radiance size to value > 512 #35552

RodZill4 opened this issue Jan 25, 2020 · 15 comments

Comments

@RodZill4
Copy link
Contributor

Godot version:
3.2RC3 and RC2 crash (3.1.2 works fine)

OS/device including version:
Windows 10
OpenGL ES 3.0 Renderer: GeForce GTX 960M (and GeForce GTX 660/PCIe/SSE2)

Issue description:
Setting the radiance size of a ProceduralSky or PanoramaSky in the editor crashes Godot.

Steps to reproduce:

  • Create a basic 3D scene with a WorldEnvironment containing a ProceduralSky
  • Set radiance size to 2048
    -> Crash

Minimal reproduction project:
radiance_size_crash.zip

@timothyqiu
Copy link
Member

On my machine, this freezed the system for around seven seconds (I can't do things like switching windows or adjusting the volume), and then some artifacts showed up in the 3D viewport for a very short time. But the editor finally recovered.

screenshot

macOS 10.15.2 / 16 GB RAM / Intel Iris Graphics 6100 1536 MB

@clayjohn
Copy link
Member

clayjohn commented Jan 25, 2020

As I have explained before, this is not a bug, but a limitation of your hardware. It is unable to handle the amount of computations needed for such a large radiance map.

The solution is to use a smaller radiance size. There is very little visual difference between a size of 256 and 2048, IMO you should never set radiance_size higher than that.

The higher options are there for people with higher-end graphics cards and extreme corner cases where you may need a larger radiance size.

See this comment for a comparison RodZill4/material-maker#73 (comment)

As you can see, there is almost no visual difference between 256 and 1024.

@RodZill4
Copy link
Contributor Author

This worked perfectly in 3.1.2 with the same hardware. The difference is noticeable if you replace the sphere with a plane.

Anyway, is there a way to detect the maximum radiance size that does not cause a crash ?

@clayjohn
Copy link
Member

No. Because it is an issue with your hardware not being able to process complex computations. We can't tell the exact limits of your hardware in advance.

@Zireael07
Copy link
Contributor

If it's an issue with computations, why did it work beforehand with the same hardware?

Also, maybe worth adding a warning when setting to > 256, since there is "almost no visual difference" as evidenced by the screenshots?

@clayjohn
Copy link
Member

@Zireael07 Because before we didnt generate irradiance at all and our radiance maps were always low-quality.

As for a warning, see #35560.

@clayjohn
Copy link
Member

@RodZill4
Copy link
Contributor Author

@clayjohn Thanks a lot for the build !
It is a lot better: 1024 works perfectly, but 2048 still crashes (on my old GeForce GTX 660). Quality is good with 1024, so this change solves my problem. Thanks !

@clayjohn
Copy link
Member

Closed by #35561

@jamie-pate
Copy link
Contributor

jamie-pate commented May 20, 2021

Has anyone filed a bug with intel on this? It could be this: https://gitlab.freedesktop.org/drm/intel/-/issues/2887
radiance_repro.zip for the linux driver.

In linux this will crash your entire x11 desktop

@akien-mga
Copy link
Member

#48906 should help by unexposing the 1024 and 2048 options (which I confirmed trigger crashes for me on Linux with Intel HD 630, Mesa 21.1.1).

512 is still exposed as it's something that makes sense for higher end hardware. 1024 and 2048 were just too costly for the minimal gain they provide in quality.

As for the crash, I don't know if it can be categorized as a driver bug. Godot is asking too much of the GPU and the GPU fails hard... we don't know how to query relevant capacities beforehand with OpenGL to prevent asking too much of the GPU. Vulkan solves this with much better metrics on the physical device and supported features.

@Calinou
Copy link
Member

Calinou commented May 20, 2021

we don't know how to query relevant capacities beforehand with OpenGL to prevent asking too much of the GPU. Vulkan solves this with much better metrics on the physical device and supported features.

In a way similar to godotengine/godot-proposals#1609, we could cap the radiance size to 256 on all Intel IGPs (and print a warning when doing so). This is kind of a blanket solution, but it'd probably work well enough in practice. In fact, this can probably already be implemented in 3.x.

@jamie-pate
Copy link
Contributor

Godot is asking too much of the GPU and the GPU fails hard...

I think once the entire Windows session crashes it's a bug in the driver. As part of the OS one of its responsibilities is to keep the system stable in the face of this kind of adverse condition even if the context and the app is not salvageable... I guess it's in Intel's hands now as I've provided an easy reproduction on the above issue.

I think that hard limiting all Intel gpus may be the best way to protect users and developers in the mean time but i can understand the reluctance to add this kind of limit and detection in too many places.

@clayjohn
Copy link
Member

@jamie-pate limiting features on a per-vender basis is certainly on our long term wishlist.

@Calinou
Copy link
Member

Calinou commented May 21, 2021

I got a X11 freeze when setting the Radiance Size to 2048, but this time on the master branch with a high-end GPU (GeForce GTX 1080). This means the issue is not limited to OpenGL somehow…

Should we also remove the radiance sizes above 512 in the master branch?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

7 participants