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.

Sign up for GitHub

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

fix: prevent PNG to JPG conversion on image crop #17840

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
+38 −1
Open

fix: prevent PNG to JPG conversion on image crop #17840

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
cb7791e
fix: prevent PNG,WEBP to JPG conversion on image crop
Scapesfear Jan 17, 2025
32f548d
Using `ContentResolverUtil.getFileName()` .
Scapesfear Jan 18, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
39 changes: 38 additions & 1 deletion AnkiDroid/src/main/java/com/ichi2/imagecropper/ImageCropper.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,10 @@ package com.ichi2.imagecropper

import android.app.Activity
import android.content.Intent
import android.graphics.Bitmap
import android.graphics.Rect
import android.net.Uri
import android.os.Build
import android.os.Bundle
import android.os.Parcelable
import android.view.Menu
Expand All @@ -34,6 +36,7 @@ import androidx.fragment.app.Fragment
import com.canhub.cropper.CropImageView
import com.ichi2.anki.R
import com.ichi2.anki.snackbar.showSnackbar
import com.ichi2.utils.ContentResolverUtil
import kotlinx.parcelize.Parcelize
import timber.log.Timber

Expand Down Expand Up @@ -88,7 +91,13 @@ class ImageCropper :
when (menuItem.itemId) {
com.canhub.cropper.R.id.crop_image_menu_crop -> {
Timber.d("Crop image clicked")
cropImageView.croppedImageAsync()
val imageFormat = cropImageView.imageUri?.let { getImageCompressFormat(it) }
Timber.d("Compress format: $imageFormat")
if (imageFormat != null) {
cropImageView.croppedImageAsync(
saveCompressFormat = imageFormat,
)
}
true
}

Expand All @@ -114,6 +123,34 @@ class ImageCropper :
else -> false
}

private fun getImageCompressFormat(uri: Uri): Bitmap.CompressFormat {
Timber.d("Original image URI: $uri")

val fileExtension =
try {
ContentResolverUtil.getFileName(requireContext().contentResolver, uri).substringAfterLast('.')
} catch (e: Exception) {
Timber.w(e, "Failed to retrieve file extension from URI")
null
}

return when (fileExtension?.lowercase()) {
"png" -> Bitmap.CompressFormat.PNG
"jpeg", "jpg" -> Bitmap.CompressFormat.JPEG
"webp" -> {
if (Build.VERSION.SDK_INT >= 30) {
Bitmap.CompressFormat.WEBP_LOSSLESS
Copy link
Member

@mikehardy mikehardy Jan 17, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

having read all the associated discussion and docs just a bit ago, I think this (using WEBP_LOSSLESS alone vs WEBP_LOSSY or trying to use both) is the safest choice.

  • there is no good way to easily determine if a webp is lossless or lossy, so choosing one is best
  • there is not a huge expansion in size taken if a webp is lossless vs lossy, so choosing LOSSLESS as the single one to use does not have much of a downside but conservatively preserves quality, which respects the user

so I think this is a reasonable choice despite it seeming like there should be support for both of the new API30+ symbols (LOSSLESS and LOSSY)

} else {
Bitmap.CompressFormat.WEBP
}
}
else -> {
Timber.w("Unknown image format: $fileExtension. Defaulting to JPEG.")
Bitmap.CompressFormat.JPEG
}
}
}

private fun onSetImageUriComplete(
@Suppress("UNUSED_PARAMETER") view: CropImageView,
@Suppress("UNUSED_PARAMETER") uri: Uri,
Expand Down
Loading