feat: Edit individual points #146
Conversation
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughAdds an EditShape tool and point/handle editing flow across UI, TypeScript, and Zig logic; introduces PackedId and PathUtils modules, refactors path/shape data to point-based representation, updates picking/drawing pipelines and SDF prechecks, adjusts shader color logic, fixes a docs typo, and removes a trivial test. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant User
participant UI
participant Logic as logic.index.zig
participant GPU as renderPick/renderDraw
participant Shapes
participant Paths as path_utils/paths
User->>UI: choose "Edit Shape"
UI->>Logic: set Tool = EditShape
User->>Logic: onPointerDown(x,y)
Logic->>GPU: renderPick()
GPU-->>Logic: hovered_asset_id (possibly packed point id)
Logic->>Shapes: updateSelectedAsset(packed id)
Note over Shapes: selected_point_id set (point-level)
User->>Logic: onPointerMove(x,y)
Logic->>Shapes: transform pointer -> local space
Shapes->>Paths: PathUtils ops (getOppositeHandle, drawControlPoint)
Logic->>GPU: renderDraw() (skeleton + shape preview)
User->>Logic: onPointerUp()
Logic->>Shapes: clear selected_point_id
Logic->>GPU: final renderDraw()
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Pull Request Overview
This PR adds functionality to edit individual points in vector shapes. It introduces a new EditShape tool that allows users to select and manipulate control points and handles in path-based shapes.
Key changes:
- Adds a new EditShape tool to the creator interface
- Implements point selection system using packed IDs to encode shape/path/point indices
- Refactors path utilities into separate modules for better code organization
Reviewed Changes
Copilot reviewed 13 out of 15 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| src/logic/transform_ui.zig | Updates import from line.zig to lines.zig |
| src/logic/squares.zig | Adds getPickVertexData function for point selection |
| src/logic/shapes/shapes.zig | Major refactoring to support point editing and reorganizes utility functions |
| src/logic/shapes/paths.zig | Refactors path handling logic and moves utilities to separate modules |
| src/logic/shapes/path_utils.zig | New utility module for path-related functions |
| src/logic/shapes/packed_id.zig | New module for encoding/decoding point selection IDs |
| src/logic/shapes/bounding_box.zig | Updates import reference |
| src/logic/index.zig | Implements EditShape tool logic and point manipulation |
| src/index.ts | Adds EditShape enum value |
| src/test/index.test.ts | Removes test file content |
| src/WebGPU/programs/drawShape/shader.wgsl | Updates stroke rendering logic |
| integration-tests/template.html | Adds EditShape option to tool selector |
| ADRs/shape - bounding box padding | Fixes typo in documentation |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 11
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/logic/transform_ui.zig (1)
69-71: Guard division by zero in aspect computationIf bounds are degenerate on Y,
distance(bounds[0], bounds[3])may be 0, makingaspectinf/NaN during rotation.- const aspect = bounds[0].distance(bounds[1]) / bounds[0].distance(bounds[3]); + const num = bounds[0].distance(bounds[1]); + const denom = @max(bounds[0].distance(bounds[3]), 1.0); + const aspect = num / denom;src/logic/shapes/shapes.zig (1)
181-213: Memory leak: points from getAllPoints are never freed.getAllPoints returns owned memory; update_bounds must free it.
Apply:
const points = try self.getAllPoints( allocator, option_preview_point, active_path_index, ); + defer allocator.free(points);
🧹 Nitpick comments (16)
ADRs/shape - bounding box padding (2)
7-7: Clarify phrasing and fix remaining typos across the noteMultiple typos and grammar issues remain; also tighten the technical statements for readability.
Apply:
-IMPORTANT: -to calculate padding we use angles from current bounding point to neighbour points. +IMPORTANT: +To compute padding we use the angles from each bounding-box corner to its neighboring corners.
1-12: Proofread pass: fix typos and tighten terminologyRecommend a quick cleanup for accuracy and readability.
-Padding cannot be a part of bounding box, because then it's gonna be grown with scaling always. -No constant option will be easy to implement then. +Padding must not be included in the bounding box; otherwise it scales with the box. +Excluding padding makes a constant-pixel padding trivial to implement. -Additionaly it siplifies point normalazing, because points should be normalzied between bounding box - padding area. +Additionally, it simplifies point normalization, because points should be normalized within the bounding-box minus padding area. -IMPORTANT: -to calculate padding we use angles from current bounding point to neighbour points. -Because of that the box cannot have 0 widht or 0 height because neighbour angles has to be apart 90 degree. +IMPORTANT: +To compute padding we use the angles from each bounding-box corner to its neighboring corners. +Therefore the box cannot have zero width or height; adjacent edge directions must differ by 90° (degenerate boxes collapse these angles). -That's why bounds have to be at least 1x1, never less. -Additionaly it helps with matrix, scale of 0 is not reversable! Currently we andle it gracefully by returning identity matrix, but it should not happen in the first place for bounds. +Hence bounds must be at least 1×1, never less. +Additionally, this prevents zero-scale matrices, which are not invertible. We currently fall back to the identity matrix, but bounds should not reach that state.src/logic/transform_ui.zig (2)
191-207: Draw path calls: OK; consider small safety and micro-refactorCalls are correct; indices look safe given RENDER_TRIANGLE_INSTANCES. Consider:
- Add a debug assert before the loop to guard buffer bounds in dev builds.
- Hoist
outer_line_width / 2.0andthickness / 2.0into locals to avoid recomputation.
152-153: Comment/value mismatchComment says “0.01” but code uses 0.1. Align them to avoid confusion.
- .x = p1.x + @cos(angle + std.math.pi / 4.0) * 0.1, // 0.01 just to make it 45 degree - .y = p1.y + @sin(angle + std.math.pi / 4.0) * 0.1, + .x = p1.x + @cos(angle + std.math.pi / 4.0) * 0.1, // small offset to encode 45° + .y = p1.y + @sin(angle + std.math.pi / 4.0) * 0.1,src/WebGPU/programs/drawShape/shader.wgsl (1)
34-36: Threshold factorization: clarify intent and remove duplicationSemantics look intentional (g>1 → no half-stroke). For readability, factor the threshold into a named local and add a brief comment.
- let stroke_factor = select(0.5, 0.0, sdf.g > 1.0); - let is_filled = select(0.0, 1.0, sdf.r > -u.stroke_width * stroke_factor); - var color = select(u.stroke_color, u.fill_color, sdf.r > u.stroke_width * stroke_factor); + // If sdf.g > 1, treat as fill-only (no half-stroke threshold). + let stroke_factor = select(0.5, 0.0, sdf.g > 1.0); + let stroke_threshold = u.stroke_width * stroke_factor; + let is_filled = select(0.0, 1.0, sdf.r > -stroke_threshold); + var color = select(u.stroke_color, u.fill_color, sdf.r > stroke_threshold);Please confirm that
sdf.g > 1.0is the stable discriminator for “filled-only” glyphs in your MSDF encoding.src/logic/squares.zig (1)
23-37: New pick data path mirrors draw path — add radius clampingImplementation is correct. Add a local clamp so
radiusstays within half the rect’s min dimension and non-negative (avoids malformed rounded data).pub fn getPickVertexData(buffer: *[2]Triangles.PickInstance, x: f32, y: f32, width: f32, height: f32, radius: f32, id: u32) void { - const points = [_]Point{ + const points = [_]Point{ .{ .x = x, .y = y }, // .{ .x = x + width, .y = y }, // .{ .x = x + width, .y = y + height }, // .{ .x = x, .y = y + height }, // }; - const p0_v = Triangles.getRoundCornerVector(0, points, radius); - const p1_v = Triangles.getRoundCornerVector(1, points, radius); - const p2_v = Triangles.getRoundCornerVector(2, points, radius); - const p3_v = Triangles.getRoundCornerVector(3, points, radius); + const r = @max(0.0, @min(radius, 0.5 * @min(@abs(width), @abs(height)))); + const p0_v = Triangles.getRoundCornerVector(0, points, r); + const p1_v = Triangles.getRoundCornerVector(1, points, r); + const p2_v = Triangles.getRoundCornerVector(2, points, r); + const p3_v = Triangles.getRoundCornerVector(3, points, r);src/logic/shapes/bounding_box.zig (2)
137-144: Clarify/comment or remove debug note; maintain consistent handle check.The comment questions prior behavior and has typos. Either remove it or replace with a clear rationale explaining why straight-line handles collapse to endpoints for bounds.
Apply:
- // Do we need this? How did it worked before????? Wit treating stright handles as any point + // Treat straight-line handles as coincident with adjacent control points when computing bounds.
116-131: Guard against malformed curve arrays (not multiples of 4).Currently you silently drop trailing points. Assert or early-return to avoid hidden bugs.
Apply:
pub fn getBoundingBox(curves: []const Point) BoundingBox { if (curves.len == 0) { return BoundingBox{}; } + // Each cubic Bezier requires 4 points. + std.debug.assert(curves.len % 4 == 0);src/logic/shapes/packed_id.zig (1)
34-46: Type-correct masks in decode to u32 for strictness.Make intent explicit and avoid accidental type widening.
Apply:
- const point_index = encoded & MASK_ID1; - const path_index = (encoded >> BITS_ID1) & MASK_ID2; - const norm_shape_id = (encoded >> (BITS_ID1 + BITS_ID2)) & MASK_ID3; + const point_index: u32 = encoded & MASK_ID1; + const path_index: u32 = (encoded >> BITS_ID1) & MASK_ID2; + const norm_shape_id: u32 = (encoded >> (BITS_ID1 + BITS_ID2)) & MASK_ID3;src/logic/shapes/shapes.zig (1)
19-22: Threshold constant LGTM, but consider exposing via config.CREATE_HANDLE_THRESHOLD = 10.0 is reasonable; optionally promote to shared config for consistency with PathUtils.
src/logic/shapes/paths.zig (1)
151-189: Closed-path points writer: doc and intent.The duplication of control points to break segments is non-obvious; add a concise comment describing the output contract (e.g., segmentization for SDF).
Apply:
- pub fn getClosedPathPoints(self: Path, buffer: *std.ArrayList(Point), preview_point: ?Point) !void { + pub fn getClosedPathPoints(self: Path, buffer: *std.ArrayList(Point), preview_point: ?Point) !void { + // Writes a path ready for SDF processing: + // - duplicates control points to mark segment boundaries, + // - inserts opposite/straight handles when needed, + // - appends closing point.src/logic/shapes/path_utils.zig (2)
9-13: Consider documenting the magic number and its relationship.The
STRAIGHT_LINE_THRESHOLDvalue of 1e+10 andSTRAIGHT_LINE_HANDLEwith x-value of 1e+11 appear to be sentinel values for identifying straight line segments. The relationship between these values (1e+11 > 1e+10) should be documented for clarity.Add a comment explaining the sentinel value pattern:
const STRAIGHT_LINE_THRESHOLD = 1e+10; +// Sentinel value used to mark straight line handles. Any point with x > STRAIGHT_LINE_THRESHOLD +// is treated as a straight line handle. The x-value must be > STRAIGHT_LINE_THRESHOLD. pub const STRAIGHT_LINE_HANDLE = Point{ .x = 1e+11, .y = 0.0, };
56-58: Make scale factors configurable constants.The
SKELETON_POINT_SIZEandPICK_POINT_SCALEare hardcoded values that affect UI appearance and interaction. Consider making these configurable or at least document their purpose.+// Visual size of skeleton points in the UI (in pixels) const SKELETON_POINT_SIZE = 10.0; +// Multiplier applied to skeleton points for pick/selection detection +// Larger value makes points easier to select const PICK_POINT_SCALE = 2.0;src/logic/index.zig (3)
154-155: Address the TODO about ID overflow handling.The TODO comment indicates a potential issue when the ID counter reaches
PackedId.ASSET_ID_MAX. This could lead to ID collisions if not handled properly.The TODO mentions re-identifying all assets when the ID counter overflows. Would you like me to generate a solution that implements ID recycling or a re-identification mechanism to handle this case?
627-627: Remove or guard debug print statement.Debug print statement should be removed or placed behind a debug flag before merging to production.
- std.debug.print("New SDF size: {}x{}\n", .{ shape.sdf_size.w, shape.sdf_size.h }); + // Remove debug print or guard with if (std.builtin.mode == .Debug)
369-423: Extract nested EditShape logic into a helper function
The pointer-movement and Bézier-handle adjustment insrc/logic/index.zig(lines 369–423) is deeply nested and hard to follow. Pull it into a standalonefn updateEditedPoint( shape: *shapes.Shape, selected_point_id: PackedId.PointId, pointer: types.Point, ) void { … }helper to improve readability and isolate the bounds-and-closed-path handling.
+fn updateEditedPoint( + shape: *shapes.Shape, + selected_point_id: PackedId.PointId, + pointer: types.Point, +) void { + // move the existing nested logic from lines 369–423 here +} if (state.tool == Tool.EditShape) { if (shapes.selected_point_id) |selected_point_id| { if (getSelectedShape()) |shape| { const matrix = Matrix3x3.getMatrixFromRectangle(shape.bounds); const pointer = matrix.inverse().get(types.Point{ .x = x, .y = y }); - // …complex logic… + updateEditedPoint(shape, selected_point_id, pointer); } } return; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (13)
ADRs/shape - bounding box padding(1 hunks)integration-tests/template.html(1 hunks)src/WebGPU/programs/drawShape/shader.wgsl(1 hunks)src/__test__/index.test.ts(0 hunks)src/index.ts(1 hunks)src/logic/index.zig(16 hunks)src/logic/shapes/bounding_box.zig(2 hunks)src/logic/shapes/packed_id.zig(1 hunks)src/logic/shapes/path_utils.zig(1 hunks)src/logic/shapes/paths.zig(2 hunks)src/logic/shapes/shapes.zig(7 hunks)src/logic/squares.zig(1 hunks)src/logic/transform_ui.zig(3 hunks)
💤 Files with no reviewable changes (1)
- src/test/index.test.ts
🧰 Additional context used
🧬 Code graph analysis (2)
src/logic/squares.zig (2)
src/run.ts (3)
vertex_data(80-83)vertex_data(68-71)vertex_data(48-58)src/WebGPU/programs/pickTriangle/getProgram.ts (2)
pickTriangle(46-68)getProgram(6-69)
src/logic/shapes/path_utils.zig (1)
src/shapes/parsePathData.ts (1)
commandsToSegments(43-248)
🔇 Additional comments (18)
src/logic/transform_ui.zig (2)
219-227: Pick path calls: OKSignature and ID threading look correct for the pick pipeline. No issues.
3-3: Import rename verified
No remaining imports of"line.zig"found.src/index.ts (1)
52-53: Enum values align – no changes required. Zig’sTool.EditShapeis ordinal 2, matching TypeScript’sEditShape = 2and the HTML option value “2”.src/logic/shapes/bounding_box.zig (1)
3-3: PathUtils import LGTM.Switching to PathUtils keeps straight-handle logic centralized.
src/logic/shapes/shapes.zig (8)
27-33: Global selected_point_id addition acknowledged.State reset on resetState() is correct.
144-158: Handle preview logic is sound.Distance-based switch between real handle and straight handle is clean and uses world space.
196-203: Normalization correctly skips straight-line handles.Good use of PathUtils.isStraightLineHandle.
214-222: Avoid unnecessary inverse when straight handle sentinel is passed.Logic is correct and avoids precision loss for sentinel.
229-239: Clearing preview on close is correct.Prevents stale preview artifacts when a path closes.
261-265: Preview point draw uses PathUtils helper. LGTM.
271-284: Pick skeleton API addition looks good.Consistent with draw path; see next file for point-index cast fix.
305-311: Post-processing half straight-lines via PathUtils is appropriate.src/logic/shapes/paths.zig (4)
20-21: Sentinel handle on new path: good.Using PathUtils.STRAIGHT_LINE_HANDLE ensures consistency.
74-86: Prev/next handle helpers are correct and improve readability.
88-125: Skeleton draw generation via PathUtils is a good refactor.Centralizes drawing logic and hover handling.
190-205: updateLastHandle logic reads well and aligns with PathUtils.src/logic/index.zig (2)
74-74: Good: Fixed typo in constant name.The rename from
ASSET_ID_TRESHOLDtoASSET_ID_MINcorrects the spelling and better describes the constant's purpose.
743-752: Good: Proper resource management with arena allocator.The use of arena allocator for temporary memory in
renderPickensures proper cleanup and prevents memory leaks.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/shapes/createShape.ts (1)
32-32: Fix incorrect error message for rect validation.The error message says "Path without 'd' property" but should reference rect's width/height properties instead.
- throw Error("Path without 'd' property") + throw Error("Rect without 'width' or 'height' property")
♻️ Duplicate comments (8)
src/logic/shapes/shapes.zig (3)
186-186: Remove or guard debug print statement.Debug print statements should not be in production code.
- std.debug.print("bounding box: ({}, {}) - ({}, {})\n", .{ box.min_x, box.min_y, box.max_x, box.max_y }); + // std.debug.print("bounding box: ({}, {}) - ({}, {})\n", .{ box.min_x, box.min_y, box.max_x, box.max_y });
249-249: Fix type mismatch in hover_id path comparison.The comparison between
h.path(u32) andi(usize) causes a type mismatch.- const hover_id = if (input_hover_id) |h| if (h.path == i) h else null else null; + const hover_id = if (input_hover_id) |h| if (h.path == @as(u32, @intCast(i))) h else null else null;
324-331: Fix memory leak on early return.The
check_pointsarray is not freed when the function returns early due to insufficient points.const check_points = try self.getAllPoints( allocator, self.preview_point, active_path_index, ); + defer allocator.free(check_points); if (check_points.len < 4) { return null; }src/logic/index.zig (3)
670-671: Remove unused variable assignment.The variable
select_point_idis assigned but immediately marked as unused.- const select_point_id = shapes.selected_point_id; - _ = select_point_id; // autofix
281-287: Add bounds check for decoded shape IDs.After decoding the packed ID, verify that
point_id.shapeis within valid range before using it.if (id >= PackedId.MIN_PACKED_ID) { const point_id = PackedId.decode(id); + if (point_id.shape < ASSET_ID_MIN or point_id.shape > PackedId.MAX_ASSET_ID) { + // Invalid shape ID, ignore the selection + return; + } shapes.selected_point_id = point_id; state.selected_asset_id = point_id.shape; } else {
621-621: Remove or conditionally compile debug print statement.Debug print statement should be removed from production code.
- std.debug.print("New SDF size: {}x{}\n", .{ shape.sdf_size.w, shape.sdf_size.h }); + // std.debug.print("New SDF size: {}x{}\n", .{ shape.sdf_size.w, shape.sdf_size.h });src/logic/shapes/paths.zig (2)
49-50: Remove invalid.clone()call on pointer.
getLast()returns a*Pointpointer, not a value. Point structs don't have a.clone()method.- const last_cp = self.points.getLast().clone(); - const next_handle = PathUtils.getOppositeHandle(last_cp, prev_handle); + const last_cp = self.points.getLast().*; + const next_handle = PathUtils.getOppositeHandle(last_cp, prev_handle);
133-133: Cast point index to u32 when encoding pick ID.The loop index
i(usize) needs to be cast to u32 forPackedId.encode.- const id = PackedId.encode(shape_id, path_index, i); + const id = PackedId.encode(shape_id, path_index, @as(u32, @intCast(i)));
🧹 Nitpick comments (4)
src/shapes/parseRect.ts (3)
16-18: Right edge now relies on implicit start anchor (topRight). Confirm consumer contract or annotate.If the path format is “[H1, H2, endAnchor] with start anchor implied from previous segment,” this change is correct. If any consumer still expects an explicit start anchor here, this will break picking/segment iteration.
Optional: add an inline note for future readers.
- // Right edge: top to bottom - STRAIGHT_LINE_HANDLE, STRAIGHT_LINE_HANDLE, bottomRight, + // Right edge: top to bottom (start anchor is implied from previous segment: topRight) + STRAIGHT_LINE_HANDLE, STRAIGHT_LINE_HANDLE, bottomRight,
19-21: Bottom edge sequence looks consistent with the implicit-anchor pattern.Direction (right→left) and end anchor (bottomLeft) are clear. A brief comment can make the intent explicit and reduce regressions.
- // Bottom edge: right to left - STRAIGHT_LINE_HANDLE, STRAIGHT_LINE_HANDLE, bottomLeft, + // Bottom edge: right to left (end anchor: bottomLeft) + STRAIGHT_LINE_HANDLE, STRAIGHT_LINE_HANDLE, bottomLeft,
22-24: Left edge provides only handles for closure; ensure the pipeline closes to topLeft.This relies on the closing segment using these two handles and the initial anchor (topLeft). Verify all path utilities/editors assume this “handles-only for final segment” convention; otherwise, add the final anchor explicitly.
Optionally, document this inline and (non-prod) assert the invariant that closed paths have length 3×anchors.
- // Left edge: bottom to top - STRAIGHT_LINE_HANDLE, STRAIGHT_LINE_HANDLE, + // Left edge: bottom to top (closure: only handles here; end anchor is the first point: topLeft) + STRAIGHT_LINE_HANDLE, STRAIGHT_LINE_HANDLE,Outside this hunk, you can add a lightweight dev-time check after building
lines:// In dev, closed paths should have length == 3 * numberOfAnchors (here: 4 * 3 = 12). if (process.env.NODE_ENV !== 'production') { console.assert(lines.length % 3 === 0, 'parseRect: closed path must have length multiple of 3'); }If any consumer still requires an explicit closing anchor, the minimal alternative is to append
topLefthere and adjust downstream logic accordingly.src/logic/shapes/paths.zig (1)
159-162: Remove commented debug print statements.Clean up commented-out debug code that's not needed in production.
- // std.debug.print("BEFORE==========================\n", .{}); - // for (buffer.items) |p| { - // std.debug.print("{d}, {d}\n", .{ p.x, p.y }); - // } - if (!self.closed) { ... try buffer.append(points[0]); - // std.debug.print("AFTER==========================\n", .{}); - // for (buffer.items) |p| { - // std.debug.print("{d}, {d}\n", .{ p.x, p.y }); - // }Also applies to: 183-186
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (11)
src/WebGPU/programs/drawShape/shader.wgsl(1 hunks)src/logic/index.zig(16 hunks)src/logic/shapes/packed_id.zig(1 hunks)src/logic/shapes/paths.zig(3 hunks)src/logic/shapes/shapes.zig(9 hunks)src/logic/types.zig(0 hunks)src/shapes/createShape.ts(1 hunks)src/shapes/parseEllipse.ts(0 hunks)src/shapes/parsePathData.ts(5 hunks)src/shapes/parseRect.ts(1 hunks)src/shapes/types.ts(0 hunks)
💤 Files with no reviewable changes (3)
- src/logic/types.zig
- src/shapes/types.ts
- src/shapes/parseEllipse.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- src/WebGPU/programs/drawShape/shader.wgsl
- src/logic/shapes/packed_id.zig
🧰 Additional context used
🧬 Code graph analysis (4)
src/shapes/parseRect.ts (1)
src/shapes/const.ts (1)
STRAIGHT_LINE_HANDLE(1-1)
src/shapes/createShape.ts (2)
src/shapes/parsePathData.ts (1)
parsePathData(196-201)src/suspended/rendering vectors/index.ts (1)
testSvgToSegments(6-38)
src/logic/index.zig (1)
src/index.ts (2)
asset(246-291)assets(243-300)
src/shapes/parsePathData.ts (2)
src/shapes/const.ts (1)
STRAIGHT_LINE_HANDLE(1-1)src/suspended/rendering vectors/index.ts (1)
testSvgToSegments(6-38)
🔇 Additional comments (3)
src/shapes/parsePathData.ts (2)
116-118: LGTM! Proper implementation of straight line segments.The code correctly pushes straight line handles for both control points when creating line segments, which aligns with the new point-based representation.
141-143: LGTM! Correct Bezier curve point representation.The code properly stores control handles and endpoints for cubic Bezier curves in the expected point triple format.
src/logic/shapes/shapes.zig (1)
139-156: LGTM! Well-implemented handle threshold logic.The implementation correctly determines whether to create smooth handles or straight line handles based on distance, using CREATE_HANDLE_THRESHOLD as the decision point. This provides an intuitive user experience for path editing.
| const matrix = Matrix3x3.getMatrixFromRectangle(self.bounds); | ||
|
|
||
| for (self.paths.items, 0..) |path, i| { | ||
| const path_skeleton = try path.getSkeletonPickVertexData(matrix, allocator, self.id, @as(u32, i)); |
There was a problem hiding this comment.
Fix missing @intcast for path index.
The path index i needs to be cast to u32 when passing to getSkeletonPickVertexData.
- const path_skeleton = try path.getSkeletonPickVertexData(matrix, allocator, self.id, @as(u32, i));
+ const path_skeleton = try path.getSkeletonPickVertexData(matrix, allocator, self.id, @as(u32, @intCast(i)));Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In src/logic/shapes/shapes.zig around line 277, the call to
path.getSkeletonPickVertexData passes the loop/index variable i without an
explicit integer cast; change the argument to cast i to u32 using Zig's @intCast
(e.g., @intCast(u32, i)) so the call matches the function's expected u32
parameter and avoids type-mismatch/runtime issues.
|
🎉 This PR is included in version 0.1.0-next.22 🎉 The release is available on: Your semantic-release bot 📦🚀 |
Summary by CodeRabbit
New Features
Graphics
Documentation
Tests