Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow to specify miter limit and arc tolerance for polypath offsetting #29758

Closed
wants to merge 1 commit into from
Closed

Allow to specify miter limit and arc tolerance for polypath offsetting #29758

wants to merge 1 commit into from

Conversation

Xrayez
Copy link
Contributor

@Xrayez Xrayez commented Jun 13, 2019

Enhances #28987.
Closes #29886.

Recently I've been thinking that exposing miter limit and arc tolerance parameters for polygon/polyline inflating/deflating would be useful. I questioned whether it's reasonable to add those until this question/request was raised.

clipper_miter

clipper_arc_tolerance

These parameters are added as default and reflect the defaults in ClipperOffset which is used internally (the only parameters which were missing).

@Xrayez Xrayez requested a review from a team as a code owner June 13, 2019 15:56
@Zireael07
Copy link
Contributor

Is that only for graphics, or can I obtain the offset path as Path2D or Vector2Ds? Just curious.

@Xrayez
Copy link
Contributor Author

Xrayez commented Jun 13, 2019

@Zireael07 the result is always an array of actual Vector2 vertices representing polygons/polylines, so you can do whatever you like with those (including drawing), think about it as core functionality. These methods could as well be integrated into "front-end" nodes like Path2D/Polygon2D/NavigationPolygon2D under the hood in the future.

@Xrayez
Copy link
Contributor Author

Xrayez commented Jun 19, 2019

Not sure about arc_tolerance parameter, I didn't see much of a difference in approximation, perhaps it's somehow related to requirement to scaling points before clipping. fixed. It can be dropped so that core/method_bind_ext.gen.inc wouldn't have to be included which would bloat the engine's binary size to my knowledge (if that's even a concern). resolved with workaround in #29758 (comment).

core/bind/core_bind.cpp Outdated Show resolved Hide resolved
@Xrayez
Copy link
Contributor Author

Xrayez commented Aug 23, 2019

Removed core/method_bind_ext.gen.inc to avoid engine binary size bloat.

It's possible to configure arc tolerance when using offset_polygon_2d but not offset_polyline_2d because of limited default number of arguments used for binding (5, need 6!).

The above inconvenience can be remedied by including core/method_bind_ext.gen.inc but then again it could potentially bloat the binary size somewhat (if that's what concerns reduz).

@akien-mga akien-mga modified the milestones: 3.2, 4.0 Nov 7, 2019
These parameters are added as default and reflect the defaults in
ClipperOffset which is used internally (the only parameters missing).

It's only possible to configure arc tolerance for `offset_polygon_2d`
and not for `offset_polyline_2d` method as the latter requires binding
more than 5 arguments to avoid engine binary size bloat. The workaround
to this is to offset the polyline as usual which would produce polygons,
and then use `offset_polygon_2d` method on the result.
@Xrayez
Copy link
Contributor Author

Xrayez commented Feb 19, 2020

Closing, see #36369 which should solve all mentioned limitations.

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

Successfully merging this pull request may close these issues.

Miter limit should be parameter with default value, not hardcoded.
4 participants