Sketch on chamfer UI #3918
Sketch on chamfer UI #3918
Conversation
|
QA Wolf here! As you write new code it's important that your test coverage is keeping up. |
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
| @@ -436,10 +436,10 @@ export function getArtifactsToUpdate({ | |||
| response.data.modeling_response.type === 'solid3d_get_opposite_edge' && | |||
| response.data.modeling_response.data.edge) || | |||
| // or is adjacent edge | |||
| (cmd.type === 'solid3d_get_prev_adjacent_edge' && | |||
| (cmd.type === 'solid3d_get_next_adjacent_edge' && | |||
There was a problem hiding this comment.
I believe this is related to https://github.com/KittyCAD/engine/issues/2467 and the fix for it engine side, writing the tests for sketch on chamfer I a bug and this is the fix (along with a rust change)
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #3918 +/- ##
=======================================
Coverage 87.20% 87.20%
=======================================
Files 70 70
Lines 25255 25255
=======================================
Hits 22023 22023
Misses 3232 3232
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
| expressionName: string | ||
| ) { | ||
| expressionName: string, | ||
| edgeCutMeta: EdgeCutInfo | null |
There was a problem hiding this comment.
you've probably explained it already, sorry if asking again, but what is edge cut meta ?
There was a problem hiding this comment.
It's some metadata that needed to know how to break up the chamfer call expressions
i.e.
chamfer({
length: 18,
tags: [getNextAdjacentEdge(seg01), seg02]
}, %)needs to be broken into two chamfer calls for each of the tags in the array, so that a tag declorator can be added to one of them, but some extra information is needed to know which one needs the tag declorator.
There was a problem hiding this comment.
Most the changes in this file are work-arounds that will ultimately be resolved by #3836
But bumping int this again is really motivating me to do the above ☝️
I left a todo comment referencing that issue too.
* initial break up * rename main fixture file * add more expect state pattern
There was a problem hiding this comment.
Sorry I'm losing steam part way through, everything is looking good but I ran into a question. Is it expected to not be covering the case where a chamfer is separately defined, not as a pipe operation from an extrude call? I can't sketch on the chamfer with the following code, for example:
const sketch001 = startSketchOn('XZ')
|> startProfileAt([-169.52, 219.21], %)
|> angledLine([0, 369.25], %, $rectangleSegmentA001)
|> angledLine([
segAng(rectangleSegmentA001) - 90,
390.68
], %, $rectangleSegmentB001)
|> angledLine([
segAng(rectangleSegmentA001),
-segLen(rectangleSegmentA001)
], %, $rectangleSegmentC001)
|> lineTo([profileStartX(%), profileStartY(%)], %)
|> close(%)
const extrude001 = extrude(200, sketch001)
const chamfer001 = chamfer({
length: 100,
tags: [
getPreviousAdjacentEdge(rectangleSegmentA001)
]
}, extrude001)| { steps: 5 } | ||
| ) | ||
|
|
||
| await scene.moveCameraTo(cameraPos, cameraTarget) |
| await rectangle2ndMove({ | ||
| pixelDiff: 50, | ||
| }) |
There was a problem hiding this comment.
I integrated an option pixelDiff into the move and click helpers, i.e do the click and wait until there's a pixel diff.
|
Made the chamfer work when it's not in a pipeExpression, and added a unit and e2e test for that case. Thanks @franknoirot |
|
https://github.com/KittyCAD/modeling-app/actions/runs/11046307526/job/30690674459?pr=3918 failed, but I'm certain this is related to the export changes we've been seeing of late. |
Screenshare.-.2024-09-23.3_42_37.PM.mp4
Related to https://github.com/KittyCAD/engine/issues/2651