-
Notifications
You must be signed in to change notification settings - Fork 218
Save a PNG instead of JPEG #157
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
Conversation
Summary of ChangesHello @riggaroo, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request, authored by riggaroo, transitions the application's image saving mechanism from JPEG to PNG format. The changes involve updating the bitmap compression format, file extensions, and MIME types in Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
In this pull request, you've updated the image saving logic to use PNG instead of JPEG. This is a good change for preserving image quality. I've added a comment with a suggestion to improve maintainability by using constants for file extensions and MIME types. This will make it easier to manage the image format in the future. I also noticed that some test files still reference JPEG files, which you might want to update to keep your tests consistent with these changes.
| val cacheFile = localFileProvider.createCacheFile("shared_image_${UUID.randomUUID()}.png") | ||
| localFileProvider.saveBitmapToFile(imageBitmap, cacheFile) | ||
| return localFileProvider.sharingUriForFile(cacheFile) | ||
| } | ||
|
|
||
| override suspend fun saveImageToExternalStorage(imageBitmap: Bitmap): Uri { | ||
| val cacheFile = localFileProvider.createCacheFile("androidify_image_result_${UUID.randomUUID()}.jpg") | ||
| val cacheFile = localFileProvider.createCacheFile("androidify_image_result_${UUID.randomUUID()}.png") | ||
| localFileProvider.saveBitmapToFile(imageBitmap, cacheFile) | ||
| return localFileProvider.saveToSharedStorage(cacheFile, cacheFile.name, "image/jpeg") | ||
| return localFileProvider.saveToSharedStorage(cacheFile, cacheFile.name, "image/png") | ||
| } | ||
|
|
||
| override suspend fun saveImageToExternalStorage(imageUri: Uri): Uri { | ||
| return localFileProvider.saveUriToSharedStorage( | ||
| imageUri, | ||
| "androidify_image_original_${UUID.randomUUID()}.jpg", | ||
| "image/jpeg", | ||
| "androidify_image_original_${UUID.randomUUID()}.png", | ||
| "image/png", | ||
| ) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To improve maintainability and avoid repeating string literals, it would be beneficial to define constants for the file extension, MIME type, and filename prefixes. This will make future changes (e.g., switching to another image format) much easier.
You could add a companion object to the ImageGenerationRepositoryImpl class to hold these constants:
companion object {
private const val IMAGE_EXTENSION = ".png"
private const val IMAGE_MIME_TYPE = "image/png"
private const val SHARED_IMAGE_PREFIX = "shared_image_"
private const val RESULT_IMAGE_PREFIX = "androidify_image_result_"
private const val ORIGINAL_IMAGE_PREFIX = "androidify_image_original_"
}Then you can use these constants to construct the filenames and specify the MIME type across the class, for example:
- Line 110:
localFileProvider.createCacheFile("$SHARED_IMAGE_PREFIX${UUID.randomUUID()}$IMAGE_EXTENSION") - Line 116:
localFileProvider.createCacheFile("$RESULT_IMAGE_PREFIX${UUID.randomUUID()}$IMAGE_EXTENSION") - Line 118:
localFileProvider.saveToSharedStorage(cacheFile, cacheFile.name, IMAGE_MIME_TYPE) - Line 124:
"$ORIGINAL_IMAGE_PREFIX${UUID.randomUUID()}$IMAGE_EXTENSION" - Line 125:
IMAGE_MIME_TYPE
This change would centralize the image format configuration, making the code cleaner and easier to maintain.
Note: I noticed that test files like FakeImageGenerationRepository.kt and ResultsViewModelTest.kt still use .jpg extensions in hardcoded URIs. You might want to update them as part of this PR to ensure tests are consistent with this change.
No description provided.