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

Feature/memory usage optimisation #35

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

Conversation

flasher297
Copy link
Contributor

@flasher297 flasher297 commented Oct 26, 2017

Memory optimisation fix.
In current implementation to get cropped image bitmap is uploaded to memory three times:
1st time - original imgae in tryLoadBitmap() method - by BitmapFactory.decodeStream(is, null, options);
2nd and 3rd - when we crop bitmap in applyCrop() method and whne we copy bytes from immutable cropped bitmap.

Usage of BitmapRegionDecoder allows us to load image to memory only once and in cropped size.
I've tested one image with constant crop area. Current implemetation allocates ~ 112MB. BitmapRegionDecoder implementation allocates 38mb.
Cropping time is slightly shorter.

memory_crop
On top of image current allocations. On the bottom of image's new version allocation.

flasher297 and others added 2 commits October 24, 2017 18:09
…RegionDecoder.

This allows to load image to memory only once and in cropped size.
@flasher297 flasher297 mentioned this pull request Oct 26, 2017
@romshiri
Copy link

romshiri commented Nov 9, 2017

Hey man, thanks for the fix.
Unfortunately, you have a bug there.

You call "ensureCorrectRotation" after the image was cropped, which leads to an incorrect result in case the image should have been corrected to the right rotation (you can reproduce it on Galaxy 8 for example, where the exif orientation is different than normal.
You should flip the image before you crop it.

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.

2 participants