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

Implement internal cropping instead of library's built-in function #401

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

terofeev
Copy link

@terofeev terofeev commented Feb 7, 2025

What is it?

  • Bugfix
  • Feature
  • Codebase improvement

Description of the changes in your PR

  • Fix saving an unnecessary copy when cropping and resizing a picture.
  • Reuse the library's built-in progress bar to indicate the cropping process.

Before/After Screenshots/Screen Record

  • Before:
Before.mp4
  • After:
After.mp4

Fixes the following issue(s)

Acknowledgement

@naveensingh naveensingh self-assigned this Feb 7, 2025
@naveensingh
Copy link
Member

naveensingh commented Feb 7, 2025

Thanks for the PR!

@terofeev
Copy link
Author

terofeev commented Feb 8, 2025

Thanks for your comment!
I've fixed the share option.

I reviewed the "recommended" solution by the developer, but it comes with a few drawbacks:

  • We still create at least one temporary image.
  • The share option always generates its own copy via getTempImagePath, storing it in the app’s cache/images directory.
  • This approach unnecessarily consumes device resources like CPU, storage, etc.

So, in a new commit, I've completely switched to using our own cropping method without saving temporary images.

@naveensingh
Copy link
Member

Okay, thanks!

I'll do some final testing and merge soon.

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.

After edit picture, a copy is saved in app data
2 participants