-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
New curve type: circle arcs #3159
Comments
This sounds like a great feature.
I'm not convinced that arc edges would be useful in a bundle as compared to beziers. Beziers can be much tighter, which is important for multigraphs.
I'd strongly advise against overloading the
Notes:
|
Fair enough, something like
I'm not sure I follow what you mean by this. Care to elaborate?
I guess the two options here are:
My personal preference would be option (1), but happy to go with either behavior. |
Let's say we have two nodes, source S and target T, connected by arc edge E. If the distance between S and T is larger than the arc radius of E, then there's no way to connect S and T with E. The circle is too small. Given that the graphs are dynamic (e.g. you can drag nodes around), many devs will want a convenient way to say 'just make the radius something that works regardless of the distance because I don't want the edge to randomly glitch out'. A bezier edge gives flexibility re. ST distance, whereas an arc edge would be fairly restricted if it has a fixed radius. Another point to consider is loops, as rightly pointed out by one of my colleagues. Arc edges would be perfect for loops (self edges, where the source and target are the same). Lots of use cases with loops can have multiple loops per node. This would be a strong motivation for supporting bundling upfront.
These points are related to the ST distance. The second option allows for some self-correction. It would help to define how exactly new style properties would affect the edges, especially with respect to how the radius is determined. It would also help to identify some cases where arc edges would be useful (e.g. loops) and other cases where they wouldn't work even though, prima facie, they might seem to work (e.g. you have a row of nodes and want all the edges to be nicely curved with the same 'height'). Identifying the specious use cases may be as important as identifying the proper ones. If you're planning on using |
I'm sorry, I must've been a bit unclear in my original explanation. The way I imagined it working is something more like the following: I only labeled the "control point" there to highlight the similarity with a bezier curve with a single The user only needs to provide the distance labeled Like you've correctly pointed out: using the radius won't work, because we're not guaranteed to always have a circle of the provided radius going through both nodes. With this construction, though, we're always guaranteed to have a circle, and it behaves smoothly when nodes are moved around. |
Great. Do you envision that arc loops would use something analogous to the control point step size to have consistent distance between subsequent loops? |
I think that would make sense, but I guess that brings us back to the question of whether or not we should have a bundled and unbundled variant, no? EDIT: Never mind, I misunderstood your comment. Yes, for loops I could imagine providing a step size, though I'm not sure if there's any need. We could equally just expect the users to provide a "control-point-distance" the same way they do for any other arc edge. If we introduce a kind of |
Yes, since loops are important and since step sizes are important for loops, we may as well support step sizes -- and therefore bundling -- for arc edges generally. |
This issue has been automatically marked as stale, because it has not had activity within the past 14 days. It will be closed if no further activity occurs within the next 7 days. If a feature request is important to you, please consider making a pull request. Thank you for your contributions. |
This issue has been automatically marked as stale, because it has not had activity within the past 14 days. It will be closed if no further activity occurs within the next 7 days. If a feature request is important to you, please consider making a pull request. Thank you for your contributions. |
Pinned |
New curve type: circle arcs
(Even though it wasn't mentioned there explicitly, this is another Feature Request spinning out from #3145)
Description of new feature
What should the new feature do? For visual features, include an image/mockup of the expected output.
Currently, Cytoscape.js only supports quadratic Bezier curves (or several of them combined as a spline) as a generic way to create curved edges. Circle arcs would be a second "curved" shape that ends up being very distinct from Bezier curves in many respects.
Compare this to the equivalent Bezier curve:
They also allow for "larger" arcs than Bezier curves, with very little effort:
I would propose a new
curve-shape: arc
style. Its shape is defined by a singlecontrol-point-distance
value that would measure the distance of the middle of the arc to the line connecting the two nodes (very similar to howcontrol-point-distance
works for Bezier curves). That is to say, we could reuse the same style property and minimize the number of user-facing changes.Motivation for new feature
Describe your use case for this new feature.
While it's possible (but difficult) to emulate circle arcs by stringing together several bezier curves, it becomes painful (and expensive) to make sure those curves retain their shape when nodes are moved around, to make sure the approximation looks passable in all possible configurations, etc...
In many cases, circular arcs are more desirable, stylistically speaking, and they are more intuitive to the average user than bezier curves.
Edge cases to consider
It feels very natural to allow both "bundled" and "unbundled" arcs, similar to what is currently possible for Bezier curves. If this feature is considered interesting enough to pursue, it would make sense to scope the initial PR to "unbundled" arcs, and add "bundled" arcs as a follow-up PR.
This leaves us with a couple of options:
unbundled-arc
style, when thereisn't a regular ("bundled")
arc
type available yet.bezier
/unbundled-bezier
andinstead opt for
arc
andbundled-arc
.bundled/unbundled bezier curves, and based on an initial prototype I did,
this would be fairly little additional code)
I would lean towards option number 3, but happy to leave that decision on the table for now.
For reviewers
Reviewers should ensure that the following tasks are carried out for incorporated issues:
unstable
branch via pull request. The corresponding pull request is cross-referenced.The text was updated successfully, but these errors were encountered: