Upstream svgpathtools improvement #233
Conversation
…and add cubic curve instead 'arc' command in path-data is not supported in vglite toolkit Signed-off-by: Harikrushna Parmar <harikrushna.parmar@nxp.com>
There needs to be a space between command and point data so that we can split draw commands and its arguments correctly. Signed-off-by: Nilam Gaikwad <nilam.gaikwad@nxp.com>
…missing or empty values Signed-off-by: Kratika Jain <kratika.jain@nxp.com>
…if no dedicated end path is provided Introduce an 'is_polygon' flag in 'polygon2pathd' function to handle the end path in polyline and polygon Signed-off-by: Harikrushna Parmar <harikrushna.parmar@nxp.com>
if path string or is_polygon flag is enableed then append path given in input if path string and is_polygon flag is enable then append 'z' path Signed-off-by: Harikrushna Parmar <harikrushna.parmar@nxp.com>
…st point and draw clockwise Signed-off-by: Harikrushna Parmar <harikrushna.parmar@nxp.com>
Add Modified by NXP Copyright Signed-off-by: Nilam Gaikwad <nilam.gaikwad@nxp.com>
NGExplorer
changed the title
|
@mathandy can you please review and integrate this PR? |
svgpathtools/svg_to_paths.py
Outdated
| points = COORD_PAIR_TMPLT.findall(polyline.get('points', '')) | ||
| raw_points = polyline.get('points', '') | ||
| if not raw_points: | ||
| points = [(0,0)] |
There was a problem hiding this comment.
points = [(0,0)]
This doesn't seem right, can you explain?
There was a problem hiding this comment.
Hello @mathandy ,
Explanation:
In polyline2pathd() Line #L65 closed variable computation don't validate if there are points.
e.g. in one of SVGT12 test vector following was generating exception
As a result we used [(0,0)].
While more digging into SVGT12 specification we find that
Reference: https://www.w3.org/TR/SVG2/shapes.html#PolygonElement
The initial value, (none), indicates that the polygon element is valid, but does not render.
As a result, we've modified our application to not call polyline2pathd() in such cases.
We will update this PR in 1-2 days after internal testing.
svgpathtools/svg_to_paths.py
Outdated
| ryKappa = ry * PATH_KAPPA; | ||
|
|
||
| return d + 'z' | ||
| #According to the SVG specification (https://lists.w3.org/Archives/Public/www-archive/2005May/att-0005/SVGT12_Main.pdf), |
There was a problem hiding this comment.
I think this should be an option, we should let users continue using arcs if they want, and for now at least.
Let's make that the default unless there is a very good reason this new behavior should be the default.
We could
- add an argument like
use_cubics=False - we could also default to True if some environmental variable is set
There was a problem hiding this comment.
Hello @mathandy , We have incorporated this suggestion. We'll upstream our changes in this PR in 1-2 days.
Done! Sorry for the delay and thanks for the nice MR. I suggested some changes. Please keep in mind, I try to avoid breaking changes when possible and practical in this repo. |
cc @NGExplorer |
|
Hello @mathandy , Thank you for sharing improvements. We'll incorporate this suggestions and update you. Best Regards, Nilam |
Hello @mathandy , We have incorporated this suggestion. We'll upstream our changes in this PR in 1-2 days. Co-authored-by: Andrew Port <andyaport@gmail.com>
Add `use_cubics` flag to optionally select between arc and cubic Signed-off-by: Harikrushna Parmar <harikrushna.parmar@nxp.com>
…data in polylines and polygons Signed-off-by: Harikrushna Parmar <harikrushna.parmar@nxp.com>
|
We have incorporated all of your suggestions, please review. Best Regards, Nilam |
| x3, y3 = x, y + h | ||
|
|
||
| d = ("M{} {} L {} {} L {} {} L {} {} z" | ||
| d = ("M {} {} L {} {} L {} {} L {} {} z" |
There was a problem hiding this comment.
Does this change have an effect?
|
I need to update the CI. Working on it. |
|
@NGExplorer CI is working again 🎉. I've also dropped support for python<3.8 (so we can now use type annotations, which I'm personally happy about). I made some additional requests, sorry I missed these issues the first time around. I may need to run CI manually for you (due to github rules for first time contributors in a repo possibly), but I'll keep an eye out for updates and run it as soon as I see any. Thanks again! |
|
^ cc @NilamG |
4 participants
Fixes #232
Some GPU does not have 'arc' so we can use cubic bezier for such case.
When GPU does not support 'arc' command Circle can be rendered using 2 arcs
@mathandy Please review and let us know if any changes are required.