fix: dispose SKPaint and SKBitmap in renderer icon/dot helpers#86
Conversation
Co-Authored-By: Afonso Dutra Nogueira Filho <afonsoft@gmail.com>
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
e8e1e3c
into
feature/devin-20260712-sonar-quality
|
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
There was a problem hiding this comment.
🔍 Pre-existing resource leak and logic bug in Resize method not addressed by this PR
The Resize method at QRCoder.Core/Renderers/ArtQRCode.cs:322-347 creates a scaledImage bitmap (line 332) that is:
- Never populated from the input
imageparameter — it's an empty bitmap drawn onto the output canvas, meaning background images will always render as blank. - Never disposed — a resource leak of the same kind this PR is fixing elsewhere.
Since this PR's theme is fixing resource leaks in these renderers, this is a missed opportunity. The logic bug (drawing an empty bitmap instead of the actual input image) is also pre-existing and means the background image feature in ArtQRCode likely doesn't work correctly.
(Refers to lines 332-343)
Was this helpful? React with 👍 or 👎 to provide feedback.
| using (var iconBgBrush = iconBackgroundSKColor != null ? new SKPaint { Color = (SKColor)iconBackgroundSKColor } : null) | ||
| { | ||
| using (var iconPath = CreateRoundedSKRectIPath(centerDest, iconBorderWidth * 2)) | ||
| if (iconBorderWidth > 0) | ||
| { | ||
| gfx.DrawPath(iconPath, iconBgBrush); | ||
| using (var iconPath = CreateRoundedSKRectIPath(centerDest, iconBorderWidth * 2)) | ||
| { | ||
| gfx.DrawPath(iconPath, iconBgBrush ?? lightBrush); |
There was a problem hiding this comment.
📝 Info: Behavioral equivalence of DrawIcon refactoring relies on C# using-with-null semantics
The old code assigned lightBrush directly to iconBgBrush when iconBackgroundSKColor was null, making iconBgBrush always non-null. The new code sets iconBgBrush to null in that case and uses iconBgBrush ?? lightBrush at the call site. This is behaviorally equivalent because:
using (var x = null)is a no-op in C# (no dispose call)- The
??operator provides the same fallback brush lightBrushis managed by the caller and should NOT be disposed here (which the new code correctly avoids)
This is a correct and safe transformation.
Was this helpful? React with 👍 or 👎 to provide feedback.
1 participant
All Submissions:
New Feature Submissions:
Changes to Core Features:
fix: dispose SKPaint and SKBitmap in renderer icon/dot helpers
This PR addresses a Devin Review finding on the renderer phase:
QRCode.DrawIcon: the custom icon backgroundSKPaintwas allocated when a background color was supplied but never disposed. The method now wraps it in ausingblock and falls back to the sharedlightBrushwhen no background color is provided.ArtQRCode.MakeDotPixel: the temporary sourceSKBitmapcreated to draw the dot is now disposed before returning the cropped bitmap.ArtQRCode.RenderGraphicCore: the dark and light module pixelSKBitmapinstances created byMakeDotPixelare now disposed after the final QR code is rendered.Build and
dotnet test QRCoder.Core.Tests/ -f net10.0 --configuration Releasepass locally (502 tests).Link to Devin session: https://app.devin.ai/sessions/20c92a1bad724f63b8be89a9e195e664
Requested by: @afonsoft