cytoscape / cytoscape.js

Graph theory (network) library for visualisation and analysis
https://js.cytoscape.org
MIT License
10.14k stars 1.64k forks source link

New curve type: circle arcs #3159

Open sroelants opened 1 year ago

sroelants commented 1 year ago

New curve type: circle arcs

(Even though it wasn't mentioned there explicitly, this is another Feature Request spinning out from https://github.com/cytoscape/cytoscape.js/discussions/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.

A circle arc

Compare this to the equivalent Bezier curve:

An equivalent bezier curve

They also allow for "larger" arcs than Bezier curves, with very little effort:

A distincly circular arc

I would propose a new curve-shape: arc style. Its shape is defined by a single control-point-distance value that would measure the distance of the middle of the arc to the line connecting the two nodes (very similar to how control-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.

A bundle of circle arc edges

This leaves us with a couple of options:

  1. We need to add unbundled arcs under an unbundled-arc style, when there isn't a regular ("bundled") arc type available yet.
  2. Break the naming pattern established with bezier/unbundled-bezier and instead opt for arc and bundled-arc.
  3. Do the work for both bundled and unbundled at once (looking at the code for 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:

maxkfranz commented 1 year ago

This sounds like a great feature.

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.

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.

Its shape is defined by a single control-point-distance value that would measure the distance of the middle of the arc to the line connecting the two nodes (very similar to how control-point-distance works for Bezier curves).

I'd strongly advise against overloading the control-point-distance property, as that would be to the detriment of DX and consistency. Other edge types that require dimensions to be provided have their own properties, e.g. segments.

I would lean towards option number 3, but happy to leave that decision on the table for now.

Notes:

  1. I suspect that most people in most use cases where arcs are applicable would want the arcs in your first figure (i.e. semicircle) rather than your third figure (i.e. nearly full circle).
  2. It's important to consider how this will play when the dev has specified fixed edge endpoints.
  3. Another use case to consider is an automatic radius size.
  4. Deciding the location of the centre of the circle is important. They're very different in figure (1) and figure (3).
  5. Deciding what properties to expose and how they affect the edges in interactive use cases is important. Do the edges still look as expected when the user drags one of the nodes?
sroelants commented 1 year ago

I'd strongly advise against overloading the control-point-distance property, as that would be to the detriment of DX and consistency. Other edge types that require dimensions to be provided have their own properties, e.g. segments.

Fair enough, something like arc-distance would do the job just fine!

Another use case to consider is an automatic radius size.

I'm not sure I follow what you mean by this. Care to elaborate?

Deciding what properties to expose and how they affect the edges in interactive use cases is important. Do the edges still look as expected when the user drags one of the nodes?

I guess the two options here are:

  1. have the arc-distance be in regular ("graph") coordinates. The arc will deform as the nodes are moved around, but I feel like this is the natural behavior.
  2. Have the arc-distance be in coordinates relative to the separation between the nodes. This means the arc effectively "scales up and down" as the nodes are moved.

My personal preference would be option (1), but happy to go with either behavior.

maxkfranz commented 1 year ago

Another use case to consider is an automatic radius size.

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.

  • have the arc-distance be in regular ("graph") coordinates. The arc will deform as the nodes are moved around, but I feel like this is the natural behavior.
  • Have the arc-distance be in coordinates relative to the separation between the nodes. This means the arc effectively "scales up and down" as the nodes are moved.

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 arcTo(), there are several limitations that touch upon the ST distance issue: https://developer.mozilla.org/en-US/docs/Web/API/CanvasRenderingContext2D/arcTo

sroelants commented 1 year ago

It would help to define how exactly new style properties would affect the edges, especially with respect to how the radius is determined.

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:

arc-distance

I only labeled the "control point" there to highlight the similarity with a bezier curve with a single control-point-distance at control-point-weight: 1/2.

The user only needs to provide the distance labeled arc-distance in the drawing (arc-distance being the new style property we'd be introducing). We can then calculate the control-point behind the scenes (again, much like for bezier curves), and find the correct circle arc.

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.

maxkfranz commented 1 year ago

Great. Do you envision that arc loops would use something analogous to the control point step size to have consistent distance between subsequent loops?

sroelants commented 1 year ago

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 step-size parameter for loops, it does lead back to that question of "why do we not also allow a step-size parameter for regular arc edges?"

maxkfranz commented 1 year ago

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.

stale[bot] commented 1 year ago

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.

stale[bot] commented 1 year ago

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.

maxkfranz commented 1 year ago

Pinned