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] - Null bitmap provided to OnCropImageCompleteListener #355

Closed
marekoid opened this issue Apr 20, 2022 · 15 comments · Fixed by #509
Closed

[BUG] - Null bitmap provided to OnCropImageCompleteListener #355

marekoid opened this issue Apr 20, 2022 · 15 comments · Fixed by #509

Comments

@marekoid
Copy link

  • Lib Version [e.g. 3.3.4-4.2.1]

Describe the bug
I've upgraded the library and now I'm getting null bitmap in CropResult provided to the listener set using setOnCropImageCompleteListener. Interestingly isSuccessful is true and error is null.

The project is using CropImageView integration. The only change I needed to do when updating was to change getCroppedImageAsync() to croppedImageAsync().

The project is using Java 8, had issues upgrading it to Java 11. The files integrating with the library are in Kotlin. Not sure if it's not Java 8 issue but I would expect any such issues to be more likely build-time issues, not such an odd runtime issue of bitmap being null right at the end of the flow.

To Reproduce
Steps to reproduce the behavior:

  1. Use view integration
  2. setOnCropImageCompleteListener
  3. setImageBitmap
  4. croppedImageAsync()

Expected behavior
cropResult.bitmap provided to the listener should not be null

Please help/advise.

@marekoid marekoid added the bug label Apr 20, 2022
@Canato
Copy link
Member

Canato commented Apr 20, 2022

Hey @marekoid thanks for the issue, will need your help to understand some details...

Lib Version [e.g. 3.3.4-4.2.1]

Which version of the library are you? [e.g. 3.3.4-4.2.1] means for example from version 3.3.4 to version 4.2.1 I need more objective information like Lib Version 4.2.1 so I know which version of the code to look

I've upgraded the library

From which version to which version?

I'm getting null bitmap in CropResult provided to the listener set using setOnCropImageCompleteListener

Could you please share this code you are using? I know can be redundant, but helps =)

The project is using Java 8, had issues upgrading it to Java 11.

Not sure what I can do here for you exactly. Still having issues?

Not sure if it's not Java 8 issue but I would expect any such issues to be more likely build-time issues, not such an odd runtime issue of bitmap being null right at the end of the flow.

You need to use Java 11 since version 3.3.4, the wrong Java version can bring unexpected behaviours.

I try to reproduce using the CropImageView Sample code we have in the library but was not successful from this steps, could you give me more insights how can I reproduce?

Sorry not been very useful so far, but we will get there =D

@marekoid
Copy link
Author

Thanks @Canato for your quick reply!

It works on 3.2.2, but doesn't work on 3.3.4. I have also checked the latest version (4.2.1) and it doesn't work there too so I assume that any version above 3.2.2 won't work.

imageView.setOnCropImageCompleteListener { view: CropImageView, result: CropResult ->
  val bitmap = result.bitmap
  // bitmap will be null...
}
imageView.setImageBitmap(bitmap)
imageView.croppedImageAsync()

Why Java 11 is needed since 3.3.4? Do you reckon that using Java 8 instead could result in such an odd issue of bitmap being null? I would expect it to be more likely a build-time issue.

@Canato
Copy link
Member

Canato commented Apr 22, 2022

Why Java 11 is needed since 3.3.4?

We update to Java 11 so we could update Gradle. And is always good to keep the most updated versions available. Those who do not update, keep using old versions of the library, but those who update can get the benefits of the latest versions.

Do you reckon that using Java 8 instead could result in such an odd issue of bitmap being null? I would expect it to be more likely a build-time issue.

Maybe, unexpected behaviours hahaha. Not sure how the file handling happens at the bottom of Java code.

I would suggest sticking with 3.2.2 for now if you don't have the time to update to Java 11. If there is a bug fix or something you need in the latest versions you can branch from 3.2.2 do the changes in a PR and we have a second main branch to release 3.2.3 for example

@marekoid
Copy link
Author

Thanks for your quick reply @Canato! I played a bit with it and it's not Java 11 related issue. I created the most basic sample project that showcases the issue. It's Java 11, and bitmap passed to the callback is null. It does work when downgrading to 3.2.2. Please check.

@marekoid
Copy link
Author

I see that Result is never created with a bitmap... so that clearly looks like a regression? Also I've noticed that it's reporting the following exception, which I don't see in the real project I'm working on (over there isSuccessful=true and error=null is reported) so the sample may not be entirely representative (it's still odd that it returns non-null bitmap on 3.2.2):

java.lang.IllegalArgumentException: width must be > 0
	at android.graphics.Bitmap.checkWidthHeight(Bitmap.java:450)
	at android.graphics.Bitmap.createBitmap(Bitmap.java:874)
	at com.canhub.cropper.BitmapUtils.cropBitmapObjectWithScale(BitmapUtils.kt:218)
	at com.canhub.cropper.BitmapUtils.cropBitmapObjectHandleOOM(BitmapUtils.kt:160)
	at com.canhub.cropper.BitmapCroppingWorkerJob$start$1.invokeSuspend(BitmapCroppingWorkerJob.kt:65)
	at kotlin.coroutines.jvm.internal.BaseContinuationImpl.resumeWith(ContinuationImpl.kt:33)
	at kotlinx.coroutines.DispatchedTask.run(DispatchedTask.kt:106)
	at kotlinx.coroutines.scheduling.CoroutineScheduler.runSafely(CoroutineScheduler.kt:571)
	at kotlinx.coroutines.scheduling.CoroutineScheduler$Worker.executeTask(CoroutineScheduler.kt:750)
	at kotlinx.coroutines.scheduling.CoroutineScheduler$Worker.runWorker(CoroutineScheduler.kt:678)
	at kotlinx.coroutines.scheduling.CoroutineScheduler$Worker.run(CoroutineScheduler.kt:665)

@marekoid
Copy link
Author

Maybe the sample was too basic. The view was not yet measured when I requested immediately croppedImageAsync() and perhaps that caused the error. Now added OK button to trigger croppedImageAsync() and I'm getting the same outcome as in the real project I'm working on. That's what I see after calling croppedImageAsync(), the last 3 log entries are properties of CropImageView.CropResult received in the callback:

I/AIC: Try get URI for scope storage - content://
D/bitmap: null
D/error: null
D/isSuccessful: true

@Canato
Copy link
Member

Canato commented Apr 26, 2022

Hey, @marekoid sorry for the long time to reply these days... going crazy with some work-related issues.

Your issue looks correct, for some reason setImageBitmap(bitmap) is not working.
Worth some investigation and opening a PR to fix this.

Plus, would be great for a small sample of its usage of it, so we won't allow this to be broken again in the future.

Will pin and tag this issue in case someone wanna help and take ownership of it

@Canato
Copy link
Member

Canato commented Apr 26, 2022

@marekoid ok! I understood the problem and found the issue.

Instead of trying to retrieve result.bitmap please use result.uriContent or result.getUriFilePath(..) depending if you wanna file:// or content:// schema.

This was done because a bitmap is too huge/big to pass as a result in the android intent. But for sure we need to improve this contract.

@marekoid
Copy link
Author

This was done because a bitmap is too huge/big to pass as a result in the android intent.

It's view integration, not activity one, so binder transaction size limits shouldn't be relevant here.

But for sure we need to improve this contract.

Do you mean that bitmap integration won't be supported any more? Currently it's present both in the README and the code API.

Instead of trying to retrieve result.bitmap please use result.uriContent or result.getUriFilePath(..)

I was indeed thinking about it but it's legacy code which would need a bigger overhaul to support bitmap reading/decoding/rendering which really should happen asynchronously off the main thread. The library was providing all of it with the loading indicator in the ImageCropperView and the work done on a background thread. I finally just updated to the latest version with bitmap integration working as I unfortunately couldn't spend more time on it.

@Canato
Copy link
Member

Canato commented May 3, 2022

Sorry that I couldn't help early =/

Still need to spend some time understanding how to better improve these. If you have a clue please feel free to drop a PR, will help your case and many others =D

@Canato
Copy link
Member

Canato commented May 9, 2022

I'm rewriting the documentation and planning to check this bug ASAP... putting this here cause may help someone or my self in the future when checking the bug
val cropped: Bitmap = cropImageView.getCroppedImage()

@vanniktech
Copy link
Contributor

@marekoid do you still have this issue or were you able to work around it?

@vanniktech vanniktech added needs-info and removed bug labels Oct 24, 2022
@SidoPillai
Copy link

SidoPillai commented Oct 26, 2022

@vanniktech @Canato I'm having this issue as well,

Try get URI for scope storage - content://

fun openGalleryForProfileImage(
    context: Context,
    activity: ManagedActivityResultLauncher<CropImageContractOptions, CropImageView.CropResult>,
) {
    activity.launch(
        options {
            setCropShape(CropImageView.CropShape.OVAL)
            setScaleType(CropImageView.ScaleType.FIT_CENTER)
            setGuidelines(CropImageView.Guidelines.ON)
            setAspectRatio(1, 1)
            setMaxZoom(4)
            setActivityTitle(UiText.StringResource(com.core_ui.R.string.edit_profile_image_cropper_title).asString(context))
            setCropMenuCropButtonIcon(R.drawable.baseline_done_24)
            setAllowFlipping(false)
            setAllowRotation(false)
            setOutputCompressFormat(Bitmap.CompressFormat.JPEG)
            setOutputCompressQuality(90)
            setMinCropResultSize(99999, 99999)
            setMaxCropResultSize(99999, 99999)
            setAutoZoomEnabled(true)
            setMultiTouchEnabled(false)
            setSnapRadius(3f)
            setTouchRadius(48f)
            setInitialCropWindowPaddingRatio(0.1f)
            setBorderLineThickness(3f)
            setFlipVertically(false)
            setImageSource(
                includeGallery = true,
                includeCamera = false,
            )
        }
    )
}
    val profileCoverFromGalleryCropLauncher = rememberLauncherForActivityResult(CropImageContract()) { result ->
        if (result.isSuccessful) {
            println("success")
        }
     }
openGalleryForProfileCover(context = context, activity = profileCoverFromGalleryCropLauncher)

@vanniktech
Copy link
Contributor

@siddeshpillai can you modify the sample app of this repository in order to create a reproducible issue? If so, I can have a look at it!

@marekoid
Copy link
Author

I created a basic sample that reproduced the issue and linked to it in that comment.

Addressing the earlier question, it didn't feel like worth looking for a work around really, just didn't upgrade beyond the version that has introduced the regression.

I'm no loner working on the project were the bug prevented the library upgrade so will be unsubscribing from this thread but definitely if feels like something worth fixing for the active users of the library.

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

Successfully merging a pull request may close this issue.

4 participants