optimise: fill QRCode with one colour
#626
Conversation
📝 WalkthroughWalkthroughPre-fills the entire bitmap with the Changes
Sequence Diagram(s)sequenceDiagram
participant Renderer as QRCode Renderer
participant Bitmap as Bitmap Canvas
participant Modules as QR Modules
Renderer->>Bitmap: Create bitmap
note right of Bitmap #f0f4ff: New step — pre-fill background
Renderer->>Bitmap: Fill entire background with lightColor
loop For each module
Renderer->>Modules: Is module dark?
alt Dark module
Renderer->>Bitmap: Draw darkColor rectangle at module position
else Light module
note right of Bitmap #f8f8f8: No-op — background already present
end
end
Renderer-->>Bitmap: Return rendered image
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Suggested reviewers
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
QRCoder/QRCode.cs (1)
98-145: Consider applying the same optimization to the icon-enabled overload.The icon-enabled
GetGraphicmethod at line 111 usesgfx.Clear(lightColor), which pre-fills the background similarly to line 66's new approach. However, lines 118-119 still conditionally draw both dark and light modules.Since the background is already filled with
lightColorat line 111, you could optimize lines 118-119 to only draw dark modules, consistent with the approach taken in the simpler overload (lines 73-76).Apply this diff to align the icon-enabled method with the optimization:
- var moduleBrush = QrCodeData.ModuleMatrix[(y + pixelsPerModule) / pixelsPerModule - 1][(x + pixelsPerModule) / pixelsPerModule - 1] ? darkBrush : lightBrush; - gfx.FillRectangle(moduleBrush, new Rectangle(x - offset, y - offset, pixelsPerModule, pixelsPerModule)); + var module = QrCodeData.ModuleMatrix[(y + pixelsPerModule) / pixelsPerModule - 1][(x + pixelsPerModule) / pixelsPerModule - 1]; + if (module) + { + gfx.FillRectangle(darkBrush, new Rectangle(x - offset, y - offset, pixelsPerModule, pixelsPerModule)); + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
QRCoder/QRCode.cs(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
QRCoder/QRCode.cs (1)
QRCoder/QRCodeGenerator/Rectangle.cs (1)
Rectangle(37-43)
🪛 GitHub Actions: Format (Pull Request)
QRCoder/QRCode.cs
[error] 66-66: WHITESPACE: Fix whitespace formatting. Insert '\s'.
[error] 66-66: IDE0055: Fix formatting
[warning] 1-1: Warnings were encountered while loading the workspace. Set the verbosity option to the 'diagnostic' level to log warnings.
🪛 GitHub Check: format
QRCoder/QRCode.cs
[failure] 66-66:
Fix formatting
[failure] 66-66:
Fix formatting
[failure] 66-66:
Fix formatting
[failure] 66-66:
Fix formatting
[failure] 66-66:
Fix formatting
[failure] 66-66:
Fix formatting
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
TimothyMakkison
force-pushed
the
fast_bitmap
branch
from
378540e to
da5c360
Compare
|
Try changing the benchmark project's target framework to Ref: |
|
FYI, it may be possible to create a single path for the dark modules, issuing a single Fill call, further improving performance. Just guessing, and not sure what the performance impact would be. |
Thanks 🙏
Yeah, I wanted to use benchmarks to test this. I'd also like to know if it's worth filling the bitmap with the light or dark colour or how much faster IIRC there was an issue with mutating a bitmap a lot in C#, I can't remember if it was related to I'll experiment more in the morning After
Before
I can see why #240 was created, half a second for a large QR code is crazy 0_0, not sure why the memory usage is so low for a huge bitmap |
2 participants
Fill bitmap with the light colour and then draw with the black, halving the number of updates.
Would love to benchmark this, but I can't get the benchmark file to use
System.Drawingto referenceQRCoder.QRCode. I've tried setting the .NET version of the benchmarks to6.0.0and referencingSystem.Drawing.Commonbut I can't referenceQRCoder.QRCodeEdit: I can do this in other places
Related #240
Summary by CodeRabbit