godotengine / godot

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

Godot attempting to load BPTC textures on Android #74451

Closed Malcolmnixon closed 1 year ago

Malcolmnixon commented 1 year ago

Godot version

Godot 4.0 Stable

System information

Windows 11, Vulkan Mobile, NVidia RTX 3070 TI - targeting Quest 2

Issue description

When running the Godot-XR-Tools demo project natively on the Quest, the main scene fails to load because the resource loader tries to load the BPTC version of the texture, even though the ASTC version was bundled into the APK.

Unable to open file: res://.godot/imported/SkyOnlyHDRI023_2K-TONEMAPPED.jpg-bcb5464e5ed42ee2463b71652f907f38.bptc.ctex.

Steps to reproduce

Build the latest godot-xr-tools demo project (https://github.com/GodotVR/godot-xr-tools) targeting the Quest.

The project has both S3TC/BPTC and ETC2/ASTC texture compression formats enabled.

The .import files mention both the S3TC/BPTC and ETC2/ASTC formats; however when exporting for Quest 2 (an Android target) Godot automatically enables the astc and etc2 features and bundles only the ASTC and ETC2 textures into the APK.

At startup the resource importer reads the .import files, sees S3TC/BPTC as the first enabled texture, sees that S3TC/BPTC is supported on the platform, and picks that format - totally ignoring what was actually bundled into the installer.

Minimal reproduction project

Build the latest godot-xr-tools demo project (https://github.com/GodotVR/godot-xr-tools) targeting the Quest.

Malcolmnixon commented 1 year ago

This seems related to a similar issue in July 2022 (https://github.com/godotengine/godot/pull/62909), and in that case a "lazy" fix was applied which just lied about the supported capabilities of the operating system to trick the resource loader into only loading the desired formats.

clayjohn commented 1 year ago

A few of us discussed this on Roacketchat and we think we know what the issue is and how to resolve it.

The Issue

At export time the exporter is removing the BPTC and S3TC files as it should. But it is not removing the reference to the S3TC and BPTC files from the import metadata. At load time, the engine checks the graphics driver to see if the format is supported, if it is, it will try to load it.

Since there is a mismatch between what we export and what the GPU supports it ends up trying to load a non-existent resource.

Accordingly, there are two parts to the problem:

  1. Export formats are too rigid for mobile (i.e. we force ETC2/ASTC at export time even though some Android devices support S3TC/BPTC)
  2. The import metadata isn't correctly being amended at export time (i.e. the references to BPTC/S3TC files are left behind)

In 3.x we solved this problem by forcibly disabling reporting S3TC from the OpenGL driver side (I don't know why we didn't run into issues with BPTC).

The solution

First we need to change the exporter settings. On PC we have checkboxes for BPTC, S3TC, ETC, and ASTC. They need to be changed to only have 2 checkboxes S3TC/BPTC and ETC2/ASTC. We need to add these checkboxes for Android and remove the hardcoded options for ETC2 and ASTC.

Second, we need to figure out why the import metadata maintains reference to BPTC at export time and change it to respect the export checkboxes added above.

akien-mga commented 1 year ago

First we need to change the exporter settings. On PC we have checkboxes for BPTC, S3TC, ETC, and ASTC. They need to be changed to only have 2 checkboxes S3TC/BPTC and ETC2/ASTC. We need to add these checkboxes for Android and remove the hardcoded options for ETC2 and ASTC.

For the record, according to @reduz it sounds like we should instead remove all configuration options for PC, and just stick to:

Malcolmnixon commented 1 year ago

We still have the "lazy" fix code present for S3TC in the gles3 driver, and this could be expanded to cover BPTC: https://github.com/godotengine/godot/blob/dac2d8fb4214d3394bfb33b28f35f6d1e61518a8/drivers/gles3/storage/config.cpp#L80-L87

It looks like we have the equivalent code on the Vulkan side for S3TC and could be expanded to cover BPTC: https://github.com/godotengine/godot/blob/dac2d8fb4214d3394bfb33b28f35f6d1e61518a8/servers/rendering/renderer_rd/storage_rd/utilities.cpp#L272-L283

clayjohn commented 1 year ago

I think I have a solution for this. Will post a PR soon. Reduz was right. The code that exports the .import files was just copying them over as-is thought regard to the file types that were being trimmed at export time. The solution is to edit the exported copy of the .import file to only include the texture types that are being exported.