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

New curve type: circle arcs #3159

Open
4 tasks
sroelants opened this issue Aug 28, 2023 · 10 comments
Open
4 tasks

New curve type: circle arcs #3159

sroelants opened this issue Aug 28, 2023 · 10 comments

Comments

@sroelants
Copy link

sroelants commented Aug 28, 2023

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.

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:

  • Ensure that the reporter has adequately described their idea. If not, elicit more information about the use case. You should iteratively build a spec together.
  • Ensure that the issue is a good fit for the core library. Some things are best done in extensions (e.g. UI-related features that aren't style-related). Some things are best done by app authors themselves -- instead of in Cytoscape libraries.
  • The issue has been associated with a corresponding milestone.
  • The commits have been incorporated into the unstable branch via pull request. The corresponding pull request is cross-referenced.
@maxkfranz
Copy link
Member

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
Copy link
Author

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
Copy link
Member

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
Copy link
Author

sroelants commented Aug 31, 2023

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
Copy link
Member

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
Copy link
Author

sroelants commented Sep 14, 2023

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
Copy link
Member

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
Copy link

stale bot commented Sep 29, 2023

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 stale bot added the stale label Sep 29, 2023
@maxkfranz maxkfranz removed the stale label Sep 29, 2023
@stale
Copy link

stale bot commented Oct 13, 2023

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 stale bot added the stale label Oct 13, 2023
@maxkfranz maxkfranz added pinned A long-lived issue, such as a discussion and removed stale labels Oct 18, 2023
@maxkfranz
Copy link
Member

Pinned

@maxkfranz maxkfranz removed the pinned A long-lived issue, such as a discussion label Aug 14, 2024
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

No branches or pull requests

2 participants