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

Centripetal Catmull-Rom spline for curve edges? #115

Open
ariutta opened this issue Apr 23, 2021 · 9 comments
Open

Centripetal Catmull-Rom spline for curve edges? #115

ariutta opened this issue Apr 23, 2021 · 9 comments

Comments

@ariutta
Copy link
Member

ariutta commented Apr 23, 2021

Problem: Cytoscape uses center of node as start and end points, while PathVisio uses a point on the perimeter of the node. This means the curves don't match when you import a WikiPathways network as pathway. You can see it most clearly for an edge connected to another edge via an anchor. Since the curves are not identical, the edges often don't actually touch.

Potential solution: calculate ahead of time x,y positions for anchors and points along the path. Use the same algorithm/library as PathVisio to ensure the positions will match. Then to draw the curve in Cytoscape, use a different algorithm like the centripetal Catmull-Rom spline. For this algorithm, you just need to specify an x,y for each point along the path, and the algorithm draws a smooth line through the points. That way, if a curve passes through the x,y of an anchor and another edge starts or ends at that anchor, they are guaranteed to touch.

@ariutta
Copy link
Member Author

ariutta commented Apr 23, 2021

(This solution does not directly apply to straight lines with anchors.)

@AlexanderPico
Copy link
Member

https://www.wikipathways.org/index.php/Pathway:WP430 provides a good example of Cytoscape (left) vs. PathVisio (center) vs. pvjs (right)
Screen Shot 2021-04-23 at 11 33 43 AM

@AlexanderPico
Copy link
Member

AlexanderPico commented Apr 23, 2021

From what I can tell. The anchors are all correctly placed in terms of x,y coordinates, but the paths are variable. Pvjs's Bezier curve is differnt from PV's (see Lipoprotein -> HDL) while the x,y anchor placement is identical, resulting in the anchor being off of the pvjs curve path. Cytoscape's curve is the same as PV's, but the curve endpoints are different (centers vs ports). Notice how that affects even the straight connections (see Cholesterol -> HDL). But the x,y anchor placement is identical to PV (and thus off of the Cytoscape paths).

@ariutta
Copy link
Member Author

ariutta commented Apr 23, 2021

Yes, it's two different problems for Cytoscape vs. pvjs. For Cytoscape, we can keep the existing anchor positions and adjust the curves to make them pass through the anchors, using something like the centripetal Catmull-Rom spline. For pvjs, we should make the JS and JavaScript code use identical algorithms.

@AlexanderPico
Copy link
Member

If Cytoscape nodes supported ports, then it would simply solve all cases. This is an ongoing discussion in the Cytoscape dev team, but no impl plan yet. They would be responsible for impl plan and making sure whatever they did worked for both Cytoscape desktop and cytoscape.js. We can simply provide GPML as a test case. So, this has the potential of solving itself (from WP team perspective).

@AlexanderPico
Copy link
Member

The PV to pvjs issue is more minor (thankfully) and solely in the WP team's court (unfortunately). Maybe a ticket in the pvjs repo could track your ideas for a better pairing of curve libs for Java and JS?

@AlexanderPico
Copy link
Member

AlexanderPico commented Apr 23, 2021

A variation on your CCR spline idea for solving the Cytoscape issues: What if we segmented the patha and used a straight line (or whatever) between the center of a node and a virtual point representing where the port should be on the node's perimeter, and then used a normal Bezier (or straight line accordingly) for the segment between virtual ports. It seems like this would result in matching paths in all cases.

However, I'm not sure how to implement this in a way that works dynamically as you move nodes around or adjust node sizes, etc...

This is where paths are currently processed (ConnectorShape.java). We could essentially try handling all edge types as "segmented" then apply the appropriate path algo to the middle segment.

@ariutta
Copy link
Member Author

ariutta commented Apr 23, 2021

That could work. Or you could alternatively update the anchor points to match where the edges are actually drawn in Cytoscape.

@AlexanderPico
Copy link
Member

AlexanderPico commented Apr 23, 2021

Totally. I was kind of surprised to see the anchors in the "right" place instead on along the Cytoscape-calculated paths per the provided percent value.

I suspect (but don't recall) that there are cases where the anchor position is more critical to interpretation than it's perfect alignment with it's parent line. But I don't know...

To be clear, this change could be a better temporary solution that the current one (though it also might not be), but it would not be a proper long-term solution since the original meaning in some cases will still be skewed or lost.

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