Skip to content

Replace the Artboard struct with a Table<Table<Graphic>> shape#4077

Merged
Keavon merged 3 commits intomasterfrom
table-artboards
Apr 29, 2026
Merged

Replace the Artboard struct with a Table<Table<Graphic>> shape#4077
Keavon merged 3 commits intomasterfrom
table-artboards

Conversation

@Keavon
Copy link
Copy Markdown
Member

@Keavon Keavon commented Apr 29, 2026

Partly closes #3779.

Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request refactors the artboard system by replacing the Artboard struct with a Table<Table> structure, storing metadata like location and dimensions as row attributes. It also transitions artboard coordinates from IVec2 to DVec2 across the editor and node graph. A bug was identified in the create_artboard node where incorrect normalization logic for negative dimensions prevents proper artboard creation when dragging from opposite corners.

Comment thread node-graph/nodes/graphic/src/artboard.rs Outdated
Copy link
Copy Markdown
Contributor

@cubic-dev-ai cubic-dev-ai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

2 issues found across 27 files

Confidence score: 3/5

  • There is a concrete regression risk in node-graph/nodes/graphic/src/artboard.rs: the current normalization/clamping order appears to break negative-dimension handling, so opposite-corner drags may produce incorrect artboard geometry.
  • editor/src/messages/portfolio/document/graph_operation/utility_types.rs introduces a semantics risk by accepting DVec2 for artboard edits, which can allow fractional origin/size values and create inconsistent behavior across tools expecting integer-aligned coordinates.
  • Given the medium severity (5–6/10) and high confidence on both findings, this looks like some merge risk rather than a hard blocker if these paths are addressed promptly.
  • Pay close attention to node-graph/nodes/graphic/src/artboard.rs and editor/src/messages/portfolio/document/graph_operation/utility_types.rs - negative-dimension normalization and integer-vs-fractional coordinate semantics are the main correctness concerns.
Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="editor/src/messages/portfolio/document/graph_operation/utility_types.rs">

<violation number="1" location="editor/src/messages/portfolio/document/graph_operation/utility_types.rs:587">
P2: Keep artboard origin and size constrained to integer coordinates instead of accepting `DVec2` in editing APIs; allowing fractional values here can introduce inconsistent artboard positioning/sizing semantics across tools.

(Based on your team's feedback about using integer coordinates for artboard origin and size.) [FEEDBACK_USED]</violation>
</file>

<file name="node-graph/nodes/graphic/src/artboard.rs">

<violation number="1" location="node-graph/nodes/graphic/src/artboard.rs:51">
P2: The normalization logic does not actually support negative dimensions: clamping before `min()` makes `normalized_location` effectively always equal to `location`, so opposite-corner drags are not represented correctly.

(Based on your team's feedback about aligning behavior with comments and intended TODO behavior.) [FEEDBACK_USED]</violation>
</file>

Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.

Comment thread node-graph/nodes/graphic/src/artboard.rs Outdated
@Keavon Keavon merged commit 471a5c6 into master Apr 29, 2026
10 of 11 checks passed
@Keavon Keavon deleted the table-artboards branch April 29, 2026 05:58
Keavon added a commit that referenced this pull request Apr 29, 2026
* Replace the Artboard struct with a Table<Table<Graphic>> shape

* Remove the never-functional, seemingly unneeded migrate_type_descriptor_names due to typo

* Allow negative artboard sizes
oluseyi pushed a commit to oluseyi/Graphite that referenced this pull request Apr 29, 2026
…iteEditor#4077)

* Replace the Artboard struct with a Table<Table<Graphic>> shape

* Remove the never-functional, seemingly unneeded migrate_type_descriptor_names due to typo

* Allow negative artboard sizes
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.

Data trees

1 participant