KittyCAD / modeling-app

The KittyCAD modeling app.
https://kittycad.io/modeling-app/download
MIT License
314 stars 24 forks source link

Selection order should not matter for constraints #2776

Open Irev-Dev opened 3 weeks ago

Irev-Dev commented 3 weeks ago

Here's a video of how to replicate the bug for the equal-length constraint, but it, likely applies to a few other's, so part of the issue is checking the other constraints.

https://github.com/KittyCAD/modeling-app/assets/29681384/c8e71d92-7c74-4151-a014-8dc08ba25a00

Tasks:

Pointers: equal length code is pretty much all in src/components/Toolbar/EqualLength.tsx so that's where I would start. It might be as simple as reordering selectionRanges.codeBasedSelections by source ranges, but I'm not sure.

jtran commented 3 weeks ago

I'm having trouble reproducing this in my development environment. I started by pasting the same initial kcl into the editor, and then I edited the sketch visually in the same way. But the resulting kcl looks like this, which is what we expect.

const sketch001 = startSketchOn('XZ')
  |> startProfileAt([0.76, 0.58], %)
  |> line([0.35, 0.83], %, $seg02)
  |> angledLine([35, segLen(seg02, %)], %)
  |> line([2.12, 0.11], %, $seg01)
  |> angledLine([-65, segLen(seg01, %)], %)

Was there some other action done that was cut from the video that might have led to an invalid state?

I also tried a few variations like sketching the initial lines, changing whether I exited sketch mode, and changing which segments I constrained.

jtran commented 3 weeks ago

We determined that the problem only arises in Chromium-based browsers. Safari, Tauri app on macOS, and Firefox all work as expected.

Irev-Dev commented 3 weeks ago

Make sure there are tests, a unit test might suffice since it's likely an AST mod bug.

We might chat about the best way to test this after we figure out what the bug actually is, because my initial hunch of AST mod bug seems less likely with the "not on webkit" revelation.

jtran commented 3 weeks ago

After investigating it some, I realized that I could only reproduce the problem when DevTools was closed. So this is likely a race condition and not browser-specific.