Skip to content

[Feature]: Implement B-spline SVG path component for workflow links #15#16

Open
LudoBroche wants to merge 6 commits into
mainfrom
15-feature-implement-b-spline-svg-path-component-for-workflow-links
Open

[Feature]: Implement B-spline SVG path component for workflow links #15#16
LudoBroche wants to merge 6 commits into
mainfrom
15-feature-implement-b-spline-svg-path-component-for-workflow-links

Conversation

@LudoBroche

@LudoBroche LudoBroche commented Jul 7, 2026

Copy link
Copy Markdown
Member

Description

Closes #15 This PR implements the isolated Bezier SVG component.

Changes Made

  • New SvgLinkCubicBezier class, to generate an svg element from a CubicBezierPath data structure and the desired style properties (color, stroke_width ...)
  • CubicBezierPath is a new data class representing the start, end and control points of the bezier path. A helper function (from_points(points, radius)) translate a polyline coordinates into rounded cubic bezier path.

Note to Reviewer(s)

This PR is strictly limited to the SVG path generation. It does not include the ewoks JSON parsing, collision avoidance, routing calculations, or node rendering.

@LudoBroche LudoBroche self-assigned this Jul 7, 2026
@LudoBroche LudoBroche linked an issue Jul 7, 2026 that may be closed by this pull request
@codecov

codecov Bot commented Jul 7, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 95.06173% with 4 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
src/ewoksdraw/svg/svg_element.py 63.63% 4 Missing ⚠️

📢 Thoughts on this report? Let us know!

@LudoBroche LudoBroche closed this Jul 7, 2026
@LudoBroche LudoBroche reopened this Jul 7, 2026
@LudoBroche

LudoBroche commented Jul 7, 2026

Copy link
Copy Markdown
Member Author

Since, the scope of this PR is not to compute the links coordinates from an ewoks workflows. I created a a small example script to show the API in examples/cubic_bezier_link.py :

Path definition using directly the bezier control points:

letter_w = CubicBezierPath(
    start=(135, 84),
    segments=[
        CubicBezierSegment((142, 124), (145, 154), (158, 156)),
        CubicBezierSegment((169, 158), (176, 102), (187, 102)),
        CubicBezierSegment((198, 102), (196, 157), (210, 157)),
        CubicBezierSegment((224, 157), (236, 105), (244, 84)),
    ],
)

Path definition using polyline coordinates and corner radius :

underline = CubicBezierPath.from_points(
    points=[
        (40, 175),
        (240, 175),
        (240, 205),
        (480, 205),
        (480, 25),
        (320, 25),
        (320, 205),
    ],
    radius=30,
)

Svg Path element create from the CubicBezierPath object :

  svg_w = SvgLinkCubicBezier(
      letter_w,
      color="#00c2a8",
      stroke_width=4,
      stroke_dash="5 10",
  )

svg_underline = SvgLinkCubicBezier(
    underline,
    color="#ff00aa",
    stroke_width=3,
    stroke_dash="12 6",
),

Add elements to the canvas and draw:

canvas = SvgCanvas(width=520, height=220)
canvas.add_element(svg_w)
canvas.add_element(svg_underline)
canvas.draw(output_path)

Of course it's compatible with groups :

links_group = SvgGroup()
links_group.add_elements([svg_w, svg_underline])
links_group.translate(5, 5)
canvas.add_element(links_group)
canvas.draw(output_path)

Voila :

cubic_bezier_link

@LudoBroche
LudoBroche requested a review from loichuder July 7, 2026 15:24
@LudoBroche
LudoBroche requested a review from a team July 8, 2026 09:27
@LudoBroche LudoBroche changed the title Implement first path object with example [Feature]: Implement B-spline SVG path component for workflow links #15 Jul 8, 2026
@LudoBroche
LudoBroche marked this pull request as ready for review July 9, 2026 08:04
Comment thread src/ewoksdraw/css_styles/css_task_line.css Outdated
Comment thread src/ewoksdraw/geometry/__init__.py Outdated
@@ -0,0 +1,3 @@
from .cubic_bezier_path import CubicBezierPath # noqa: F401
from .cubic_bezier_path import CubicBezierSegment # noqa: F401
from .cubic_bezier_path import Point # noqa: F401

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would not export the members of cubic_bezier_path here since this adds maintenance for not much.

Members can be imported by users and consumer modules from their original location.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agree

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed all init.py imports

Comment thread src/ewoksdraw/geometry/cubic_bezier_path.py Outdated
Comment thread src/ewoksdraw/svg/svg_element.py Outdated
if tag not in ("rect", "circle", "text", "line"):
if tag not in ("rect", "circle", "text", "line", "path"):
raise ValueError(
f"Invalid SVG tag: {tag}. Supported tags are 'rect', 'circle', 'text',"

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
f"Invalid SVG tag: {tag}. Supported tags are 'rect', 'circle', 'text',"
f"Invalid SVG tag: {tag}. Supported tags are 'rect', 'circle', 'text',"path", "

I would suggest to make a constant out of the supported tags to avoid this issue in the future.

"""

if self._tag == "path":
return

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we print a warning there?

@LudoBroche LudoBroche Jul 13, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Absolutely

        if self._tag == "path":
            warnings.warn(
                "set_position() has no effect on 'path' elements; position is "
                "defined by the path data instead.",
                stacklevel=2,
            )
            return

Comment thread .gitignore Outdated
Comment on lines +59 to +61
def test_stroke_width_formatted_without_trailing_zero():
link = SvgLinkCubicBezier(SIMPLE_PATH, stroke_width=4.0)
assert link.get_attr("style") == "stroke-width:4"

@loichuder loichuder Jul 13, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is the point of this test? Is "stroke-width:4.0" not valid SVG?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You right I wanted to reach that line of code.
I can do it in the previous test since it's just a consmetic formating.

(None, None, None, None),
],
)
def test_style_attribute_combinations(color, stroke_width, stroke_dash, expected_style):

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The test is so simple that it could be made more readable by doing the combinations one after the other rather than using parametrize.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done split the tests in the different style calls.

@LudoBroche
LudoBroche requested a review from loichuder July 13, 2026 12:19
@@ -0,0 +1,98 @@
from dataclasses import dataclass
from typing import Self

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
from typing import Self
import sys
if sys.version_info < (3, 11):
from typing_extensions import Self
else:
from typing import Self

return (1 if end[0] > start[0] else -1, 0)


def _distance(start: Point, end: Point) -> float:

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This distance is computed with the L1 norm, not the usual L2. Is this normal?

"""

def __init__(self, width: int, height: int):
def __init__(self, width: int | float, height: int | float):

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

from xml.etree.ElementTree import Element

SvgTag = Literal["rect", "circle", "text", "line", "path"]
SUPPORTED_TAGS: Final[tuple[SvgTag, ...]] = get_args(SvgTag)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Didn't know about get_args, pretty sweet.

Final is probably overkill since it does not raise an error at runtime and we don't run type checking.

path: CubicBezierPath,
color: str | None = None,
stroke_width: float | None = None,
stroke_dash: str | None = None,

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
stroke_dash: str | None = None,
stroke_dasharray: str | None = None,

Might as well use the same name as the SVG property 🤷

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Feature]: Implement B-spline SVG path component for workflow links

2 participants