fix(Geometries): Relative line drawing

[workgroupengineering]

What does the pull request do?

fix parsing relative line drawing

What is the current behavior?

When using this F1 M50,50z l -5,-5 the line is not drawn as expected

<Window xmlns="https://github.com/avaloniaui"
        xmlns:x='http://schemas.microsoft.com/winfx/2006/xaml'
        x:Class="Sandbox.MainWindow">
  <Window.Resources>
    <Geometry x:Key="redoGeometry1">F1 M50,50z l -5,-5</Geometry>
    <DrawingGroup x:Key="redoDrawingGroup" >
      <DrawingGroup Opacity="1" Transform="1,0,0,1,0,-246.2">
        <GeometryDrawing Geometry="{StaticResource redoGeometry1}">
          <GeometryDrawing.Pen>
            <Pen Brush="#FF000000" Thickness="2" LineCap="Round" LineJoin="Round" />
          </GeometryDrawing.Pen>
        </GeometryDrawing>
      </DrawingGroup>
    </DrawingGroup>
    <DrawingImage x:Key="redoDrawingImage" Drawing="{StaticResource redoDrawingGroup}" />
  </Window.Resources>
  <Grid Height="200" Width="200">
    <Image Source="{StaticResource redoDrawingImage}"/>
  </Grid>
</Window>

Expected(WPF)

Avalonia(Skia)

Avalonia(D3D)

What is the updated/expected behavior with this PR?

Avalonia D3D and Skia as same behavior of WPF

How was the solution implemented (if it's not obvious)?

Checklist

• Added unit tests (if possible)?
• Added XML documentation to any related classes?
• Consider submitting a PR to https://github.com/AvaloniaUI/avalonia-docs with user documentation

Breaking changes

Obsoletions / Deprecations

Fixed issues

Fixes #13235

[avaloniaui-bot]

You can test this PR using the following package version. 11.1.999-cibuild0043090-beta. (feed url: https://nuget-feed-all.avaloniaui.net/v3/index.json) [PRBUILDID]

[workgroupengineering]

The other commands for drawing lines also suffer from the same problem

Horizontal Line h

Command: F1 M50,50z h 10

Expected(WPF)

Avalonia(D3D)

Avalonia(Skia)

Vertical Line v

Command: F1 M50,50z v 10

Expected(WPF)

Avalonia(D3D)

Avalonia(Skia)

[avaloniaui-bot]

You can test this PR using the following package version. 11.1.999-cibuild0043115-beta. (feed url: https://nuget-feed-all.avaloniaui.net/v3/index.json) [PRBUILDID]

[Gillibald]

M50,50z l -5,-5

-Move to 50,50
-Close current segment by drawing a line straight to to starting point 0,0
Draw a line relative to current point 0,0 to -5,-5

So the result should be a line from -5,-5 to 50,50

All this should be covered in a unit test.

I personally don't trust the WPF implementation and instead the SVG spec should be followed.

[avaloniaui-bot]

You can test this PR using the following package version. 11.1.999-cibuild0043292-beta. (feed url: https://nuget-feed-all.avaloniaui.net/v3/index.json) [PRBUILDID]

[Gillibald]

This test is failing:

 [Fact]
 public void Should_Parse_Path_With_Segments()
 {
     var pathGeometry = new PathGeometry();
     using (var context = new PathGeometryContext(pathGeometry))
     using (var parser = new PathMarkupParser(context))
     {
         parser.Parse("M50,50z l -5,-5");

         Assert.Equal(2, pathGeometry.Figures.Count);

         var firstFigure = pathGeometry.Figures[0];

         Assert.Equal(new Point(50, 50), firstFigure.StartPoint);

         var secondFigure = pathGeometry.Figures[1];

         Assert.Equal(new Point(0,0), secondFigure.StartPoint);
     }
 }

M50,50z

When the first figure is closed it should draw a line from 50,50 to 0,0 because we explicitly close the figure

l -5,-5

A LineTo should start at 0,0 and end at -5,-5

In the end, there should be one line starting at -5,-5 and ending at 50,50

The current PathMarkupParser implementation is not properly dealing with segments that only contain a MoveTo and are explicitly closed. This should always produce a LineTo

[kekekeks]

Do we have a geometry sink that doesn't produce an actual platform impl? If so, it should be used in tests instead of render tests

[Gillibald]

Do we have a geometry sink that doesn't produce an actual platform impl? If so, it should be used in tests instead of render tests

There are existing tests here: https://github.com/AvaloniaUI/Avalonia/blob/master/tests/Avalonia.Base.UnitTests/Media/PathMarkupParserTests.cs

They are producing PathFigures but you can also mock IGeometryContext

[workgroupengineering]

M50,50z

When the first figure is closed it should draw a line from 50,50 to 0,0 because we explicitly close the figure

The WPF Spec M50,50z produce empty figure. I would like to do a separate PR that addresses issue #4913

I personally don't trust the WPF implementation and instead the SVG spec should be followed.

I think if team decide to do this it should be implemented in v12. It's a big change that would lead to big problems even in font management. It should be possible to set for single font to use WPF compatibility.

Tell me what direction the team wants to take.
And whether to resolve the issue of empty figures here or separately.

[Gillibald]

https://svg-path-visualizer.netlify.app/#M50%2C50z%20l%20-5%2C-5

This shows the correct result

[workgroupengineering]

https://svg-path-visualizer.netlify.app/#M50%2C50z%20l%20-5%2C-5

This shows the correct result

So do I have to follow the SVG Specification?

if i following spec the test

 [Fact]
 public void Should_Parse_Path_With_Segments()
 {
     var pathGeometry = new PathGeometry();
     using (var context = new PathGeometryContext(pathGeometry))
     using (var parser = new PathMarkupParser(context))
     {
         parser.Parse("M50,50z l -5,-5");

         Assert.Equal(2, pathGeometry.Figures.Count);

         var firstFigure = pathGeometry.Figures[0];

         Assert.Equal(new Point(50, 50), firstFigure.StartPoint);

         var secondFigure = pathGeometry.Figures[1];

         Assert.Equal(new Point(0,0), secondFigure.StartPoint);
     }
 }

Step down M50,50z l -5,-5

Command	Description	Initial Point Before Command	Current Point Before Commad	Initial Point After Command	Current Point After Commad	Note
M 50,50	Set intial Point	null	null	50,50	50,50
z	Close path	50,50	50,50	null	50,50	Since the current point is the same as the starting point, the figure is empty (don't draw anything)
l -5,-5	dra relative line	null	50,50	50,50	45,45	draw the line from (50,50) to (45,45). The inti point is 50,50 because there is no moveto command after the closepath command.

the correct test should be:

 [Fact]
 public void Should_Parse_Path_With_Segments()
 {
     var pathGeometry = new PathGeometry();
     using (var context = new PathGeometryContext(pathGeometry))
     using (var parser = new PathMarkupParser(context))
     {
         parser.Parse("M50,50z l -5,-5");

         Assert.Equal(2, pathGeometry.Figures.Count);

         var firstFigure = pathGeometry.Figures[0];

         Assert.Equal(new Point(50, 50), firstFigure.StartPoint);

         var secondFigure = pathGeometry.Figures[1];

         Assert.Equal(new Point(50, 50), secondFigure.StartPoint);
     }
 }

<?xml version="1.0" encoding="UTF-8" standalone="no"?>
<!-- Created with Inkscape (http://www.inkscape.org/) -->

<svg
   width="50"
   height="50"
   viewBox="0 0 50 50"
   version="1.1"
   id="svg1"
   xmlns="http://www.w3.org/2000/svg"
   xmlns:svg="http://www.w3.org/2000/svg">
  <path
     d="M 50,50 Z l -5,-5"
     id="path1"
     style="fill:none;stroke:#000000;stroke-width:1px;stroke-linecap:butt;stroke-linejoin:miter;stroke-opacity:1" />
</svg>

[Gillibald]

Please remove added render tests and rewrite them so that the tests only use the PathMarkupParser. The rendered result might depend on the backend but we want to ensure the parser properly parses some sequences that are currently not working as expected.

[avaloniaui-bot]

You can test this PR using the following package version. 11.1.999-cibuild0043640-beta. (feed url: https://nuget-feed-all.avaloniaui.net/v3/index.json) [PRBUILDID]