Remove polygons, introduce SKPath CU-86c3mbjx8 #60
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR streamlines the SVG generation pipeline by removing intermediate polygon representations and using SKPath + PathOps for boolean operations.
- Directly performs boolean operations on SKPath objects instead of converting to/from polygons.
- Revamps markup generation to iterate SKPath segments and truncate coordinates.
- Cleans up unused geometry modules and updates dependencies (e.g., replacing pyclipper with skia-pathops).
Reviewed Changes
Copilot reviewed 15 out of 15 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| vectorizing/svg/markup.py | Introduces truncate, to_SVG_color_string, and generate_SVG_markup using SKPath segments |
| vectorizing/solvers/color/clip.py | Replaces pyclipper pipeline with pathops.op for difference |
| vectorizing/solvers/color/ColorSolver.py | Removes tolerance parameter, updates solver call signatures |
| vectorizing/geometry/potrace.py | Converts potrace paths to SKPath, removes old flattening helpers |
| vectorizing/geometry/bounds.py | Adds compound_paths_bounds and drops legacy Bounds class use |
| vectorizing/init.py | Switches to new generate_SVG_markup and compound_paths_bounds |
| requirements/* | Bumps package versions, removes pyclipper, adds skia-pathops |
Comments suppressed due to low confidence (2)
vectorizing/svg/markup.py:33
- The variable name
tupleshadows the built-in. Rename it to something likecomponentsorvaluesfor clarity.
tuple = "".join(
vectorizing/geometry/potrace.py:1
- The
numpyimport is no longer used in this file. You can remove it to clean up unused dependencies.
import numpy as np
| d = "" | ||
| for segment in segments: | ||
| command = segment[0] | ||
| if command == "moveTo": |
There was a problem hiding this comment.
[nitpick] Consider using if/elif/else instead of multiple if blocks to avoid checking every condition once a match is found.
| d = "" | ||
| for segment in segments: | ||
| command = segment[0] | ||
| if command == "moveTo": |
There was a problem hiding this comment.
There is no fallback for an unexpected segment command. Consider adding an else branch that raises or logs an error if command is unrecognized.
| # NOTE: The clipper library uses integer coordinates only for numerical robustness. | ||
| # That's why coordinates are scaled by a big factor, to preserve precision. | ||
| def remove_layering(traced_bitmaps, tolerance): | ||
| def remove_layering(traced_bitmaps): |
There was a problem hiding this comment.
Add a docstring to remove_layering to explain its parameters and return value, matching the style of other utility functions.
Description:
This PR updates the pipeline so that now:
trace -> convert to polygons -> perform boolean operations -> convert back to paths -> generate markupis
trace -> perform boolean operations -> generate markupThis makes results much better and reduces path complexity a lot.