KittyCAD / modeling-app

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

[BUG]: InternalEngine error during revolve that happens on one axis but not another #2540

Closed yeroca closed 2 months ago

yeroca commented 5 months ago

Describe the bug

If I have kcl that looks like this:

const part001 = startSketchOn('XY') |> startProfileAt([2, 0], %) |> line([10, 0], %) |> line([0, 2], %) |> line([-10, 0], %) |> close(%) |> revolve({ axis: 'Y' }, %) // default angle is 360

it produces a model as I would expect, but if I change the Y axis to Z in both places as follows:

const part001 = startSketchOn('XZ') |> startProfileAt([2, 0], %) |> line([10, 0], %) |> line([0, 2], %) |> line([-10, 0], %) |> close(%) |> revolve({ axis: 'Z' }, %) // default angle is 360

I get the following error: image

However, if I leave just the revolve axis set to Y, there's no error and I get the model I want, which doesn't seem right. Am I misunderstanding the axis parameter, or is revolve in fact behaving incorrectly?

Steps to Reproduce

Try out the two examples given in the description

Expected Behavior

I expected to get a model that's revolved about the Z axis.

Screenshots and Recordings

No response

Desktop OS

Win 10

Browser

N/A

Version

V0.21.9

Additional Context

No response

nadr0 commented 2 months ago

I was able to recreate this issue and I found several problems overall.

Bug 1

Specifically the API definition for revolve is wrong in documentation. It can only accept a local sketch axis of Y, -Y, X or -X. It cannot accept the value of Z.

Bug 2

This means I need to go through all of the possible permutations to see what will fail because of small numerical issues

Issue 1

Do we update the KCL documentation or code to easily let users identify if an axis needs to be global or local?

nadr0 commented 2 months ago

I'll be marking this specific ticket as completed since the revolve API does not take a Z axis value.

I'll make a PR to fix the documentation within the rust code. Also I made an engine issue to identify the other revolve issues.

However, if I leave just the revolve axis set to Y, there's no error and I get the model I want, which doesn't seem right. Am I misunderstanding the axis parameter, or is revolve in fact behaving incorrectly?

The axis argument you provided needs to be in the local sketch plan which is X or Y. Not the global axis.