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

Positionable: Common interface for canvas items #256

Merged
merged 8 commits into from
Nov 3, 2024
Merged

Conversation

webfiltered
Copy link
Collaborator

@webfiltered webfiltered commented Nov 2, 2024

Allows user-positionable canvas objects (groups, nodes, future-types) to share common features such as selection, arranging, and movement. Enables a more intuitive and natural UX.

Nodes

  • Node size is now a getter/setter backed by a Rect; array ref can no longer be overwritten (in line with pos and groups)
  • Node is_selected is now a legacy accessor for Positionable.selected
  • Adds renderArea - calculated once per frame, replacing repeat calls to getBounding (and onBounding callback)

Groups

  • Adds id to groups

  • Removes unused margin checks from get_X_OnPos functions
  • Some test failures are expected; there are minor differences in node resize
  • The arbitrary +1 width added to nodes during render is still present on this branch

Refactor out duplicated code from Node
Node bounds once per render
Cached results
- Removes margin param from getNodeOnPos
- Removes margin param from getGroupOnPos
- Hitboxes now uniform for render / mouse features
- Simplifies code
@webfiltered
Copy link
Collaborator Author

Requires Comfy-Org/ComfyUI_frontend#1405

@webfiltered
Copy link
Collaborator Author

Requires Comfy-Org/ComfyUI_frontend#1408 🐬

@webfiltered webfiltered marked this pull request as ready for review November 3, 2024 00:36
@huchenlei
Copy link
Member

LGTM! This is such a huge leap forward on code structure!

@huchenlei huchenlei merged commit e661dec into master Nov 3, 2024
4 checks passed
@huchenlei huchenlei deleted the positionable branch November 3, 2024 16:12
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