ArthurHub / Android-Image-Cropper

Image Cropping Library for Android, optimized for Camera / Gallery.
Apache License 2.0
6.4k stars 1.38k forks source link

Cropbox getting dislocated #99

Closed mshukla19 closed 8 years ago

mshukla19 commented 8 years ago

I am facing a strange problem. I am using this in a viewpager which has different fragments. Each fragment has a crop imageview. After I set the cropbox, i save the value of croppoints in a variable. When i move in this pattern between fragments 0->1->2->1->0 then 0th fragment doesn't show cropimageview correctly and box is dislocated. I am using this code in OnViewCreated ->

public void onBitmapLoaded(Bitmap bitmap, Picasso.LoadedFrom from) {
mProgressBar.setVisibility(View.GONE);
cropImageView.setVisibility(View.VISIBLE);
cropImageView.setImageResource(bitmap);
                if (answerModel.isFilled()) {
                    cropImageView.setCropRect(answerModel.getRect());
                    lockBoundingBox();
                } else {
                    unlockBoundingBox();
                }
            }

and to save the croppoints in answermodel,

answerModel.addRect(cropImageView.getCropRect());

Can you help me solve this?

ArthurHub commented 8 years ago

this scenario is too complex without a sample project. if you create a simple project that reproduces this issue I will look into it. (preferably on github)

mshukla19 commented 8 years ago

@ArthurHub thank you for the quick reply. I will try to make a sample project. Till then, If you could help me with some common pitfalls that i should look into, it would be great.

ArthurHub commented 8 years ago

I honestly don't know, it may be a bug in the library.

mshukla19 commented 8 years ago

Here it is - https://github.com/mshukla19/android-ViewPagerWithFragmentPagerAdapter Steps for reproducing - 1) Modify box on first page - click on save box - slide to next page 2) Modify box on second page - click on save box - slide to next page 3) Modify box on third page - click on save box - slide to next page 4) Move back to 3rd and 2nd page. And then 1st page either 2nd or 1st page show inconsistent boxes.

mshukla19 commented 8 years ago

@ArthurHub just to add a bit more information. I have figured out the cropImageview gets disturbed if onPause of the fragment is called. I am using the setCropRect method in onResume but it still doesn't work properly. In some cases, the imageview also moves a little bit. Maybe a problem with Onlayout/initcropwindow?

ArthurHub commented 8 years ago

It looks like the problem is with state save/restore. state is restored after the onCreateView so it overwrites the rectangle set in onCreateView. Also Android reuses CropImageView so if you change the crop window on box 1 and move to box 4 you will see the it will restore box 1 crop window. I'm trying to see if I can disable state without providing explicit configuration in the library.

mshukla19 commented 8 years ago

@ArthurHub if you want i can post a gif here to explain a bit better :)

ArthurHub commented 8 years ago

sure

mshukla19 commented 8 years ago

Here you go - ezgif-1875333090

Notice the first box gets larger when i come back to first fragment

ArthurHub commented 8 years ago

fixed the issue by preventing state restore if already set, so

  1. Make sure you use latest version (2.2.3), it should just work when you use 2.2.+ in gradle.
  2. cropImageView.setImageResource(R.drawable.xyz) should be before cropImageView.setCropRect(rect1) as rectangle cannot be set before there is image to crop.
mshukla19 commented 8 years ago

Thank you. Not able to test right now. Will test it asap. :) On 17-Jun-2016 7:52 PM, "Arthur" notifications@github.com wrote:

fixed the issue by preventing state restore if already set, so

  1. Make sure you use latest version (2.2.3), it should just work when you use 2.2.+ in gradle.
  2. cropImageView.setImageResource(R.drawable.xyz) should be before cropImageView.setCropRect(rect1) as rectangle cannot be set before there is image to crop.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/ArthurHub/Android-Image-Cropper/issues/99#issuecomment-226782136, or mute the thread https://github.com/notifications/unsubscribe/AHPg3nL6y5ZyPJS7IMoIkHj33u6GW5_yks5qMq2agaJpZM4I4PJH .

mshukla19 commented 8 years ago

@ArthurHub checked it right now. Still facing the same problem. I think the problem is related to recycling of the crop image view. Is there a way i can disable this. And I am setting the rectangle after setting the image bitmap in Picasso's onBitmaploaded. BTW my viewpager's off screen limit it set to default (i.e. 1).

ArthurHub commented 8 years ago

Using your test app: ezgif com-optimize

not sure what I'm missing...

mshukla19 commented 8 years ago

Thank you for taking out time for this. Maybe try doing it a bit fast/ a few more times. With new 2.2.3 (same issue) Device MOTO X Play, Android 6- https://giphy.com/gifs/LCRByYInM9Quk

ArthurHub commented 8 years ago

I will try more tomorrow (GMT+2) not sure I understand the issue looking at your gif though...

mshukla19 commented 8 years ago

yeah sure. It's the same issue. I draw a box on 0,1,2 and then go back to 0. The box is dislocated as well as the image. During some testing, I have become pretty confident that it is happening because the cropimageview is being recycled. Another thing to note is, these placeholder are of same size, in my actual app the images are of varying sizes and I am using adjustViewBounds = true

mshukla19 commented 8 years ago

Bump. Did you get time to check this?

ArthurHub commented 8 years ago

sorry, not yet, hope to tackle it in the next few days.

ArthurHub commented 8 years ago

Went over it again, sorry for the delay, busy with job stuff.

I can't reproduce corruption issue moving fast from 0 to 2-4 and back again. The UX feels very clunky because the pager adapter constantly destroys and re-creates the test fragment. So if I were you I would try:

  1. Change the UX into something else that doesn't require multiple live fragments with crop UI.
  2. Manually re-use the same fragment and not letting the adapter destroy/re-create them.
  3. Caching the bitmaps objects, it is one of the pain points as depending on image size in can be expensive (in the test app loading the large image from resource hurts really bad)

Without the constant stress of creating the fragment and loading the image for cropping you will probably see much better UX.

amitseth1000 commented 4 years ago

I could fix this dislocation of rectangle issue by switching off and switching on AutoZoom. following is sample code myicon.setAutoZoomEnabled(false); myicon.setImageBitmap(myBitmap); myicon.setAutoZoomEnabled(true); myicon.setCropRect(new Rect(0,0,CropSize,CropSize));