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.

Sign up for GitHub

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

Jump to bottom

Scale “display” text appears misleading. #650

Closed
ashiagr opened this issue May 4, 2020 · 2 comments · May be fixed by #651
Closed

Scale “display” text appears misleading. #650

ashiagr opened this issue May 4, 2020 · 2 comments · May be fixed by #651

Comments

@ashiagr
Copy link

ashiagr commented May 4, 2020 •
edited

Do you want to request a feature or report a bug?
bug

What is the current behavior?
The scale “display” text currently appears misleading.

What is the expected behavior?
The scale text should ideally display the scale in which the image is going to be saved.

Steps to reproduce

  1. Load a square image - size 2000x2000 in the sample app in the original aspect ratio.
  2. Notice that the image entirely fits into the crop rectangle on the scale tab.
  3. Notice that the scale “display” percent is not 100% .
  4. When you save this image, it is saved in original dimension 2000x2000.

While the image is saved in correct dimension, displayed scale percent might be confusing to end-users as it gives an impression that the image is likely scaled down, the scale "display" text not being 100%.

Please attach any image files, URL and stack trace that can be used to reproduce the bug.
device-2020-05-04-114546

Which versions of uCrop, and which Android API versions are affected by this issue?
uCrop v2.2.5
@ashiagr ashiagr mentioned this issue May 4, 2020
Open
@dmitriy1morozov
Copy link
Contributor

@ashiagr , thank you for the PR. Sorry for such a huge delay in response. I reviewed your code and found one place for improvement. It is better not to show the fractional part in scaleView at all. Due to the floating-point calculations, it is possible to scale to 99.99% sometimes which seems to be a bug. I would suggest using Math.round() and showing only the resulting integer value as a scale.

@dmitriy1morozov
Copy link
Contributor

To be fixed in #651
@ashiagr please continue this thread in the PR so we will merge it eventually.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Fix scale “display” percent ashiagr/uCrop
2 participants
@ashiagr @dmitriy1morozov