Migrate the Select tool to the document graph #1433

0HyperCube · 2023-10-08T11:55:22Z

TODO:

Fix with multiple transform nodes
Fix deep select (new implementation ~10 lines - although it probably doesn't match the previous behaviour as I haven't really tested it)
Don't select artboards with select tool.

Keavon · 2023-10-08T20:59:28Z

!build

github-actions · 2023-10-08T21:02:31Z

📦 Build Complete for `7daee21`

Keavon · 2023-10-08T21:58:11Z

!build

github-actions · 2023-10-08T21:59:45Z

📦 Build Complete for `7daee21`

Keavon · 2023-10-09T00:11:14Z

!build

github-actions · 2023-10-09T00:12:59Z

📦 Build Complete for `7daee21`

Keavon · 2023-10-09T06:07:42Z

!build

github-actions · 2023-10-09T06:10:01Z

📦 Build Complete for `7daee21`

Keavon · 2023-10-09T19:20:06Z

!build

github-actions · 2023-10-09T19:22:06Z

📦 Build Complete for `7daee21`
https://ec60da70.graphite.pages.dev

0HyperCube · 2023-10-11T21:34:39Z

!build

github-actions · 2023-10-11T21:37:33Z

📦 Build Complete for `4be5ccf`
https://c049f077.graphite.pages.dev

editor/src/messages/tool/tool_messages/select_tool.rs

Keavon · 2023-10-11T22:00:43Z

How easy would it be to make the Select tool not select the artboard? As soon as I click the artboard, it breaks the rendering of the artwork (but the shape-hover overlays show the current positions after navigating).

I also notice the "Empty Stack" node is appearing as a layer in the (still-to-be-rewritten) layer panel, but if it's a real quick change, it'd be nice to temporarily remedy that until I rewrite the layer panel in the next few weeks.

0HyperCube · 2023-10-11T22:37:56Z

I see, yes that makes sense. Do you agree that preventing the Select tool from interacting with artboards would be the right solution here? (And then only the Artboard tool can do that.)

Fixed @Keavon

Keavon · 2023-10-11T22:41:57Z

!build

github-actions · 2023-10-11T22:44:29Z

📦 Build Complete for `a39f29f`
https://2f9a71ab.graphite.pages.dev

Keavon · 2023-10-11T23:22:50Z

This looks great! Only a few little things I'm noticing:

I can't reproduce it reliably, but thrice I've noticed that as I multi-select (shift-clicking one shape then another in the viewport), it ends up making both shapes disappear permanently from the render until I Ctrl+Z it. We'll have to keep watching this one since I have no idea how to make it happen and it's sort of rare.
The pivot widget should also be grayed out, not hidden, just like the alignment and flip widgets.

I think that's all that I directly notice pertaining to the Select tool, so this should be ready to merge pretty soon then! In fact, I'll just push the pivot widget change now so it's in good shape for a review also by @TrueDoctor and then we'll be good to merge.

After this, I think the most obvious problems users will encounter in this branch is that artboards should be fed into from a layer stack, rather than living at the base of a layer stack; and that selecting a layer in the node graph doesn't show the upstream nodes in the Properties panel. But those are both fine to fix in a separate PR and are probably somewhat approachable by new contributors. Thank you so much for this!

Keavon · 2023-10-12T01:16:19Z

!build

Keavon · 2023-10-12T01:20:10Z

!build

github-actions · 2023-10-12T01:22:06Z

📦 Build Complete for `a68263b`
https://88f27cf3.graphite.pages.dev

TrueDoctor · 2023-10-12T06:02:29Z

I'm not quite happy with the overall approach of relying on node names for special behaviour. We could potentially add a dedicated node tag (or just in general some sort of tagging system) to make the system a bit more robust.
Another issue with the current approach is that we still don't support Flip nodes or other user-custom transform nodes right? But I can address that in a followup pr.

TrueDoctor

21 files left to review, will continue asap

editor/src/messages/portfolio/document/document_message_handler.rs

editor/src/messages/portfolio/document/node_graph/graph_operation_message_handler.rs

0HyperCube · 2023-10-12T17:08:50Z

I'm not quite happy with the overall approach of relying on node names for special behaviour. We could potentially add a dedicated node tag (or just in general some sort of tagging system) to make the system a bit more robust.

I have previously mentioned that the approach of relying on node names and input indices is not robust.

Another issue with the current approach is that we still don't support Flip nodes or other user-custom transform nodes right? But I can address that in a followup pr.

This approach should work fine if there are any sort of transform nodes downstream. I forgot about other nodes being downstream, so we probably need some slight code changes to handle this. Also we may potentially need to fix the code for adjusting the translation (so that the shape appears to stay still as the bounding box is changed in the path tool) to work with custom transform nodes.

I can't reproduce it reliably, but thrice I've noticed that as I multi-select (shift-clicking one shape then another in the viewport), it ends up making both shapes disappear permanently from the render until I Ctrl+Z it. We'll have to keep watching this one since I have no idea how to make it happen and it's sort of rare.

Is there any error logs? Does it still happen when you save and then reload? Do the layers still appear in the layer tree? If you drag a connection between two nodes in the graph UI then does anything change (this causes the layer tree to be recomputed)?

Keavon · 2023-10-16T19:10:51Z

!build

github-actions · 2023-10-16T19:13:22Z

📦 Build Complete for `37b8225`
https://c5b8c1c5.graphite.pages.dev

Keavon · 2023-10-17T16:59:50Z

!build

github-actions · 2023-10-17T17:02:25Z

📦 Build Complete for `549d523`
https://48ba8420.graphite.pages.dev

Keavon · 2023-10-17T17:13:52Z

!build

github-actions · 2023-10-17T17:16:22Z

📦 Build Complete for `b727536`
https://d5c61a09.graphite.pages.dev

Keavon

QA looks great!

* function for accessing document metadata * Better select tool * Fix render * Fix transforms * Fix loading saved documents * Populate graph UI when loading autosave * Multiple transform nodes * Fix deep select * Graph tooltips * Fix flip axis icon * Show disabled widgets * Stop select tool from selecting artboards * Disable (not hide) the pivot widget; remove Deep/Shallow select for now * Code review changes * Fix pivot position with select tool * Fix incorrectly selected layers when shift clicking --------- Co-authored-by: Dennis Kobert <[email protected]> Co-authored-by: Keavon Chambers <[email protected]>

TrueDoctor and others added 4 commits October 8, 2023 12:27

function for accessing document metadata

058b305

Better select tool

9409cef

Fix render

61afc5f

Fix transforms

f25bd7e

0HyperCube mentioned this pull request Oct 8, 2023

Tracking Issue: Document graph migration MVP #1394

Closed

62 tasks

0HyperCube added 2 commits October 8, 2023 19:21

Fix loading saved documents

32ba8b3

Populate graph UI when loading autosave

7daee21

Keavon changed the title ~~Select tool port~~ Migrate the Select tool to the document graph Oct 9, 2023

0HyperCube added 3 commits October 10, 2023 19:14

Multiple transform nodes

2eaca83

Fix deep select

808842a

Graph tooltips

4be5ccf

0HyperCube requested review from Keavon and TrueDoctor October 11, 2023 21:30

GraphiteEditor deleted a comment from github-actions bot Oct 11, 2023

Keavon reviewed Oct 11, 2023

View reviewed changes

editor/src/messages/tool/tool_messages/select_tool.rs Outdated Show resolved Hide resolved

Keavon reviewed Oct 11, 2023

View reviewed changes

editor/src/messages/tool/tool_messages/select_tool.rs Outdated Show resolved Hide resolved

Fix flip axis icon

dffb7ea

0HyperCube added 2 commits October 11, 2023 23:13

Show disabled widgets

721504d

Stop select tool from selecting artboards

a39f29f

0HyperCube requested a review from Keavon October 11, 2023 22:38

Disable (not hide) the pivot widget; remove Deep/Shallow select for now

a68263b

TrueDoctor reviewed Oct 12, 2023

View reviewed changes

Code review changes

37b8225

Fix pivot position with select tool

549d523

Fix incorrectly selected layers when shift clicking

b727536

0HyperCube force-pushed the select-tool-port-rebase branch from 41063eb to b727536 Compare October 17, 2023 17:03

Keavon requested a review from TrueDoctor October 17, 2023 17:55

Keavon approved these changes Oct 17, 2023

View reviewed changes

Keavon merged commit 8daf183 into full-document-graph Oct 17, 2023

Keavon deleted the select-tool-port-rebase branch October 17, 2023 17:59

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Migrate the Select tool to the document graph #1433

Migrate the Select tool to the document graph #1433

0HyperCube commented Oct 8, 2023 •

edited

Loading

Keavon commented Oct 8, 2023

github-actions bot commented Oct 8, 2023

Keavon commented Oct 8, 2023

github-actions bot commented Oct 8, 2023

Keavon commented Oct 9, 2023

github-actions bot commented Oct 9, 2023

Keavon commented Oct 9, 2023

github-actions bot commented Oct 9, 2023

Keavon commented Oct 9, 2023

github-actions bot commented Oct 9, 2023

0HyperCube commented Oct 11, 2023

github-actions bot commented Oct 11, 2023

Keavon commented Oct 11, 2023 •

edited

Loading

0HyperCube commented Oct 11, 2023

Keavon commented Oct 11, 2023

github-actions bot commented Oct 11, 2023

Keavon commented Oct 11, 2023 •

edited

Loading

Keavon commented Oct 12, 2023

Keavon commented Oct 12, 2023

github-actions bot commented Oct 12, 2023

TrueDoctor commented Oct 12, 2023

TrueDoctor left a comment

0HyperCube commented Oct 12, 2023 •

edited

Loading

Keavon commented Oct 16, 2023

github-actions bot commented Oct 16, 2023

Keavon commented Oct 17, 2023

github-actions bot commented Oct 17, 2023

Keavon commented Oct 17, 2023

github-actions bot commented Oct 17, 2023

Keavon left a comment

Migrate the Select tool to the document graph #1433

Migrate the Select tool to the document graph #1433

Conversation

0HyperCube commented Oct 8, 2023 • edited Loading

Keavon commented Oct 8, 2023

github-actions bot commented Oct 8, 2023

Keavon commented Oct 8, 2023

github-actions bot commented Oct 8, 2023

Keavon commented Oct 9, 2023

github-actions bot commented Oct 9, 2023

Keavon commented Oct 9, 2023

github-actions bot commented Oct 9, 2023

Keavon commented Oct 9, 2023

github-actions bot commented Oct 9, 2023

0HyperCube commented Oct 11, 2023

github-actions bot commented Oct 11, 2023

Keavon commented Oct 11, 2023 • edited Loading

0HyperCube commented Oct 11, 2023

Keavon commented Oct 11, 2023

github-actions bot commented Oct 11, 2023

Keavon commented Oct 11, 2023 • edited Loading

Keavon commented Oct 12, 2023

Keavon commented Oct 12, 2023

github-actions bot commented Oct 12, 2023

TrueDoctor commented Oct 12, 2023

TrueDoctor left a comment

Choose a reason for hiding this comment

0HyperCube commented Oct 12, 2023 • edited Loading

Keavon commented Oct 16, 2023

github-actions bot commented Oct 16, 2023

Keavon commented Oct 17, 2023

github-actions bot commented Oct 17, 2023

Keavon commented Oct 17, 2023

github-actions bot commented Oct 17, 2023

Keavon left a comment

Choose a reason for hiding this comment

0HyperCube commented Oct 8, 2023 •

edited

Loading

Keavon commented Oct 11, 2023 •

edited

Loading

Keavon commented Oct 11, 2023 •

edited

Loading

0HyperCube commented Oct 12, 2023 •

edited

Loading