fix: harden canvas badge sizing and tails

[PrzemyslawKlys]

Summary:\n- Skip badge rendering when WidthPx/HeightPx <= 0\n- Clamp ribbon tail dimensions for tiny badges\n- Add regression tests for zero-size and tiny ribbon badges\n\nNotes:\n- Stacks on PR #42 (feature/art-enhancements-20260127)\n

[claude]

Claude finished @PrzemyslawKlys's task —— View job

---

Code Review Complete ✅

Reviewing PR #44: "fix: harden canvas badge sizing and tails"

Todo List:

• Read and analyze changed files
• Review code quality and best practices
• Check for potential bugs or issues
• Evaluate performance considerations
• Assess security concerns
• Review test coverage
• Address Copilot reviewer feedback
• Provide final recommendations

---

🔍 Review Summary

This PR implements defensive programming improvements to handle edge cases in canvas badge rendering. The changes are well-structured and include proper test coverage.

✅ Strengths

1. Robust Early Return Logic

• The early return if (badge.WidthPx <= 0 || badge.HeightPx <= 0) return; at QrPngRenderer.Internal.A.cs:2575 is clean and prevents unnecessary computation
• Eliminates redundant Math.Max(1, ...) clamping that was masking the underlying issue

2. Smart Tail Dimension Clamping

• Lines 2718-2721 and 2766-2769 add intelligent bounds checking for ribbon tails
• The logic var tailBase = Math.Max(4, Math.Min(w / 3, h * 2)) provides reasonable default proportions
• Additional safety check if (tailWidth < 2) return; prevents degenerate triangles

3. Comprehensive Test Coverage

• Render_With_Zero_Size_Badge_Skips_Drawing() verifies zero-dimension handling
• Render_With_Tiny_Ribbon_Badge_Skips_Tails() tests edge cases for tiny ribbons
• Tests use proper pixel inspection to validate behavior

4. Documentation Updates

• XML comments in QrPngCanvasBadgeOptions.cs:20,25 clearly state that 0 disables drawing

⚠️ Issues Found

1. Duplicate Test Assertion (Confirmed by Copilot)

// QrPngRendererTests.cs:1537 - This line appears to be duplicate from line 1483
Assert.True(foundBand, "Expected band pixels outside the QR bounds.");

2. Incomplete Test Coverage (Confirmed by Copilot)
The test Render_With_Zero_Size_Badge_Skips_Drawing() only tests WidthPx = 0 but not HeightPx = 0. Since the condition checks both dimensions, both cases should be tested.

3. Minor Code Style

• The tail clamping logic could benefit from explanatory comments about the magic numbers (4, 2)

🚀 Performance & Security

Performance: ✅ Excellent - Early returns prevent unnecessary computation for invalid badges

Security: ✅ No concerns - All input validation is defensive and bounds-checked

📝 Recommendations

1. Fix duplicate assertion in QrPngRendererTests.cs:1537

2. Add HeightPx = 0 test case:

[Fact]
public void Render_With_Zero_Height_Badge_Skips_Drawing() {
    // Similar to existing test but with HeightPx = 0, WidthPx = 24
}

3. Consider adding explanatory comments for tail dimension constants:

var tailBase = Math.Max(4, Math.Min(w / 3, h * 2)); // Min 4px for visible tail
// ...
if (tailWidth < 2) return; // Skip if too narrow to render properly

🎯 Overall Assessment

Code Quality: ⭐⭐⭐⭐⭐ (Excellent)
Bug Prevention: ⭐⭐⭐⭐⭐ (Excellent)
Test Coverage: ⭐⭐⭐⭐☆ (Very Good - minor gaps)

This is a high-quality defensive programming improvement. The logic is sound, performance is optimized, and the approach follows good software engineering principles. The issues identified are minor and easily addressed.

Recommendation: ✅ Approve with minor fixes for the duplicate assertion and test coverage gap.

[Copilot]

Pull request overview

This PR hardens canvas badge rendering to handle zero-size and tiny badge dimensions gracefully.

Changes:

• Add early return to skip badge rendering when either WidthPx or HeightPx is <= 0
• Remove redundant Math.Max(1, ...) clamping that's now unnecessary after early return
• Add ribbon tail dimension clamping to prevent degenerate tails on tiny badges
• Update documentation to clarify that 0 disables badge drawing
• Add regression tests for zero-size and tiny ribbon badges

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File	Description
CodeGlyphX/Rendering/Png/QrPngRenderer.Internal.A.cs	Adds early return for zero-size badges and clamps ribbon tail dimensions to prevent rendering issues with tiny badges
CodeGlyphX/Rendering/Png/QrPngCanvasBadgeOptions.cs	Updates XML documentation to clarify that 0 width/height disables badge drawing
CodeGlyphX.Tests/QrPngRendererTests.cs	Adds regression tests for zero-size badge skip and tiny ribbon badge tail clamping

[Copilot]

Duplicate assertion detected. This line is identical to line 1537 and should be removed.

Suggested change

Assert.True(foundBand, "Expected band pixels outside the QR bounds.");

[Copilot]

The test only covers the case where WidthPx = 0, but does not test HeightPx = 0. Since the early return condition checks both dimensions (line 2575 in QrPngRenderer.Internal.A.cs), both cases should be tested to ensure complete coverage of the zero-size badge skip logic.

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud