Fix leading divider rendering for boxes without title area#18
Merged
Conversation
… area RenderBoxCompartments (in both SvgRenderer and SkiaRasterRenderer) always drew a full-width divider line above every compartment, including the first. When a box has no Label/Keyword, the first compartment starts at the box's own top edge, so this divider coincided with (and duplicated) the box outline. For rectangular boxes this was an invisible no-op, but for BoxShape.Note the outline's top edge is a diagonal fold cut short of the box's full width, so the divider line protruded past the fold as a visible stray line artifact. Fix: skip the leading divider before the first compartment unless a title area (Label or Keyword) precedes it, in both renderers. Added regression tests covering a Note-shaped box with a compartment and no title in both the SVG and Skia raster test suites. Reported via a screenshot of an Avalonia app rendering an SVG generated by an external tool (sysml2tools) that consumes this library; confirmed via docs/design/generated/PublishingView.svg in a sibling repo showing the exact stray <line> element. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Fixes a rendering artifact where the divider line above the first compartment could be drawn even when a box has no title area (label/keyword), which is visually incorrect for Note-shaped boxes (line can protrude past the folded corner). The change keeps compartment dividers only where they actually separate a title area from compartment content, and adds regression tests for both SVG and Skia raster outputs.
Changes:
- Update compartment rendering in both SVG and Skia renderers to skip the leading divider when there is no title area.
- Add an SVG regression test ensuring the Note + compartment + no title case omits the top-edge divider while still rendering compartment text.
- Add a Skia PNG regression test scanning pixels near the fold area to ensure no stray divider appears past the Note fold.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
src/DemaConsulting.Rendering.Svg/SvgRenderer.cs |
Skip first compartment divider when there is no label/keyword title area. |
src/DemaConsulting.Rendering.Skia/SkiaRasterRenderer.cs |
Mirror the same “omit leading divider when no title area” logic in the raster renderer. |
test/DemaConsulting.Rendering.Svg.Tests/SvgRendererPortedTests.cs |
Adds SVG regression coverage for Note-shaped boxes with compartments but no title. |
test/DemaConsulting.Rendering.Skia.Tests/SkiaPortAndContentInsetTests.cs |
Adds PNG pixel-based regression coverage for the Note-fold stray-divider artifact. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- SkiaPortAndContentInsetTests: scan a 4-row vertical band (instead of a single row) at the note box's top edge for the stray-divider check, and offset xStart past where the diagonal fold edge (1:1 slope) plus its anti-aliasing could reach, so the pixel scan won't miss/flake on the regression or false-positive on the legitimate fold edge. - SvgRendererPortedTests: assert on the parsed XML <line> element's numeric attributes (x1/y1/x2/y2, matched with a small tolerance) instead of a literal attribute-string match, so the test is robust to harmless formatting/attribute-ordering changes in the SVG serialization. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- SvgRenderer/SkiaRasterRenderer: derive hasTitleArea from labelAreaHeight > 0 instead of duplicating the Label/Keyword null-check, so the divider logic stays consistent with whatever TitleAreaHeight defines as a title area. - SvgRendererPortedTests: derive the expected stray-divider coordinates from box/options.Scale instead of hard-coding them, and use pattern matching on the line element's attributes instead of null-suppression, avoiding a potential null reference if non-standard <line> elements appear in the SVG. Verified via the code-review agent: no remaining issues found. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The prior name 'hasTitleArea' was ambiguous between two readings - 'is a Label/Keyword present' vs. 'does the title area occupy non-zero space' - and automated review feedback flip-flopped between both interpretations across rounds as a result. Renamed to titleAreaOccupiesSpace and expanded the comment to make explicit that this is deliberately the geometric check (labelAreaHeight > 0), since the actual bug condition is whether compartmentY coincides with box.Y - which depends on reserved space, not on Label/Keyword presence alone (a zero-padding/zero-font-size theme could set labelAreaHeight to 0 even with a Label present, and the leading divider must still be skipped in that case). Also hardened SkiaPortAndContentInsetTests per review feedback: - Use Math.Ceiling instead of int truncation for xFold, so the pixel scan never starts left of the true fold boundary. - Assert.NotNull(bitmap) after decode so a decode failure surfaces as a clear assertion rather than a downstream NullReferenceException. Verified via the code-review agent: no remaining issues found. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The prior comment claimed titleAreaOccupiesSpace checks whether compartmentY differs from box.Y, but compartmentY also incorporates ContentInsetTop and (for BoxShape.Folder) the tab offset via ResolveTitleAreaTop, so that equivalence doesn't actually hold in general. Reworded both renderer comments to describe the actual check (labelAreaHeight > 0) without overclaiming a strict geometric equivalence that isn't true. No logic change; comment-only fix. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This pull request addresses a rendering bug where a stray divider line could appear past the folded corner of a Note-shaped box when it contains compartments but no label or keyword. The fix ensures that the divider above the first compartment is only drawn if there is a title area (label/keyword), preventing visual artifacts in both the Skia (PNG) and SVG renderers. Comprehensive tests are added to verify the corrected behavior.
Rendering logic fixes:
RenderBoxCompartmentsin bothSkiaRasterRenderer.csandSvgRenderer.csto only draw the divider above the first compartment if a title area (label or keyword) is present, preventing stray lines in Note-shaped boxes. [1] [2] [3]Testing improvements:
SkiaPortAndContentInsetTests.csto verify that rendering a Note-shaped box with a compartment and no label/keyword does not draw a stray divider past the fold.SvgRendererPortedTests.csto verify that the SVG renderer omits the leading divider line in the same scenario, and that compartment text is still rendered.