[BUG] - setOnCropImageCompleteListener does not work

[kingspride]

• Lib Version 3.3.6

Describe the bug
setOnCropImageCompleteListener does not work
Lambda function never gets called.

To Reproduce
use the CropImageView, then:
create a Lambda function:

binding.cropImageView.setOnCropImageCompleteListener{ view, result ->
    Log.d("imageComplete", "does this even get called")
}

notice that it never gets called when moving and scaling the crop region.

Expected behavior
Lambda should get called when releasing the cropping rectangle so that the selected region can be automatically refreshed into i.E. a Preview Drawable

Media

Smartphone (please complete the following information):

• Device: Xiaomi Mix 2S and Pixel 4 Emulator
• OS: Android 10 MIUI 12.5 and Android 10 with Play Services

[Canato]

Hey @kingspride thanks for the detailed issue, helps a lot to understand and be useful ^^

notice that it never gets called when moving and scaling the crop region.

It should only get called after you call to drop. While selecting the area, moving, scaling we are not cropping yet.
So the way method expectation is to be called after we call crop and after is complete, then we have a result in setOnCropImageCompleteListener.

To use crop from the cropImageView you need to call binding.cropImageView.croppedImageAsync()

Can check more in the sample code.

Obs: We have a strange shadow bug that I was trying to fix last weekend, not lucky yet ;)

[Canato]

I'm closing, since the behaviour looks ok from the text, let's reopen if need, let me know ^^

[kingspride]

@Canato
thanks for the clarification; but then I think the lib lacks an important feature.

I need to get the cropped image as soon as the user releases the cropping rectangle.

there's also a setOnSetCropOverlayReleasedListener which in principle does what I want, but unfortunately using this breaks the cropper's automatic zoom.
find a demo video here:
https://cloud.xtlk.de/index.php/s/wCPXQMMJKTSSQPS

maybe you want to look into that aswell.
also I dont know if you meant that by mentioning "Obs: We have a strange shadow bug that I was trying to fix last weekend, not lucky yet ;)"

otherwise, this is a really great lib. feels like a unix tool: does only one thing, but its very streamlined and good at that.

EDIT: heres the code snippet:

binding.cropImageView.setOnSetCropOverlayReleasedListener{
            binding.cardPreviewInclude.profile.setImageBitmap(binding.cropImageView.croppedImage?.let { it1 -> cropToSquare(it1) })
        }

[Canato]

This is what you mean by broke zoom?

This is what I mean here:

Obs: We have a strange shadow bug that I was trying to fix last weekend, not lucky yet ;)

If this is the bug, is not exactly related to the listener, but something else we need to fix, so you can use the setOnSetCropOverlayReleasedListener and hopefully we fix this bug soon, feel free to open a PR if you wanna fix faster ^^

[kingspride]

yes.
notice how the image doesnt zoom anymore when I reduce the cropping rectangle, while the rectangle does indeed resize itself. also the preview below shows the correctly zoomed portion.

If I dont use the Listener, its behaving normally.

[Canato]

Exactly, I can reproduce this some times on normal usage, so I'm sure is not related to this listener =|

But is some pain to fix this, since is not consistent, but maybe is consistent with setOnSetCropOverlayReleasedListener what will make easier to fix, I will try to check on Friday/Saturday @@

[kingspride]

... I just wanted to add that the zoom gets applied when dragging the cropping rectangle to an edge - maybe this helps finding the root cause ;)

[kingspride]

I just ran some debugging on this matter and I think the error lies somewhere here:

this is the state after I resized the cropping rectangle, and released it.

as you can see, the newZoom Value remains the same, but shouldnt it be different from mZoom ?

maybe this should be reopened.

[Canato]

as you can see, the newZoom Value remains the same, but shouldnt it be different from mZoom ?

On the line 1248 we set mZoom equal newZoom so is expected to have the same value.

Why you believe they should be different?

[kingspride]

Yeah I think its a misunderstanding. sorry.
I'll come back when I did more debugging.

[kingspride]

I think I found the cause of the shadow bug:
as long as you dont have getCroppedImage() or .croppedImage in the setOnSetCropOverlayReleasedListener, the bug stays away.
but if you call it, the image zoom animation gets canceled in getCroppedImage on line 553 in CropImageView.kt

also, if I would use croppedImageAsync() in combination with setOnCropImageCompleteListener, the shadow bug persists because startCropWorkerTask() clears the animation aswell. (line 961 in CropImageView.kt)

[Canato]

Will take a better look ASAP, but if you found consistency, let's open a PR and merge it =D

[kingspride]

I could verify now that the clearAnimation() is indeed the problem, but I wonder why its there in the first place... Maybe something left over from older android SDKs that isnt needed anymore?

I've tested on Android 6.0.1 (SDK23) on an old Moto G 3rd Gen, and it worked as expected.
(the snapdragon 410 is just ... SLOW, its lagging like hell)

If you are OK with it, I would open a PR and just remove the 3 occurences of imageView.clearAnimation() in CropImageView.kt

[Canato]

Please open, yeah big chance this was forgot in some part or wrong copied pasted. On the PR, please add the steps to reproduce

[kingspride]

ready to merge :)

[Canato]

@kingspride I'm having a little of problem finding time to test the ticket, but will soon XD. Thanks for the amazing work