Improve image downscaling using progressive steps to reduce artifacts. #3069
Conversation
…ying light patterns.
|
Can one of the admins verify this patch? |
📝 WalkthroughWalkthroughThis change introduces a new private static method, Sequence Diagram(s)sequenceDiagram
participant Caller
participant FileUtil
participant Bitmap
Caller->>FileUtil: getBitmapScaledByMaxDimen(originalBitmap, maxDimen)
FileUtil->>FileUtil: Calculate targetWidth, targetHeight
FileUtil->>FileUtil: stepDownscale(originalBitmap, targetWidth, targetHeight)
loop While dimensions > target
FileUtil->>Bitmap: Scale bitmap to half size
end
FileUtil->>Bitmap: Final scale to targetWidth, targetHeight
FileUtil-->>Caller: Return downscaled bitmap
Possibly related issues
Suggested labels
Suggested reviewers
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🔇 Additional comments (6)
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
|
@rahulrajesh21 Thanks for submitting this, can you fill this up for us to take next steps here. |
|
@shubham1g5 I have filled out the form. |
|
@damagatchi ok to test |
shubham1g5
left a comment
shubham1g5
left a comment
There was a problem hiding this comment.
@rahulrajesh21 The code here looks good to me, though I am a little worried about performance implications of step down sizing, can you provide a rough timing of how long it takes to perform stepDownscale for the sample image you provided in comparison to direct one step scaling.
|
@shubham1g5 for sample image (4080 x 3060 pixels ) |
shubham1g5
left a comment
shubham1g5
left a comment
There was a problem hiding this comment.
@rahulrajesh21 Thanks for putting out those timings, I think that's something we should be able to live with. Left one more question for you where it would be nice to get your thoughts on.
|
|
||
| // Final step to exactly match target dimensions | ||
| if (width != targetWidth || height != targetHeight) { | ||
| Bitmap finalBitmap = Bitmap.createScaledBitmap(currentBitmap, targetWidth, targetHeight, false); |
There was a problem hiding this comment.
I would be curious on your reasoning of putting bilinear filtering as false here and true above while wondering if it should be false even above or true even here ?
There was a problem hiding this comment.
@shubham1g5 Bilinear filtering is set as true to prevent visual artifacts at the cost of image quality. For the final step it is kept false to persevere image quality and reduce performance impact.
There was a problem hiding this comment.
Can you elaborate more on what do you mean by preventing visual artifacts and why does it not matter in the final step when it matters in the intermediary steps ?
There was a problem hiding this comment.
@shubham1g5 Visual artifacts like moiré patterns and stippling occur primarily when downsampling at large ratios. Bilinear filtering averages neighboring pixels, which helps smooth out these artifacts.
When reducing dimensions by large factors, the sampling grid can align improperly with high-frequency details in the original image. This causes interference patterns - similar to strange patterns in digital photos of striped fabrics. In intermediary steps, bilinear filtering helps by averaging pixels, softening these problematic interference patterns.
In the final step, these artifacts matter less because:
- The scaling ratio is much smaller.
- The major frequency mismatches that cause interference have already been addressed in previous steps.
- When sampling distance is smaller, the likelihood of problematic frequency interactions is reduced.
In the final step, bilinear filtering could also be kept true, but may reduce image quality and impact performance. So it is kept false to preserve sharpness and detail in the final image while maintaining better performance.
There was a problem hiding this comment.
that's insightful, thanks for the detailed explanation here.
3 participants
/claim #2742
Product Description
Improves image downscaling quality in CommCare by implementing a progressive step-wise scaling approach. This reduces visual artifacts like amplifying light patterns when resizing large images.
Technical Summary
Ticket: #2742
• The original implementation of direct downscaling introduced visual artifacts in images with fine patterns. This change introduces progressive halving to preserve image quality.
• DownScaling is done in multiple halving steps followed by a final scale, instead of a single step.
Image used for testing
Before Implementation (stippled appearance):
After Implementation (improved appearance):
Feature Flag
Safety Assurance
Safety story
Automated test coverage
QA Plan
Labels and Review