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

Refactor GraphEdit connections #40513

Closed

Conversation

Jummit
Copy link
Contributor

@Jummit Jummit commented Jul 19, 2020

There was code to draw bezier curves in graph_edit.cpp, which I removed to use Curve2D instead.
This PR makes it possible to retrieve the points of a connection line between two given points, and to customize how connections are drawn:

Examples of custom connections
func _get_connection_line(from: Vector2, to: Vector2):
	var distance = to - from
	return [from, from + Vector2(distance.x / 2, 0), to - Vector2(distance.x / 2, 0), to]

straight connection

func _get_connection_line(from: Vector2, to: Vector2):
	var distance = to - from
	return [from, to]

direct connection

When _get_connection_line isn't overriden:

curve

By adding a way to access the connection line, it makes it possible to implement interaction, like putting nodes in between or adding reroute nodes:

demo

Gist:
https://gist.github.com/Jummit/545d193b9f5119a428862abfefe4be53

Issues to fix:
  • not tested on the latest build, as my computer can't run Vulkan
  • Colors don't work
    Example
    2020-07-20-122233_2646x1024_scrot
  • I'm not sure about the get_connection_line name, as it's very close to get_connection_list.
  • inconsistent with the old line when the connection goes backwards (or is this ok?)
    Example
    image
    Slide Comparison: https://imgsli.com/MTkyOTc

@Jummit Jummit force-pushed the refactor-graphedit-connections branch 4 times, most recently from d0a34ca to 4585c68 Compare July 19, 2020 18:40
@Calinou Calinou added cherrypick:3.x Considered for cherry-picking into a future 3.x release enhancement topic:gui labels Jul 19, 2020
@Calinou Calinou added this to the 4.0 milestone Jul 19, 2020
@swarnimarun
Copy link
Contributor

swarnimarun commented Jul 20, 2020

I wanted to do it soo badly.
You have my thanks.
(Now if someone adds better styling support for graph/graphnodes and then zooming(I mean finish the PR), I will be soo happy that I will scream... :p)

Once it's done I will see if I can find time to add these features to VisualScript and VisualShader Editor.

Will review it once I have some free time.

I have like 2 questions,

  1. How does it handle overlapping?
  2. What do you mean by wrong result when backwards.?
    How is it affected by overloading get_connection_line?

Also you can do, "get_connection_spline" instead more technically accurate I think.

@Jummit
Copy link
Contributor Author

Jummit commented Jul 20, 2020

I wanted to do it soo badly.
You have my thanks.

No problem.

1. How does it handle overlapping?

Like the old system, there should not be any changes. It still uses draw_polyline_colors, just with different points.

2. What do you mean by wrong result when backwards.?
   How is it affected by overloading get_connection_line?

It is inconsistent with the old look:

https://imgsli.com/MTkyOTc

Tell me if that's fine, or if it should be made to look more like the old line.

Also you can do, "get_connection_spline" instead more technically accurate I think.

It's only a spline if you use a Curve2D (the default), but you can add any line, as shown in the examples.

@akien-mga
Copy link
Member

Needs a rebase on current master to fix CI.

@swarnimarun
Copy link
Contributor

swarnimarun commented Jul 20, 2020

Also you can do, "get_connection_spline" instead more technically accurate I think.

It's only a spline if you use a Curve2D (the default), but you can add any line, as shown in the examples.

I mean splines can be linear as well, spline represents a set of line segments between the two adjacent data points

Aside: If I am wrong pls correct me. :3

@Jummit Jummit force-pushed the refactor-graphedit-connections branch from 4585c68 to 3539150 Compare July 20, 2020 11:45
…rve insteadallow retrieving the points of a connection lineallow custom connections lines
@Jummit Jummit force-pushed the refactor-graphedit-connections branch from 3539150 to b48ca93 Compare July 20, 2020 11:50
@Jummit
Copy link
Contributor Author

Jummit commented Jul 20, 2020

Needs a rebase on current master to fix CI.

Done.

@Jummit Jummit marked this pull request as ready for review July 20, 2020 12:02
@Jummit Jummit requested a review from a team as a code owner July 20, 2020 12:02
@fire
Copy link
Member

fire commented Jul 21, 2020

I and @swarnimarun will review on a spare moment.

@fire
Copy link
Member

fire commented Jul 29, 2020

I'm looking at the code and the resulting lines are too thin.

image

It seems to be related to

p_where->draw_polyline_colors(points, colors, 2 * EDSCALE);

However, I don't remember where the default for this is.

@fire
Copy link
Member

fire commented Jul 29, 2020

RenderingServerCanvas::canvas_item_add_polyline has not implemented width. I'll have to circle back to that.

@aaronfranke
Copy link
Member

@Jummit Is this still desired? If so, it needs to be rebased and tested on the latest master branch.

@Jummit
Copy link
Contributor Author

Jummit commented Feb 27, 2021

I'm hesitant to work on it because I can't test the master branch, and there are some graphical inconsistencies I would need to look into. So closing this, but it's pretty salvageable if anyone would like to pick it up. If I ever get a PC that supports Vulkan I'll rebase and test it.

@akien-mga akien-mga removed the cherrypick:3.x Considered for cherry-picking into a future 3.x release label Mar 1, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants