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

[HOLD for payment 2021-12-13][$500] Image is blurry and not rendered in correct resolution in mobile devices #5193

Closed
isagoico opened this issue Sep 10, 2021 · 43 comments
Assignees
Labels
Awaiting Payment Auto-added when associated PR is deployed to production Engineering External Added to denote the issue can be worked on by a contributor Weekly KSv2

Comments

@isagoico
Copy link

isagoico commented Sep 10, 2021

If you haven’t already, check out our contributing guidelines for onboarding and email [email protected] to request to join our Slack channel!


Action Performed:

  1. Navigate to a conversation
  2. Send this image
  3. Tap on the image and zoom

Expected Result:

Image should display the same resolution it was sent.

Actual Result:

Image is blurry.

Workaround:

Can the user still use Expensify without this being fixed? Have you informed them of the workaround?

Platform:

Where is this issue confirmed?

  • iOS
  • Android

Version Number: 1.0.96-0

Logs: https://stackoverflow.com/c/expensify/questions/4856

Notes/Photos/Videos: Any additional supporting documentation

image

image

Expensify/Expensify Issue URL:

Issue reported by: @PrashantMangukiya
Slack conversation: https://expensify.slack.com/archives/C01GTK53T8Q/p1631206579123200

View all open jobs on GitHub

@MelvinBot
Copy link

Triggered auto assignment to @chiragsalian (Engineering), see https://stackoverflow.com/c/expensify/questions/4319 for more details.

@chiragsalian
Copy link
Contributor

Looking into this today

@MelvinBot MelvinBot removed the Overdue label Sep 13, 2021
@chiragsalian
Copy link
Contributor

Okay for starters I just tested on regular images and they looked fine. Might be something just specific to the sample image as its very tall. I don't think this is daily since regular images appear just fine and this is quite an edge case with the sample image provided.
Either way, i'm testing the sample image next.

@chiragsalian
Copy link
Contributor

Trying to figure out how this works. Either way i dont think its a daily since its a pretty edge case scenario. Demoting to weekly for now as i continue to investigate.

@chiragsalian chiragsalian added Weekly KSv2 and removed Daily KSv2 labels Sep 14, 2021
@chiragsalian
Copy link
Contributor

From what i can tell the image obtained from the server is the correct size, its just when we scale it down here we seem to loose image quality. We could always load an image with 100% width as the parent and allow the height to be scrolled. This could solve the problem but i'm unsure if react-native-image-pan-zoom allows for that.

Anyway since this is purely an issue with the front end code i'll place the External label on it.

@chiragsalian chiragsalian added the External Added to denote the issue can be worked on by a contributor label Sep 20, 2021
@MelvinBot
Copy link

Triggered auto assignment to @NicMendonca (External), see https://stackoverflow.com/c/expensify/questions/8582 for more details.

@NicMendonca
Copy link
Contributor

still on hold

@MelvinBot MelvinBot removed the Overdue label Oct 4, 2021
@kadiealexander
Copy link
Contributor

Please refer to this post for updated information on the n6 hold, we've raised the bonus to $250 for all issues where a PR is created before the N6 hold is lifted.

@aswin-s
Copy link
Contributor

aswin-s commented Nov 21, 2021

To further clarify my observations, these are the proposed fixes.

  1. Just apply the fix mentioned here which will solve the blurred image issue across iOS & Android platform. But this fix is limited to images below resolutions 2048 x 2048 px on Android.

  2. In addition to above fix, to support high resolution images (above 2048px) on Android, replace built in Image component with FastImage and measure image dimensions using react-native-image-size.

@iwiznia
Copy link
Contributor

iwiznia commented Nov 22, 2021

1 sounds good, I am not sure what 2 really entails. Is there anything specifically better/worse on using that new library?
Also asked here to see if more people have opinions on this.

@aswin-s
Copy link
Contributor

aswin-s commented Nov 22, 2021

I was just pointing out that if we go ahead with option 1, extra long images like the one mentioned in OP (image) will continue to look blurred in attachment view. It has a resolution of 875px x 14894px, which is way above the 2048px limit.

@iwiznia
Copy link
Contributor

iwiznia commented Nov 23, 2021

Yeah I got that, 2 is needed only to support large images. Seems like a good solution, so let's try that out 👍

@NicMendonca
Copy link
Contributor

@aswin-s sorry! The old job post expired. Sent you an offer here.

@aswin-s
Copy link
Contributor

aswin-s commented Nov 23, 2021

@NicMendonca Reapplied .

@NicMendonca
Copy link
Contributor

^ waiting deploy to production

@MelvinBot MelvinBot removed the Overdue label Dec 1, 2021
@botify botify added Weekly KSv2 Awaiting Payment Auto-added when associated PR is deployed to production and removed Weekly KSv2 labels Dec 6, 2021
@botify botify changed the title [$500] Image is blurry and not rendered in correct resolution in mobile devices [HOLD for payment 2021-12-13] [$500] Image is blurry and not rendered in correct resolution in mobile devices Dec 6, 2021
@botify
Copy link

botify commented Dec 6, 2021

The solution for this issue has been 🚀 deployed to production 🚀 in version 1.1.17-7 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

If no regressions arise, payment will be issued on 2021-12-13. 🎊

@roryabraham
Copy link
Contributor

roryabraham commented Dec 6, 2021

@roryabraham roryabraham changed the title [HOLD for payment 2021-12-13] [$500] Image is blurry and not rendered in correct resolution in mobile devices [$500] Image is blurry and not rendered in correct resolution in mobile devices Dec 6, 2021
@roryabraham
Copy link
Contributor

@NicMendonca We're tracking the regression here. It must be fixed before this issue can be paid out.

@aswin-s
Copy link
Contributor

aswin-s commented Dec 7, 2021

@roryabraham Linked issue is unrelated to blurred image bug. Should I do something about it? What's the next step here?

@roryabraham
Copy link
Contributor

So sorry @aswin-s, I got confused. The linked regression is related to #6326, not this issue. My mistake. Nothing more for you to do here.

@roryabraham roryabraham changed the title [$500] Image is blurry and not rendered in correct resolution in mobile devices [HOLD for payment 2021-12-13][$500] Image is blurry and not rendered in correct resolution in mobile devices Dec 10, 2021
@NicMendonca
Copy link
Contributor

@aswin-s @PrashantMangukiya paid!

@valery-lavrik
Copy link

valery-lavrik commented Aug 11, 2022

I also searched for a solution to this problem for a long time, and finally decided to modify react-native-fast-image a little and added the preloadDimension method to it, which does the same as getSize only returns the real size of the image. I also use the same library to show the image, which solves the problem of quality reduction

https://github.com/valery-lavrik/react-native-fast-image

And so that the application does not crash on very large pictures, I added the useHardwareTextureAndroid=true parameter to zoom

@parasharrajat
Copy link
Member

@valery-lavrik This issue is already resolved and marked as closed.

@valery-lavrik
Copy link

@parasharrajat I just shared my decision, maybe it will be useful to someone. It's much simpler

@parasharrajat
Copy link
Member

Ok. Thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Awaiting Payment Auto-added when associated PR is deployed to production Engineering External Added to denote the issue can be worked on by a contributor Weekly KSv2
Projects
None yet
Development

No branches or pull requests