Minor performance improvement in BitmapByteQRCode + clarify padding calculation#698
Conversation
📝 WalkthroughWalkthroughBitmap QR generation now calculates BMP row padding from the row’s byte length and validates its alignment. Integer serialization uses ChangesBitmap encoding
Estimated code review effort: 2 (Simple) | ~10 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@QRCoder/BitmapByteQRCode.cs`:
- Around line 75-78: Update the padding calculation in the BitmapByteQRCode
row-building logic to use the 24-bit row byte count, 3 * sideLength, rather than
sideLength itself; revise the Debug.Assert invariant to validate that byte count
plus padding is divisible by 4, and add regression coverage with an odd
pixelsPerModule value such as 3.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: bf0cb601-4c81-4763-8114-7ed94d213c46
📒 Files selected for processing (1)
QRCoder/BitmapByteQRCode.cs
Note: An extensive analysis of the old and new calculation code by ChatGPT determined that it is mathematically equivalent for 24bpp images, but the new calculation better represents the intent, and is more correct if the data wasn't 24bpp. |
Summary
This PR fixes/implements the following bugs/features:
pixelsPerModuleis odd.Closing issues
Fixes #697
Summary by CodeRabbit