Skip to content

Conversation

@andrewvarga
Copy link
Contributor

Fixes #8837

@andrewvarga andrewvarga requested a review from a team as a code owner November 10, 2025 21:06
@vercel
Copy link

vercel bot commented Nov 10, 2025

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Preview Comments Updated (UTC)
modeling-app Ready Ready Preview Comment Nov 11, 2025 8:58am

Copy link
Contributor

@max-mrgrsk max-mrgrsk left a comment

Choose a reason for hiding this comment

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

I think that this fix is good and if you wish we can be approved, but it will not close the issue, because I have got the same bug on the 8th try right now. I think the problem is related to the grid itself. The grid is slightly shifted.

kcl:

sketch001 = startSketchOn(XY)
profile008 = startProfile(sketch001, at = [2, 5.5])
  |> line(endAbsolute = [0, 6.25])
  |> yLine(length = 1.25)
  |> xLine(length = 2)
  |> yLine(length = -2)
  |> line(end = [0.5, 0.25])
snap.not.snap.mp4

@andrewvarga
Copy link
Contributor Author

I think that this fix is good and if you wish we can be approved, but it will not close the issue, because I have got the same bug on the 8th try right now. I think the problem is related to the grid itself. The grid is slightly shifted.

kcl:

sketch001 = startSketchOn(XY)
profile008 = startProfile(sketch001, at = [2, 5.5])
  |> line(endAbsolute = [0, 6.25])
  |> yLine(length = 1.25)
  |> xLine(length = 2)
  |> yLine(length = -2)
  |> line(end = [0.5, 0.25])

snap.not.snap.mp4

@max-mrgrsk yes absolutely, this is still in Draft and doesn't fix

I think that this fix is good and if you wish we can be approved, but it will not close the issue, because I have got the same bug on the 8th try right now. I think the problem is related to the grid itself. The grid is slightly shifted.

kcl:

sketch001 = startSketchOn(XY)
profile008 = startProfile(sketch001, at = [2, 5.5])
  |> line(endAbsolute = [0, 6.25])
  |> yLine(length = 1.25)
  |> xLine(length = 2)
  |> yLine(length = -2)
  |> line(end = [0.5, 0.25])

snap.not.snap.mp4

@max-mrgrsk thanks for testing this, you're correct, this is definitely not working yet, and the reason I put it to Draft is that I realized it may need a bigger overhaul.

The problems I know of:

  • We could clean up the order of preference for the various snapping methods: snap to tangent (when drawing a line from a tangential arc), snap to main axis lines, snap to grid, snap to profileStart.
    One reason this PR doesn't fix your original issue yet is that snap to profileStart doesn't override snap to grid completely: it is possible that profileStart is not snapped yet (because raycastRing() radius is not close enough), but snap to grid still snaps the line to the same point that the profileStart is located in. I wanted to apply profileStart after snap to grid but in the current code raycastRing() runs before that and it's a bigger refactor do change that.
  • You're right that snapping to main axis lines doesn't work well with snap to grid also, there should be a similar fix for this.
  • Unrelated to snapping logic, if you zoom in too much (0.01 mm) the 2 digit precision we use in our generated KCL code is not enough to represent the correct snap to grid values, this can happen especially if you have "Infinite grid" enabled and the result is that the line points are not matching the grid anymore. I think probably we need to generate more than 2 digits of precision for numerical values in this case to make sure snap to grid works as expected. This c
  • There may be a rendering issue with the grid too which results in incorrect positions, this needs to be verified after the previous issue is fixed

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.

Sketch doesn’t close with grid snap ON

3 participants