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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use triple slash in localUri of local assets #12428

Merged
merged 2 commits into from Apr 6, 2021

Conversation

tsapeta
Copy link
Member

@tsapeta tsapeta commented Apr 6, 2021

Why

Fixes https://linear.app/expo/issue/ENG-724/textures-not-loading-in-expo-gl-in-published-android-apps
Fixes #10906
Related #12101

How

Local assets returned by expo-updates have just one slash in their file URIs. expo-gl requires at least two, see below 馃憞

if (strncmp(localUri.c_str(), "file://", 7) != 0) {

expo-updates used file.toURI() which returns java.net.URI object that stringifies to file:/... URI, which is actually not correct on many platforms. Instead, we need to use Uri.fromFile(file) from android.net.Uri that returns file:///... URI.

Test Plan

Creating GL texture from the asset works and it renders correctly (tried some GL examples in published NCL). Also tested against some other examples using assets.

Checklist

  • Documentation is up to date to reflect these changes (eg: https://docs.expo.io and README.md).
  • This diff will work correctly for expo build (eg: updated @expo/xdl).
  • This diff will work correctly for expo prebuild & EAS Build (eg: updated a module plugin).

@tsapeta tsapeta marked this pull request as ready for review April 6, 2021 13:01
@tsapeta tsapeta requested a review from esamelson as a code owner April 6, 2021 13:01
@tsapeta tsapeta requested a review from brentvatne April 6, 2021 15:13
@brentvatne
Copy link
Member

do we need to do anything to backport this to sdk 41? we should ship this to the sdk 41 beta today or tomorrow so people can kick the tires

@tsapeta
Copy link
Member Author

tsapeta commented Apr 6, 2021

Nope, this part of expo-updates is not being versioned so it's gonna work on all supported SDKs. Am I right @esamelson?

Copy link
Contributor

@esamelson esamelson left a comment

Choose a reason for hiding this comment

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

yes, you are right @tsapeta -- this will work out of the box with all sdk versions!

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

Successfully merging this pull request may close these issues.

File asset localUri impedance mismatch between AssetUtils.resolveAsync and gl.texImage2D
3 participants