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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for curved polylines/polygons using B茅zier curves #9286

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

serso
Copy link

@serso serso commented Mar 3, 2024

No description provided.

noClip: false,

// @option curved: Boolean = false
// Enable polyline B茅zier curves. Curved polylines don't support clipping - each line is rendered fully.
Copy link
Member

Choose a reason for hiding this comment

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

This needs to reflect that bezier curves are not supported when polylines are drawn on an L.Canvas.

Copy link
Author

Choose a reason for hiding this comment

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

But that's supported. See changes in src/layer/vector/Canvas.js. SVG drawing commands are transformed into Canvas drawing commands in Canvas.js. I have also added vector-canvas-curved.html to test that.

const cmds = curvedPathCommands(points, closed);
for (j = 0; j < cmds.length; j++) {
const cmd = cmds[j];
switch (cmd.type) {
Copy link
Member

Choose a reason for hiding this comment

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

I can't see the point of holding "command" objects. The non-bezier code can differentiate between lineTo and moveTo by j===0, so there is no need to do an initial array.push of a M "command" and then push C commands.

Copy link
Author

Choose a reason for hiding this comment

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

I don't understand what you mean.
Do you want me to replace curvedPathCommands with in-loop calculation of Bezier curve parameters?
It would be more efficient but then the code in the loop would become more complicated.
Also, for polygons (or polylines with closed=true) curvedPathCommands modifies the first C command at the end of the loop to make the joint smooth. It would not be possible if curvedPathCommands was extracted here.

@IvanSanchez
Copy link
Member

Besides, there's plugins like https://github.com/elfalem/Leaflet.curve , so I must ask the question of what does this need to be in the Leaflet core.

@serso
Copy link
Author

serso commented Mar 4, 2024

so I must ask the question of what does this need to be in the Leaflet core.

My thinking was that smooth drawing (using curves and not just connecting the dots) of polylines/polygons is an essential feature and if the Leaflet core supports polylines/polygons it should also be able to draw them smoothly.

there's plugins like https://github.com/elfalem/Leaflet.curve

This particular plugin doesn't do what I want - it draws curves via SVG commands passed explicitly. I want to draw curved polygons, e.g. have an ordered set of dots and draw curves between smoothly (and automatically). It should be possible though to transform the polygons into SVG commands by hand and pass the result to Leaftlet.curve.

I leave it up to the maintainers to decide whether this should be a core feature of the lib or it should be implemented externally.

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.

None yet

2 participants