Skip to content

Minor performance improvement in BitmapByteQRCode + clarify padding calculation#698

Open
KrisVandermotten wants to merge 2 commits into
Shane32:masterfrom
KrisVandermotten:BitmapByteQRCode
Open

Minor performance improvement in BitmapByteQRCode + clarify padding calculation#698
KrisVandermotten wants to merge 2 commits into
Shane32:masterfrom
KrisVandermotten:BitmapByteQRCode

Conversation

@KrisVandermotten

@KrisVandermotten KrisVandermotten commented Jul 12, 2026

Copy link
Copy Markdown

Summary

This PR fixes/implements the following bugs/features:

Closing issues

Fixes #697

Summary by CodeRabbit

  • Bug Fixes
    • Improved QR bitmap generation across supported platforms for more consistent output.
    • Corrected bitmap row padding calculations to ensure proper byte alignment.
    • Enhanced binary integer writing on newer .NET Core versions to reliably produce the expected byte order.

@coderabbitai

coderabbitai Bot commented Jul 12, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Walkthrough

Bitmap QR generation now calculates BMP row padding from the row’s byte length and validates its alignment. Integer serialization uses BinaryPrimitives on supported .NET Core targets while retaining the existing fallback implementation.

Changes

Bitmap encoding

Layer / File(s) Summary
Row padding alignment
QRCoder/BitmapByteQRCode.cs
Computes padded row length with bitwise rounding and adds a debug assertion for valid 4-byte alignment.
Little-endian integer serialization
QRCoder/BitmapByteQRCode.cs
Adds the conditional binary primitives import and uses BinaryPrimitives.WriteInt32LittleEndian for supported .NET Core targets, with manual byte splitting retained elsewhere.

Estimated code review effort: 2 (Simple) | ~10 minutes

Suggested reviewers: gfoidl

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Linked Issues check ✅ Passed The row padding calculation is corrected in BitmapByteQRCode.GetGraphic as required by issue #697.
Out of Scope Changes check ✅ Passed No unrelated changes are evident; the added optimization and assertion stay within the BitmapByteQRCode fix.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately reflects the padding calculation fix and the small performance optimization in BitmapByteQRCode.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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

📥 Commits

Reviewing files that changed from the base of the PR and between 4b97eb1 and 348e0a9.

📒 Files selected for processing (1)
  • QRCoder/BitmapByteQRCode.cs

Comment thread QRCoder/BitmapByteQRCode.cs Outdated
@KrisVandermotten KrisVandermotten changed the title Fix bug and a minor performance improvement in BitmapByteQRCode Minor performance improvement in BitmapByteQRCode + clarify padding calculation Jul 12, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Bug in BMP padding calculation when pixelsPerModule is odd

1 participant