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

Fix GLTF texture filename decoding #57685

Merged
merged 1 commit into from
Jul 3, 2022

Conversation

cesarizu
Copy link
Contributor

@cesarizu cesarizu commented Feb 5, 2022

This PR backports a specific fix to gltf filename decoding. GLTF files uri-encode references to external filenames. When importing into Godot, if the filename or a directory have any special characters, Godot fails to find the file. The solution is to decode the uri before looking for the file. On the master branch this is done using String::uri_decode but this is not available on 3.x. For these versions, and in this PR, I used String::percent_decode which covers most of the cases of uri encoding for GLFT files.

A more complete solution would be to backport the String::uri_(en|de)code methods and use those.

Test Project

gltf-tests.zip

Output without this PR:

Resource file not found: res://cube%2Btexture.png.
Can't open file from path 'res://cube%2Btexture.png'.
modules/gltf/gltf_document.cpp:3070 - glTF: Image index '0' couldn't be loaded as a buffer of MIME type 'image/png' from URI: res://cube%2Btexture.png. Skipping it.

@fire
Copy link
Member

fire commented Feb 6, 2022

There's another percent_decode missing that exists in Godot master 4.

@cesarizu
Copy link
Contributor Author

cesarizu commented Feb 6, 2022

I think percent_(en|de)code and http_(en|de)code (also missing in 4.x), are partial methods of encoding/decoding strings for URIs and neither do what uri_decode does completely. I think it would be better to backport the 4.x method than to restore the 3.x methods.

@fire
Copy link
Member

fire commented Feb 6, 2022

Oh I meant import has a flow and then export has one. I only saw one in this pr.

@akien-mga
Copy link
Member

Oh I meant import has a flow and then export has one. I only saw one in this pr.

@fire was right, there was another similar case in _parse_buffer that had to be handled.

I force pushed a rebase to include it, fix another typo, and switch to http_unescape which is the closer equivalent to uri_decode in 3.x (see #43978).

@akien-mga akien-mga force-pushed the gltf-texture-filename-decoding branch from 47b8060 to f66200e Compare July 3, 2022 01:03
Copy link
Member

@akien-mga akien-mga left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested with the attached project, it works fine 👍

@akien-mga akien-mga merged commit 2d54d3d into godotengine:3.x Jul 3, 2022
@akien-mga
Copy link
Member

Thanks!

@akien-mga
Copy link
Member

Cherry-picked for 3.4.5.

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

Successfully merging this pull request may close these issues.

4 participants