Fix port label collisions and align text positioning in renderers#19
Merged
Conversation
Three related port-rendering bugs, all rooted in LayeredLayoutAlgorithm's box-sizing/label-clearance logic and the SVG/Skia renderers' port-label offset formulas: 1. Labeled multi-port bottom collision: a titled/labeled node's bottom-most port label could run past the box's own bottom edge because the downward-shift compensation term was not scaled per anchor slice. 2. Unlabeled multi-port growth-floor gap: a face with 2+ anchors skipped its entire growth-floor computation whenever none of the anchors carried a label, so boxes with many unlabeled ports never grew to give them room and the ports visibly bunched together. Fixed for both the Left/Right height floor and the Top/Bottom width floor (scaled by anchor count). 3. Boundary-port dual-label collision: a boundary port's ExternalLabel is drawn on the same outward face its external approach edge's arrowhead marker occupies, and the fixed port-glyph offset did not account for the marker's own length, causing the label text and arrowhead to visually collide. Fixed in both SvgRenderer and SkiaRasterRenderer by adding NotationMetrics.EndMarkerLength to the boundary-port label offset, and widening ResolveContentInsets' title clearance to match. Also fixed a PNG/SVG text-position mismatch for port labels: Skia's DrawText treats Y as a baseline while SVG's dominant-baseline="middle" treats the same Y as the text's vertical center, so identical offset formulas rendered visibly different text positions between renderers. SkiaRasterRenderer.DrawPortLabel now derives the correct baseline from the font's own ascent/descent metrics so both renderers agree. Adds a new PortsShowcaseUnlabeledFanOut gallery scene and regression tests for the growth-floor fix (Left/Right and Top/Bottom), verified via git-stash fail/pass toggling. Regenerated docs/gallery to reflect all fixes; visually cross-checked SVG output via a throwaway Puppeteer-based browser renderer against the Skia PNG output. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR fixes a layout bug in the layered layout engine where faces with multiple unlabeled ports could fail to reserve enough space, causing connector/port glyph crowding, and it aligns port-label placement behavior between the SVG and Skia renderers (including boundary-port clearance and Skia text vertical alignment).
Changes:
- Updated
LayeredLayoutAlgorithmgrowth-floor logic so faces with 2+ anchors reserve per-port clearance even when ports/edges are unlabeled, and improved multi-port label-shift compensation. - Synchronized SVG and Skia port-label offset rules for boundary ports, and corrected Skia text baseline handling to match SVG vertical centering.
- Added new gallery showcase + tests and refreshed gallery SVG outputs to demonstrate and lock in the fix.
Reviewed changes
Copilot reviewed 8 out of 25 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| test/DemaConsulting.Rendering.Layout.Tests/LayeredLayoutAlgorithmTests.cs | Adds regression tests for unlabeled-port growth floors and multi-port label margin behavior. |
| test/DemaConsulting.Rendering.Gallery/GalleryShowcaseTests.cs | Adds SVG/PNG rendering tests for the new unlabeled-port fan-out gallery diagram. |
| test/DemaConsulting.Rendering.Gallery/GalleryDiagrams.cs | Adds the new unlabeled-port fan-out diagram used for visual regression coverage. |
| test/DemaConsulting.Rendering.Gallery/GalleryCatalog.cs | Registers the new gallery image and description text in the catalog. |
| src/DemaConsulting.Rendering.Svg/SvgRenderer.cs | Adjusts port-label offset to add extra clearance for boundary ports (end-marker collision avoidance). |
| src/DemaConsulting.Rendering.Skia/SkiaRasterRenderer.cs | Mirrors boundary-port label clearance and fixes text baseline alignment to match SVG centering. |
| src/DemaConsulting.Rendering.Layout/LayeredLayoutAlgorithm.cs | Applies per-anchor growth floors for unlabeled faces and widens title clearance for top/bottom ports. |
| docs/gallery/README.md | Documents the new unlabeled-port fan-out gallery image. |
| docs/gallery/ports-showcase-vertical.svg | Updated rendered output reflecting layout changes. |
| docs/gallery/ports-showcase-unlabeled-fan-out.svg | New gallery SVG demonstrating unlabeled port fan-out spacing. |
| docs/gallery/ports-showcase-multi-connector-vertical.svg | Updated rendered output reflecting layout changes. |
| docs/gallery/ports-showcase-multi-connector-horizontal.svg | Updated rendered output reflecting layout changes. |
| docs/gallery/parallel-edges-preserved.svg | Updated rendered output reflecting layout changes. |
| docs/gallery/boundary-ports-showcase-vertical.svg | Updated rendered output reflecting boundary-port label offset changes. |
| docs/gallery/boundary-ports-showcase-horizontal.svg | Updated rendered output reflecting boundary-port label offset changes. |
| docs/gallery/boundary-ports-showcase-deep-chain.svg | Updated rendered output reflecting boundary-port label offset changes. |
Comments suppressed due to low confidence (1)
src/DemaConsulting.Rendering.Layout/LayeredLayoutAlgorithm.cs:870
- There are two extra blank lines at the end of the file after the closing brace, which is likely accidental and can create noisy diffs going forward.
}
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Test assertions for the unlabeled-port growth-floor fix asserted a weaker threshold (Total * ConnectorClearance) than the algorithm's actual floor (2 * ConnectorClearance * Total); tightened all three affected tests to match. - PortsShowcaseUnlabeledFanOut's doc comment, gallery caption, and README alt text said the hub has 5 outgoing ports; it actually has 4 (memory is reached via cpu, not directly from the hub). Corrected the wording in GalleryDiagrams.cs and GalleryCatalog.cs; docs/gallery/README.md regenerated via gallery.ps1 to pick up the corrected caption. - Fixed US English spelling (labelled -> labeled) in the new gallery caption text. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- SvgRenderer/SkiaRasterRenderer: gate the EndMarkerLength offset on true boundary ports (both InternalLabel and ExternalLabel present), not just InternalLabel alone - LayeredLayoutAlgorithm: correct comment wording (height, not half-height, is preserved per slice) and trim trailing blank lines at EOF - Tests: reference LayeredLayoutMetrics.ConnectorClearance directly instead of duplicating the literal constant; add real anchor-spacing assertions to the two growth-floor tests; remove unused 'ports' list in the titled unlabeled-ports test Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
… centered) Fixes flagged in PR review: LayeredLayoutAlgorithm.cs, SvgRenderer.cs, SkiaRasterRenderer.cs, LayeredLayoutAlgorithmTests.cs, and GalleryDiagrams.cs comments. 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 significant layout bug where nodes with multiple unlabeled ports on the same face could become too small, causing port connectors to visually bunch together. The changes ensure that any face with two or more ports (even if unlabeled) reserves sufficient space to keep connectors well-separated. The fix is thoroughly documented and covered by new gallery diagrams and tests. Additional improvements clarify and synchronize port label rendering logic between the SVG and raster renderers, especially for boundary ports.
Layout and Rendering Fixes:
LayeredLayoutAlgorithm.cs) [1] [2] [3] [4]SvgRenderer.cs,SkiaRasterRenderer.cs) [1] [2] [3]SkiaRasterRenderer.cs)Gallery and Documentation Updates:
ports-showcase-unlabeled-fan-out.svg/.png) demonstrating the fix: a node with several unlabeled ports now grows tall enough to spread them apart. Updated documentation and test catalog accordingly. (README.md,GalleryCatalog.cs,GalleryDiagrams.cs,GalleryShowcaseTests.cs) [1] [2] [3] [4] [5] [6]Other improvements:
LayeredLayoutAlgorithm.cs)These changes resolve the "Motherboard" bug and improve the clarity and consistency of port rendering throughout the codebase.