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

[BUG] - When using setImageBitmap, rotating screen and calling setImageBitmap again, the crop window will be incorrectly restored #68

Closed
chriscoomber opened this issue Feb 25, 2021 · 0 comments · Fixed by #69

Comments

@chriscoomber
Copy link
Contributor

  • Lib Version [e.g. 2.1.0]

Describe the bug

They say an image is a thousand words. How about a video?

bug.mp4

To Reproduce

  1. Write a fragment whose layout file contains a CropImageView which does simply the following:

    • in onActivityCreated, kick off some work in the background to produce a bitmap whose width is the same as the width of the CropImageView (which is smaller in portrait mode than in landscape mode) and whose height is twice its with. When it's done, on the main thread call cropImageView.setImageBitmap(theBitmap). (As long as you have images of different dimensions across the two cases, you will see this issue.)
  2. Run the app in portrait mode and navigate to the fragment. Notice that the crop rectangle is defaulted to cover most of the image.

  3. Rotate the device to landscape mode. Notice that the old crop rectangle has been restored, which is way smaller than what the default crop rectangle should be.

Expected behavior
Since I called cropImageView.setImageBitmap to a potentially arbitrary unrelated bitmap, it should not restore the old image's crop rectangle.

Screenshots
See video above

Smartphone (please complete the following information):

  • Device: Samsung S20FE
  • OS: Android 10

Additional context
I would like to be in charge of how the CropImageView's state is restored. I have my own system which renders a bitmap image from JPEG file or from PDF page, and subsamples according to the size of the view it needs to fill. Everytime the fragment is recreated after a configuration change, I expect to be able to set up the CropImageView completely fresh, and restore it's state myself using my business logic. I don't need it to restore state.

Setting a new bitmap image should be enough to reset the CropImageView's state, including state remembered from before it was recreated.

It looks like this is a very simple fix. Consider the difference between setImageUriAsync and setImageBitmap. In the former, the following code is run:

      clearImageInt();
      mRestoreCropWindowRect = null;
      mRestoreDegreesRotated = 0;
      mCropOverlayView.setInitialCropWindowRect(null);

However, in the latter, only the following is run:

      mCropOverlayView.setInitialCropWindowRect(null);
      [...]
      clearImageInt();

In particular mRestoreCropWindowRect and mRestoreDegreesRotated are never cleared. Then, later on, in onLayout, once there is an image available these kick in and incorrectly restore the old window rect and degrees rotated.

I think the restoring code is fine to live in onLayout, since you often want to wait until your view has been measured before doing some logic. However, we definitely should be clearing the old remembered state when setting a new bitmap image.

Otherwise, there should be a way to clear the saved instance state, or a flag that tells the CropImageView not to bother with managing its state.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants