CompositorEffect in Mobile renderer throws "Image needs the TEXTURE_USAGE_STORAGE_BIT usage flag"

manuelmaceira commented 2 months ago

Tested versions

Reproducibe in: v4.3.stable

System information

Godot v4.3.stable unknown - Arch Linux #1 SMP PREEMPT_DYNAMIC Wed, 04 Sep 2024 15:16:37 +0000 - Tty - Vulkan (Mobile) - dedicated NVIDIA GeForce RTX 3060 Laptop GPU - 11th Gen Intel(R) Core(TM) i7-11800H @ 2.30GHz (16 Threads)

Issue description

I'm trying to use compositor effects in godot 4.3 using the mobile renderer, according to the docs it should work (https://docs.godotengine.org/en/stable/tutorials/rendering/compositor.html first note says "only supported by the Mobile and Forward+ renderers"). I've followed the tutorial in the docs and i've implemented a simple invert color effect for testing, it works with the Forward+ renderer but it breaks when using Mobile.

The errors printed to console are:

Image (binding: 0, index 0) needs the TEXTURE_USAGE_STORAGE_BIT usage flag set in order to be used as uniform.
servers/rendering/renderer_rd/uniform_set_cache_rd.h:130 - Condition "rid.is_null()" is true. Returning: rid
servers/rendering/rendering_device.cpp:4392 - Parameter "uniform_set" is null.
Uniforms were never supplied for set (0) at the time of drawing, which are required by the pipeline.

Steps to reproduce

Follow the tutorial from the docs using the Mobile renderer.

Minimal reproduction project (MRP)

https://github.com/manuelmaceira/MRP-CompositorEffectsMobile

BastiaanOlij commented 3 weeks ago

@DarioSamo @clayjohn I didn't think of this, we generally don't set things up for compute shaders in the mobile renderer so indeed the storage flags aren't set on most internal buffers. I don't think it's safe to just add this flag as it may cause issues on hardware that doesn't support compute fully but maybe you guys can verify?

Else we need to make sure that we add information into the docs that if you want to use compositor effect on mobile, that you can only use raster based shaders, not compute shaders.

Or we could make this an setting? Where you can enable setting the storage bit on render buffers in Mobile but we add in a warning that if you enable this, your project may not work on older hardware?

clayjohn commented 3 weeks ago

The biggest problem with the storage flag isn't compatibility, it's performance. I think it's supported on all the formats that we use internally. On every platform that uses framebuffer compression the storage flag force disables it. For mobile this leaves a huge amount of performance on the table.

We need to remove the flag wherever we can. Which is going to increase the need for a system for users to request the flag where they need it.

We probably need something internally as well so we can avoid requesting the copy flags as well if they won't be needed.

godotengine / godot