border-radius property for node shapes with rounded corners

[EliotRagueneau]

Description of new feature

It would be really great if users could easily specify what is the border-radius of all the "rounded" node shapes.
Additionally, adding this property would enable effectively new shapes, such as the pill shape which is just a rounded rectangle with a corner radius of half the height

Motivation for new feature

In the reactome-cytoscape-style library we're developing, we need bigger border-radius than the ones provided by default .
At the moment we're using background-image svgs to do it, but that is extremely heavy for something that could just be a simple property of a shape, without impacting too much performances (I believe).

We would like to boost our performances by using native shape renderers instead of custom SVGs

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.

[EliotRagueneau]

This is I guess somehow related to #1167 , but it never got through.
In my context I don't care about parent nodes specifically, just nodes in general

[maxkfranz]

Adding initial thoughts while I'm online just now: A large border radius on compound parent nodes would be troublesome. The bounds of the parent are defined by the collective bounds of the descendants -- a rectangle. A large border radius on a parent would allow a child to visually escape the bounds of the parent at the corners, possibly escaping entirely or possibly only overlapping the border.

For non-parent nodes, large border radii are probably without conflict.

Some initial options:

(1) Forbid custom border radii on parent nodes.

• Inexpensive to implement
• Consumers of the library can't make mistakes

(2) Allow custom border radii on parent nodes and put harsh warnings and mitigations in the documentation.

• Inexpensive to implement
• Consumers of the library can easily make mistakes
• One mitigation: Set a padding that's appropriate to the radius

(3) Implement more complex bounds checking on compound parents.

• Expensive to implement
• May not be practical to support properly in headless environments
• Would allow for more compound parent shapes, e.g. ellipses
• May significantly slow down compound graphs

If you have other options in mind, please list them in this issue's thread.

[EliotRagueneau]

In my opinion, option 2 seems like the best solution.
Specially if we set by default a padding equal to the border-radius, in which case it would dodge the issue. This combined with proper documentation of the impact of border-radius on the padding should be enough I assume

[maxkfranz]

@EliotRagueneau, we probably can't change the defaults without affecting forwards compatibility.

@chrtannus @mikekucera, would you weigh in on this feature?

[mikekucera]

I agree that option 2 seems the best. If we forbid border-radius on compound nodes it might prevent mistakes, but it could just as easily lead to frustrated developers wondering why its not working on compounds. I agree about warnings in the documentation. The documentation should probably include an example of the selector that's needed to apply border-radius to only non-parent nodes.

Re. default padding for compound nodes... It won't affect any existing networks because border-radius would be a totally new property not used by any existing networks. Compound nodes already have a different default style from regular nodes, so it seems like it would be fine, unless I'm missing something. Although the padding would have to grow in relation to the border radius, so pill-shaped compound nodes might end up with a lot of padding, which might not look good. I'm on the fence about this.

[maxkfranz]

Re. default padding for compound nodes... It won't affect any existing networks because border-radius would be a totally new property not used by any existing networks. Compound nodes already have a different default style from regular nodes, so it seems like it would be fine, unless I'm missing something. Although the padding would have to grow in relation to the border radius, so pill-shaped compound nodes might end up with a lot of padding, which might not look good. I'm on the fence about this.

I agree that (2) seems to be the most pragmatic.

Could it be automatic by default without adding a new enum value on padding (e.g. match-radius)? Further, each shape may be very different (e.g. ellipse) if we support new compound parent shapes in future, so an automatic value may be complex or non-pragmatic.

We could allow for automatic padding if the user has set a default (e.g. 0 or 10, whatever it is), but this would be counterintuitive if you explicitly set the same value as the default.

It may be best to start simple (i.e. everything manual) -- and caveat emptor. The docs may just need prominent warnings and appropriate snippets and demos for this use case.

Is there any conflict with this manual approach if we would consider enhancing it with features like automation in future?

Great discussion so far. Let's scope this out a bit more, and I'm sure we'll be able to integrate something effective into the lib.

Thanks, everyone

[EliotRagueneau]

Personally, either solutions works fine as I won't be using border-radius for containers, so we could start with just warning in the documentation warnings, and maybe implement the auto margins in the next breaking changes release.

Are you waiting for an input from me, or further discussion with the rest of Cytoscape team to start tackling this issue? Should I create a PR for this?

[maxkfranz]

@EliotRagueneau, feel free to start your PR. I agree that the fully-manual option is best to start. Even if we decide to support the automatic enum value, or similar, we'd need to start with the fully-manual implementation anyway.

If anyone has further thoughts on the issue, feel free to post it here in the thread.

[EliotRagueneau]

Hi @maxkfranz , I have started working on this, and while I managed to create the property corener-radius property, testing it made me realise 2 things

1. I need to limit the corner radius so that it doesn't take more than half the distance between the previous and the next point, in order to avoid lines going back and forth
2. The current algorithm to create rounded polygons is not so great: it creates artifacts when the polygon is not regular, e.g.

I'm gonna try to improve that at the same time

[EliotRagueneau]

https://stackoverflow.com/a/56214413/11028828
This algorithm, though it is using quadratic curves instead of proper arcs, looks quite good to me, and is simpler to understand.

The problems arrive with sharp corners though ...

[EliotRagueneau]

This algorithm https://stackoverflow.com/a/44856925/11028828 is closer to your previous one, and is producing great results in my opinion.

Should I go forward with this algorithm and try to optimise it is within Cytoscape context?
This also implies some modifications to the inersect-line algorithm, as well as to the checkPoint algorithms

[maxkfranz]

This algorithm https://stackoverflow.com/a/44856925/11028828 is closer to your previous one, and is producing great results in my opinion.

This is probably closer to what people would expect when they think of rounded corners. This approach would be the same as the border-radius in CSS proper for rounded divs, so it sounds sensible.

Should I go forward with this algorithm and try to optimise it is within Cytoscape context? This also implies some modifications to the inersect-line algorithm, as well as to the checkPoint algorithms

Sounds good

[EliotRagueneau]

I've figured out that I could reuse this strategy for #3201 so I'll probably do 1 PR with both new "round" aspects together.

Here's what it looks like when I apply the same roundness to each corner of an edge

[EliotRagueneau]

Question regarding rounded rectangle: was there a reason why you didn't use canvas roundRect() function but instead used your custom polygon function ?

[maxkfranz]

Canvas roundrect() is too new, and the lib supports legacy browsers. You'd need the manual implementation anyway as a polyfill.