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

Playground: Color sockets based on node input/output types #27759

Open
wants to merge 9 commits into
base: dev
Choose a base branch
from

Conversation

abc013
Copy link
Contributor

@abc013 abc013 commented Feb 17, 2024

Description
The first commit of this PR adds two more hotkeys: 'escape' for deselecting nodes, and 'delete' for deleting the selected one.
Previously, only some in- and ouputs were rendered with color based on their type. This PR aims to fix that by streamlining the coloring (and setting of the type length, see in example below) into separate functions.

Previously, it looked like this:
image

Which now looks like this:
image
As you can see, all types now incluence the color of sockets and connections. Furthermore, the amount of lines rendered per connection now correctly represents the length of the node behind that.

Example file used here can be downloaded here.

Some notes

  • The RGB-coloring of multiple lines rendered per connection disappears when setting a custom color (see the 'color' node output in the example above), however, I find this to be a nice effect. Since this is a change in the flow.js module, I'll probably open a PR there.
  • I hardcoded node type lengths in the playground files, however, I am wondering whether (or how) one can access node type lengths somewhere else?
  • Math nodes, such as Multiply or Addition, don't have a type, really (They are using the 'node' type), thus their sockets are rendered in a separate color. However, i am wondering whether, given two input types, one can determine what the output type of given node will be?

I am quite new to the code and hope I haven't overlooked anything. I am open for further suggestions and requests. :)

@sunag
Copy link
Collaborator

sunag commented Feb 17, 2024

I suggest separating the PR shortcuts from the colors.

I didn't find a significant improvement in colors. In the example below I can see the number of connections indicating the type of vector and the RGBA colors by channel, I think the current design is more intuitive than decorating the color of the vectors, I think it's more beautiful too.

Current PR
image image

@abc013
Copy link
Contributor Author

abc013 commented Feb 17, 2024

I suggest separating the PR shortcuts from the colors.

Done. #27762 :)

I didn't find a significant improvement in colors. In the example below I can see the number of connections indicating the type of vector and the RGBA colors by channel, I think the current design is more intuitive than decorating the color of the vectors, I think it's more beautiful too.

I see and I think I do understand. For now, let's ignore the colors for a second and talk about the number of connections: The PR also corrects some nodes which output vectors (and thus, when connected, should have three connections), but only have a single connection. You can see it in the sample:

Original PR
image image

Ignoring the colors, is this change fine?

Back to colors: My hope is that, when using separate colors for separate types, it becomes more obvious to users on the first glance what kind of in- and outputs a node has and, thus, what to plug where and what the result might be when connecting to other nodes. Especially when only having a single connection between two sockets, it's hard to recognize the type: It might be a float, a Material, a bool, a string, or something else (most cases were already color-coded here).
When having multiple connections, these are mostly a vector or a color: Imo the main difference is that colors might be interpreted as vectors, but are mostly limited to the range of [0;1] per element. Here, the goal would be to get more attention to the difference. This might help if people are, as shown in the example, plugging vector inputs into color outputs. Here I can see personally that something with a bigger range gets plugged into something ranged, which could lead to other results than expected.
If you don't think that this is useful, however, it is also okay for me to drop this change, since I do now understand the motivation behind the number of connections.

I agree that the RGB-Colorcoding of the separate connections is a nice touch, and I would like to keep it in a way such that it is there but tinted with the custom color for the connection as a compromise, if you want. Here is an example:

Original PR PR with changes to flow.js
image image image

I quickly implemented the color mixing here: https://github.com/abc013/three.js/tree/propercoloring2 (Relevant lines are 1933 and 1939 in min.flow.js, if you want to play around with the values)

Is that an improvement to you, or should I drop the whole multiple connection coloring at once?

I also realized that the color node does not support inputting a alpha value, so the connection count should be 3, instead of 4. I fixed that in the recent push.

Is there anything else I could do to improve the coloring so it fits you better?

@sunag
Copy link
Collaborator

sunag commented Feb 17, 2024

Thank you for that :)

Ignoring the colors, is this change fine?

Yeah, this improvement makes sense to me.

Would it be possible to implement this in another PR? and we will be able to think about colors more carefully.

@abc013
Copy link
Contributor Author

abc013 commented Feb 20, 2024

Would it be possible to implement this in another PR? and we will be able to think about colors more carefully.

That shouldn't be a problem, I'll do it as soon as possible :)

@abc013
Copy link
Contributor Author

abc013 commented Mar 9, 2024

Would it be possible to implement this in another PR? and we will be able to think about colors more carefully.

See #27891. Sorry, it took a while.

Updated this PR accordingly to be based upon #27891.

@abc013 abc013 changed the title playground: Color sockets based on node input/output types Playground: Color sockets based on node input/output types Mar 25, 2024
@abc013
Copy link
Contributor Author

abc013 commented Mar 25, 2024

@sunag since #27891 has been merged, when can get back to this. Have you thought about the color tinting thing yet?

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

Successfully merging this pull request may close these issues.

2 participants