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

Regression: iOS images loaded from filesystem don't respect scaling factor in filename #22383

Closed
ascherkus opened this issue Nov 22, 2018 · 10 comments
Labels
Bug Component: Image Impact: Regression Describes a behavior that used to work on a prior release, but stopped working recently. Platform: iOS iOS applications. Resolution: Locked This issue was locked by the bot.

Comments

@ascherkus
Copy link
Contributor

Environment

React Native Environment Info:
System:
OS: macOS High Sierra 10.13.6
CPU: (8) x64 Intel(R) Core(TM) i7-8559U CPU @ 2.70GHz
Memory: 206.29 MB / 16.00 GB
Shell: 3.2.57 - /bin/bash
Binaries:
Node: 10.9.0 - /src/homebrew/bin/node
Yarn: 1.9.4 - /src/homebrew/bin/yarn
npm: 6.2.0 - /src/homebrew/bin/npm
SDKs:
iOS SDK:
Platforms: iOS 12.0, macOS 10.14, tvOS 12.0, watchOS 5.0
Android SDK:
API Levels: 28
Build Tools: 28.0.2
IDEs:
Android Studio: 3.1 AI-173.4907809
Xcode: 10.0/10A255 - /usr/bin/xcodebuild
npmPackages:
react: 16.6.1 => 16.6.1
react-native: 0.57.5 => 0.57.5

Description

Image assets loaded from the filesystem don't respect iOS's image scaling naming convention.

This appears to be a regression of functionality introduced by 998197f where [UIImage imageWithContentsOfFile:filePath]; was replaced by [UIImage imageWithData:fileData];, but that leaves UIKit unable to inspect the filePath for the presence of @3x filename suffixes.

Reverting that change locally fixes the issue.

Where this really comes into play is in projects using CodePush where the image assets are loaded from the filesystem (e.g., file:///.../Library/Application%20Support/CodePush/path/to/Image@3x.png) vs. the bundle included with the application at build time (e.g., file:///var/containers/Bundle/Application/.../path/to/Image@3x.png) where the filename-based constructor [UIImage imageNamed:imageName]; is used.

Reproducible Demo

CodePush bugs contain references to sample repros:
microsoft/react-native-code-push#1222
microsoft/react-native-code-push#1276

Let me know if you need any help setting up a repro... although at a minimum if the desired functionality is to always respect iOS's image scaling filename convention then I feel it should be fairly safe to revert (a portion of) the original change.

@MichielDeMey
Copy link

This is a legit issue, we're currently unable to add CodePush support to our app due to this limitation. 😔

@jvt
Copy link

jvt commented Jan 17, 2019

Agreed, same as @MichielDeMey, we're also unable to implement CodePush into our application due to it loading the incorrect images for various screen resolutions.

@tbergquist-godaddy
Copy link

We are also having this issue.

This is how it looks in development:
image

And this is how it looks with code-push
image

@ScreamZ
Copy link

ScreamZ commented Jan 31, 2019

Facing something similar with Look like i've the same issue here ? Any facebook guy has a solution ?

@hramos hramos removed the Bug Report label Feb 6, 2019
@zhongwuzw
Copy link
Contributor

Hi @ScreamZ @tbergq @ascherkus, can you guys test wether this can fix ?

@tbergquist-godaddy
Copy link

@zhongwuzw, I can confirm that your PR fixes the issue 👍

@ScreamZ
Copy link

ScreamZ commented Feb 22, 2019

Release version is ?

@holgersindbaek
Copy link

Same question as @ScreamZ ?

@danielgreane
Copy link

I have just tested the codepush bug described in the latest stable release, 0.58.6, and the problem still persists

Assuming this was merged into the 0.59rc which is not yet stable?

grabbou pushed a commit that referenced this issue Mar 11, 2019
Summary:
Regression, fix image load from `~/Library` not respect scale factor.
Fixes #22383 , the bug comes from [Clean up some URL path handling](998197f).

[iOS] [Fixed] - Fix image wrong scale factor when load image from file system
Pull Request resolved: #23446

Differential Revision: D14099614

Pulled By: cpojer

fbshipit-source-id: eb2267b195a05eb70cdc4671536a4c1d47fb03e2
@prashantvv
Copy link

prashantvv commented Mar 18, 2019

this belongs to react native itself. When I add a image to marker, its size was small while in release apk/ipa its size was bigger (3x). Then i finally added a small image than what i need, but then the image quality became poor.

@facebook facebook locked as resolved and limited conversation to collaborators Feb 15, 2020
@react-native-bot react-native-bot added the Resolution: Locked This issue was locked by the bot. label Feb 15, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Bug Component: Image Impact: Regression Describes a behavior that used to work on a prior release, but stopped working recently. Platform: iOS iOS applications. Resolution: Locked This issue was locked by the bot.
Projects
None yet
Development

Successfully merging a pull request may close this issue.