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

Lines with rounded corners #158

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Conversation

macfreek
Copy link

So far only rects could have rounded corners. It would be great if lines could also have rounded corners.

Please discuss this patch before merging the code.

This patch is very simple (only two lines) and effective for most scenarios. However, it has three disadvantages:

  1. It uses arcTo() which is tagged as "problematic in Opera" elsewhere in this library.
  2. it does not yet correct for very acute angles, where the radius must be decreased for the rounding to fit.
  3. It is not possible to combine dashed lines with rounded corners

I'm willing to write code that resolves the first two issues, but be aware that this is computationally intensive, so I like some feedback if you are willing to merge such a patch before writing it. As for the third item, I'm sure I can write the code to support both rounded corners and dashes lines, but that really is a mayor effort.

Currently, I'm only interested in rounded corners for lines, and am willing to write that code. Do you want support for other type of shapes too, and if so -- which shapes would you consider?

(edit: not to self: don't mix MediaWiki and Markdown markup languages...)

So far only rects could have rounded corners.
Warning 1: this code uses arcTo() which is tagged as "problematic in Opera" elsewhere in this library.
Warning 2: this code does not yet correct for very acute angles, where the radius must be decreased for the rounding to fit.
Manually making the calculation is possible, but computationally intensive.
Limitation: Combine dashed lines with rounded corners will not be supported (it is possible, but that requires a lot of code)
If it doesn't fit, reduce the radius till it fits.
Note: this code is not optimized yet.
@chrisritter
Copy link

Love to see this in master.

@rsperlazza
Copy link

I would as well. I ask about this a few weeks ago.

@macfreek
Copy link
Author

There seems interest, so I'm following up. Rough results of speed tests for a line with 1000 points:

  • lineTo (current code, no corners): 135 ± 5 ms
  • arcTo (d194f14, no correction for small corners): 200 ± 5 ms
  • arcTo with corrected radius (8c48884, smaller radius for very acute angles): 210 ± 10 ms
  • arc instead of arcTo (12019cc): 180 ± 10 ms

I anticipated that my code (lots of if/then and trigonometry) would severely reduce the speed, but that's not really the case. The arc itself had a drawback. Given that the lineTo is called if the cornerRadius is 0, there is no significant speed penalty for current lines, and thus the code can be merged as soon as I rewrote the arcTo() to a regular arc(). I presume this is requested, right?

@chrisritter
Copy link

Eric, what do you think?

@macfreek
Copy link
Author

@ericdrowell These three commits are my contribution. I'm sure you like to reformat the code to your code conventions (sorry, I could not find the guidelines). If you like to refactor the code (e.g. because you like to reuse it for other shapes), I'm happy to explain some things. I tried to add a bunch of in-line comments, but I needed a picture. I you like that, just let me know.

(The trigonometry was fun, but it is a bit annoying that Javascript -the Math.atan2() function- uses a different coordinate system than what HTML5 canvas uses. The picture at http://www.html5canvastutorials.com/tutorials/html5-canvas-arcs/ helped. Thanks. FYI, Math.atan2() returns 0 rad for a line pointing south, the range is -π to +π and the numbers are going counter clockwise. While for arc() 0 rad is pointing East, the range is 0 to 2π, and it is going clockwise. Try explaining that to my wife when I was cursing when the angles didn't work out at first...)

@ericdrowell
Copy link
Owner

planning on using Rob's excellent spline tutorial to enable curve smoothing for the Line shape

http://scaledinnovation.com/analytics/splines/aboutSplines.html

Do you think this will be sufficient?

@macfreek
Copy link
Author

macfreek commented Dec 7, 2012

Two comments:

  • I would very much welcome the addition of Bezier-curves to KineticJS, either as part of a Line or a new Shape.
  • If you're not going to support rounded corners, please remove them from the documentation. They are currently there, and from a users perspective it is annoying that something is documented but isn't implemented.

@ericdrowell
Copy link
Owner

Agree very much with you there. Will keep doing my best to keep up with documentation. Thanks!

@ericdrowell ericdrowell closed this Dec 8, 2012
@djmccormick
Copy link

Did this ever happen?

@ericdrowell
Copy link
Owner

Update on this thread:

  1. yes, the documentation has been updated
  2. The Spline shape has been created and works great. Here's a tutorial:

http://www.html5canvastutorials.com/kineticjs/html5-canvas-kineticjs-spline-tutorial/

  1. The spline shape is great for creating curved shapes based on points, but sometimes people don't actually want a spline, they want rounded corner connections (as was the original reason for this thread). After a bit more thought, I think it makes sense to keep the Spline shape as is, and also add cornerRadius functionality to Line, afterall. macfreak, I'll take another look at the work you did some 9 months ago.

v4.6.1 is purely for bug fixes at the moment, so I'll try to get this feature added into v4.6.2

Reopening the ticket.

@ericdrowell ericdrowell reopened this Aug 26, 2013
@ghost ghost assigned ericdrowell Aug 26, 2013
@macfreek
Copy link
Author

I would appreciate the inclusion of this feature.

A small recommendation: While writing the code about a year ago, I was thinking that if would be useful to create a few more functions. For example, the Line object had multiple points, and is actually a open (non-closed) Polygon. When drawing such an object with both rounded corners and dashed lines, I would say there should be three steps: (1) calculate arcs and lines based on rounded corners property; (2) calculate arc- and line-segments based on dashed property; (3) actually draw to canvas. When I made the patch, all functions where in the same method, and I was not able to re-use the code to draw dashed arcs (that's presumably present in the Circle object).

I haven't used KineticJS in a while, so I won't be able to help much, and I'm not sure how much of the above still applies. Let me know if you have any questions on the patches, when I'm back from holiday, I'll try to answer them.

@ericdrowell
Copy link
Owner

This will get into the code base soon. I'm actually now planning on supporting rounded corners for all shapes, including rect, polygon, star, regularPolygon, line, etc. This will most likely make it to v4.7.1 or 4.7.2 depending on other priorities (I'm currently working on splitting the Canvas class into a Canvas and Context class, adding context tracing, fixing some major bugs, and migrating unit tests over to Mocha, which will be in v4.7.0)

@ericdrowell
Copy link
Owner

update on this thread. All shapes will support rounded corners with v4.8.0. This will definitely make it into that release. Stay tuned.

@mkrutsky
Copy link

Hi, did this feature ever made it to release? It's 5.1.0 now and I can't seem to find it in documentation...

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.

None yet

6 participants