Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 12 additions & 2 deletions src/DemaConsulting.Rendering.Skia/SkiaRasterRenderer.cs
Original file line number Diff line number Diff line change
Expand Up @@ -508,14 +508,22 @@
var scale = (float)options.Scale;

// Compartments start below the title area (keyword + label), computed via shared metrics
var hasTitleArea = box.Label != null || box.Keyword != null;
var labelAreaHeight = BoxMetrics.TitleAreaHeight(theme, box.Label != null, box.Keyword != null);
var compartmentY = ResolveTitleAreaTop(box, theme) + box.ContentInsetTop + labelAreaHeight;
Comment thread
Malcolmnixon marked this conversation as resolved.
Outdated
Comment thread
Malcolmnixon marked this conversation as resolved.

var isFirstCompartment = true;
foreach (var compartment in box.Compartments)
{
// Draw a full-width horizontal divider at the top of this compartment
using (var divPaint = new SKPaint())
// The divider above the first compartment only has something to separate from when a
// keyword/label title area precedes it. With no title area, this divider would sit
// exactly on the box's own top edge — redundant for a plain rectangle (the two lines
// coincide) but a genuine visible defect for shapes whose top edge isn't a plain
// straight line across the full width (e.g. Note's folded corner cutout), where the
// full-width divider ignores the cut and renders as a stray line past the fold.
if (!isFirstCompartment || hasTitleArea)
{
using var divPaint = new SKPaint();
divPaint.Color = strokeColor;
divPaint.Style = SKPaintStyle.Stroke;
divPaint.StrokeWidth = (float)theme.StrokeWidth * scale;
Expand All @@ -527,6 +535,8 @@
divPaint);
}

isFirstCompartment = false;

// Draw the optional bold compartment title
if (compartment.Title != null)
{
Expand Down Expand Up @@ -568,7 +578,7 @@
/// <param name="canvas">Canvas to draw on.</param>
/// <param name="line">Line node to render.</param>
/// <param name="options">Render options providing theme and scale.</param>
private static void RenderLine(SKCanvas canvas, LayoutLine line, RenderOptions options)

Check warning on line 581 in src/DemaConsulting.Rendering.Skia/SkiaRasterRenderer.cs

View workflow job for this annotation

GitHub Actions / Build / Build macos-latest

Refactor this method to reduce its Cognitive Complexity from 20 to the 15 allowed.

Check warning on line 581 in src/DemaConsulting.Rendering.Skia/SkiaRasterRenderer.cs

View workflow job for this annotation

GitHub Actions / Build / Build ubuntu-latest

Refactor this method to reduce its Cognitive Complexity from 20 to the 15 allowed.
{
// Lines with fewer than 2 waypoints cannot be drawn
if (line.Waypoints.Count < 2)
Expand Down Expand Up @@ -918,7 +928,7 @@
/// <param name="vertex">The tip-relative marker vertex in notation units.</param>
/// <param name="scale">Uniform scale factor.</param>
/// <returns>The mapped canvas point.</returns>
private static SKPoint MarkerPoint(

Check warning on line 931 in src/DemaConsulting.Rendering.Skia/SkiaRasterRenderer.cs

View workflow job for this annotation

GitHub Actions / Build / Build macos-latest

Method has 8 parameters, which is greater than the 7 authorized.

Check warning on line 931 in src/DemaConsulting.Rendering.Skia/SkiaRasterRenderer.cs

View workflow job for this annotation

GitHub Actions / Build / Build ubuntu-latest

Method has 8 parameters, which is greater than the 7 authorized.
float tipX, float tipY, float dx, float dy, float px, float py, MarkerVertex vertex, float scale)
{
var along = (float)vertex.Along;
Expand Down Expand Up @@ -1077,7 +1087,7 @@
/// Builds the triangle marker path from <see cref="NotationMetrics.TriangleVertices"/>, optionally
/// closing the base edge (closed for the hollow/filled triangle, open for the chevron).
/// </summary>
private static SKPath TrianglePath(

Check warning on line 1090 in src/DemaConsulting.Rendering.Skia/SkiaRasterRenderer.cs

View workflow job for this annotation

GitHub Actions / Build / Build macos-latest

Method has 8 parameters, which is greater than the 7 authorized.

Check warning on line 1090 in src/DemaConsulting.Rendering.Skia/SkiaRasterRenderer.cs

View workflow job for this annotation

GitHub Actions / Build / Build ubuntu-latest

Method has 8 parameters, which is greater than the 7 authorized.
float tipX, float tipY, float dx, float dy, float px, float py, float scale, bool close)
{
var vertices = NotationMetrics.TriangleVertices();
Expand Down
21 changes: 17 additions & 4 deletions src/DemaConsulting.Rendering.Svg/SvgRenderer.cs
Original file line number Diff line number Diff line change
Expand Up @@ -597,15 +597,28 @@
private static void RenderBoxCompartments(StringBuilder sb, LayoutBox box, Theme theme, double scale)
{
// Compartments start below the title area (keyword + label), computed via shared metrics
var hasTitleArea = box.Label != null || box.Keyword != null;
var labelAreaHeight = BoxMetrics.TitleAreaHeight(theme, box.Label != null, box.Keyword != null);
var compartmentY = ResolveTitleAreaTop(box, theme) + box.ContentInsetTop + labelAreaHeight;
Comment thread
Malcolmnixon marked this conversation as resolved.
Outdated

var isFirstCompartment = true;
foreach (var compartment in box.Compartments)
{
// Full-width horizontal divider at the top of this compartment
sb.Append(CultureInfo.InvariantCulture,
$""" <line x1="{F(box.X * scale)}" y1="{F(compartmentY * scale)}" x2="{F((box.X + box.Width) * scale)}" y2="{F(compartmentY * scale)}" stroke="{theme.StrokeColor}" stroke-width="{F(theme.StrokeWidth)}"/>""");
sb.AppendLine();
// The divider above the first compartment only has something to separate from when a
// keyword/label title area precedes it. With no title area, this divider would sit
// exactly on the box's own top edge — redundant for a plain rectangle (the two lines
// coincide) but a genuine visible defect for shapes whose top edge isn't a plain
// straight line across the full width (e.g. Note's folded corner cutout), where the
// full-width divider ignores the cut and renders as a stray line past the fold.
if (!isFirstCompartment || hasTitleArea)
{
// Full-width horizontal divider at the top of this compartment
sb.Append(CultureInfo.InvariantCulture,
$""" <line x1="{F(box.X * scale)}" y1="{F(compartmentY * scale)}" x2="{F((box.X + box.Width) * scale)}" y2="{F(compartmentY * scale)}" stroke="{theme.StrokeColor}" stroke-width="{F(theme.StrokeWidth)}"/>""");
sb.AppendLine();
}

isFirstCompartment = false;

// Draw optional bold compartment title
if (compartment.Title != null)
Expand Down Expand Up @@ -783,7 +796,7 @@
/// <param name="sourceEnd">End-marker style at the first waypoint (source endpoint).</param>
/// <param name="targetEnd">End-marker style at the last waypoint (target endpoint).</param>
/// <returns>SVG path data string starting with <c>M</c>.</returns>
private static string BuildLinePath(

Check warning on line 799 in src/DemaConsulting.Rendering.Svg/SvgRenderer.cs

View workflow job for this annotation

GitHub Actions / Build / Build macos-latest

Refactor this method to reduce its Cognitive Complexity from 24 to the 15 allowed.

Check warning on line 799 in src/DemaConsulting.Rendering.Svg/SvgRenderer.cs

View workflow job for this annotation

GitHub Actions / Build / Build ubuntu-latest

Refactor this method to reduce its Cognitive Complexity from 24 to the 15 allowed.
IReadOnlyList<Point2D> waypoints,
double cornerRadius,
double scale,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -217,6 +217,55 @@ private static int RightmostForegroundX(SKBitmap bitmap, SKColor background, int
return -1;
}

/// <summary>
/// Proves that rendering a Note-shaped box with a compartment and no Label/Keyword does not
/// draw a stray divider line protruding past the note's folded-corner cutout. With no title
/// area, the first compartment's divider would land exactly on the box's own top edge; for a
/// plain rectangle that's an invisible duplicate, but for Note it must not extend past the
/// diagonal fold (from xFold to the right edge) as a visible artifact.
/// </summary>
[Fact]
public void PngRenderer_Render_NoteBoxWithCompartmentAndNoTitle_NoStrayLinePastFold()
{
// Arrange: a Note-shaped box with a compartment but no Label/Keyword
var renderer = new PngRenderer();
var compartment = new LayoutCompartment(null, ["Some body text"]);
var box = new LayoutBox(10, 10, 150, 80, null, 0, BoxShape.Note, [compartment], []);
var options = new RenderOptions(Themes.Light);
var background = SKColor.Parse(Themes.Light.BackgroundColor);

using var stream = new MemoryStream();
renderer.Render(new LayoutTree(200, 120, [box]), options, stream);
stream.Position = 0;
using var data = SKData.Create(stream);
using var bitmap = SKBitmap.Decode(data);
Comment thread
Copilot marked this conversation as resolved.

// Compute the fold's scaled x-position (matching RenderNotePng's own geometry) and scan a
// vertical band just below the box's top edge. The divider (a ~1.5px stroke) can land
// partially on adjacent rows once anti-aliased, so a single row risked missing or flaking
// on the regression; scanning several rows makes the check reliable. The diagonal fold edge
// has a 1:1 slope (rise == run == fold size) starting at (xFold, yTop), so the scan's xStart
// is offset past where that diagonal (plus its anti-aliasing) could reach within the band,
// ensuring only a genuine stray horizontal divider - not the legitimate fold edge - would be
// detected.
var fold = Math.Min(Math.Min(box.Width, box.Height) * NotationMetrics.NoteFoldFraction, NotationMetrics.NoteFoldMaxSize);
var scale = options.Scale;
var xFold = (int)((box.X + box.Width - fold) * scale);
var yTop = (int)(box.Y * scale);
Comment thread
Copilot marked this conversation as resolved.
const int bandHeight = 4;
const int diagonalClearance = 3;

var rightmost = RightmostForegroundX(
bitmap,
background,
yStart: yTop,
yEnd: yTop + bandHeight,
xStart: xFold + bandHeight + diagonalClearance);

// Assert: no stray foreground pixel found past the fold at the top edge
Assert.Equal(-1, rightmost);
}

/// <summary>
/// Proves that when 3+ parallel labeled connectors force the label placer to nudge labels
/// downward to avoid collisions, the raster renderer grows the bitmap (rather than sizing it
Expand Down
42 changes: 42 additions & 0 deletions test/DemaConsulting.Rendering.Svg.Tests/SvgRendererPortedTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -284,6 +284,48 @@ public void SvgRenderer_Render_BoxWithCompartment_ProducesLineAndText()
Assert.Contains("+ radius : Real", svgText, StringComparison.Ordinal);
}

/// <summary>
/// Render a LayoutBox with a Note shape, a compartment, and no Label/Keyword does not emit a
/// divider line before the first compartment, since with no title area the divider would sit
/// on the box's own top edge and (unlike a plain rectangle) protrude past the Note shape's
/// folded-corner cutout as a stray line.
/// </summary>
[Fact]
public void SvgRenderer_Render_NoteBoxWithCompartmentAndNoTitle_OmitsLeadingDivider()
{
// Arrange: a Note-shaped box with a compartment but no Label/Keyword
var renderer = new SvgRenderer();
var compartment = new LayoutCompartment(null, ["Some body text"]);
var box = new LayoutBox(10, 10, 150, 80, null, 0, BoxShape.Note, [compartment], []);
var layout = new LayoutTree(200, 120, [box]);
var options = new RenderOptions(Themes.Light);
using var output = new MemoryStream();

// Act
renderer.Render(layout, options, output);

// Assert: no full-width divider <line> element is drawn at the box's own top edge (the
// previously buggy behavior), but the compartment row text still renders. Parsing as XML
// and inspecting the element's attributes (rather than matching a literal substring) keeps
// this robust to harmless formatting/attribute-ordering changes in the SVG serialization.
output.Position = 0;
var svgText = ReadAllText(output);
var document = System.Xml.Linq.XDocument.Parse(svgText);
var hasStrayDivider = document.Descendants()
.Where(e => e.Name.LocalName == "line")
.Any(e =>
IsClose(e.Attribute("x1")!.Value, 10.0) &&
IsClose(e.Attribute("y1")!.Value, 10.0) &&
IsClose(e.Attribute("x2")!.Value, 160.0) &&
IsClose(e.Attribute("y2")!.Value, 10.0));
Comment thread
Malcolmnixon marked this conversation as resolved.
Outdated
Assert.False(hasStrayDivider);
Assert.Contains("Some body text", svgText, StringComparison.Ordinal);
}

/// <summary>Parses an SVG numeric attribute value and compares it to an expected value within a small tolerance.</summary>
private static bool IsClose(string attributeValue, double expected) =>
Math.Abs(double.Parse(attributeValue, System.Globalization.CultureInfo.InvariantCulture) - expected) < 0.01;

/// <summary>
/// Render a LayoutBox with RoundedRectangle shape produces SVG output containing an
/// rx attribute, confirming that rounded corners are applied via the rx/ry attributes.
Expand Down
Loading