Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Gizmo Normal Snapping #2539

Merged
merged 41 commits into from
Jun 5, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
41 commits
Select commit Hold shift + click to select a range
da56ced
gizmo 2.0
max-mrgrsk May 26, 2024
2781261
A snapshot a day keeps the bugs away! 📷🐛 (OS: ubuntu)
github-actions[bot] May 27, 2024
956adc1
initial mouse position fix
max-mrgrsk May 27, 2024
609866e
animation loop / disposal optimization
max-mrgrsk May 27, 2024
f0a506d
A snapshot a day keeps the bugs away! 📷🐛 (OS: ubuntu)
github-actions[bot] May 27, 2024
5edbb04
reset camera tweak
Irev-Dev May 27, 2024
a47d1c1
Merge branch 'main' into 2445-gizmo-snapping
Irev-Dev May 27, 2024
6093f61
add cam target to debug panel
Irev-Dev May 28, 2024
fc37031
test stub
Irev-Dev May 28, 2024
f7be198
reset camera position handle removed from gizmo
max-mrgrsk May 28, 2024
1a98418
gizmo refactoring
max-mrgrsk May 28, 2024
0e224e9
small fix
max-mrgrsk May 28, 2024
73ddfa6
reset camera view
max-mrgrsk May 29, 2024
1d6af39
nicer updateCameraToAxis
max-mrgrsk May 29, 2024
1d01595
micro refactoring
max-mrgrsk May 29, 2024
eaf5a8d
playwright update
max-mrgrsk May 29, 2024
9348b90
playwright remove timeout + fmt
max-mrgrsk May 30, 2024
c18f820
hide gizmo while loading stream
max-mrgrsk May 30, 2024
e575bf0
Revert "A snapshot a day keeps the bugs away! 📷🐛 (OS: ubuntu)"
Irev-Dev May 31, 2024
78eb92c
Revert "A snapshot a day keeps the bugs away! 📷🐛 (OS: ubuntu)"
Irev-Dev May 31, 2024
d73ba82
Merge remote-tracking branch 'origin' into 2445-gizmo-snapping
Irev-Dev May 31, 2024
2e08856
try make gizmo test more realiable
Irev-Dev May 31, 2024
bc3cef1
tweak
Irev-Dev May 31, 2024
4c2ba6a
refactoring
max-mrgrsk May 31, 2024
fb30528
increase timeout time
max-mrgrsk May 31, 2024
125b217
1 sec wait after mouse click
max-mrgrsk May 31, 2024
30c4482
3 sec timeout
max-mrgrsk May 31, 2024
96940bc
Merge branch 'main' into 2445-gizmo-snapping
max-mrgrsk May 31, 2024
24cd4e2
better clickPosition
max-mrgrsk Jun 1, 2024
6e99d14
test with 10 sec timeout
max-mrgrsk Jun 1, 2024
ce1db5d
0.5 sec timeout
max-mrgrsk Jun 1, 2024
3c37e31
add passive update for gizmo to avoid some edge cases
max-mrgrsk Jun 1, 2024
bea2a06
default_camera_get_settings after click
max-mrgrsk Jun 2, 2024
e8d3d10
Merge branch 'main' into 2445-gizmo-snapping
max-mrgrsk Jun 3, 2024
a3313e2
try and remove timeouts
Irev-Dev Jun 5, 2024
079fcf9
Merge remote-tracking branch 'origin' into 2445-gizmo-snapping
Irev-Dev Jun 5, 2024
aafadb0
fix unrelated test
Irev-Dev Jun 5, 2024
2e19a05
Merge remote-tracking branch 'origin' into 2445-gizmo-snapping
Irev-Dev Jun 5, 2024
e22785d
test tweak
Irev-Dev Jun 5, 2024
c5f6e14
Merge remote-tracking branch 'origin/main' into 2445-gizmo-snapping
Irev-Dev Jun 5, 2024
8222fb5
put waits back in
Irev-Dev Jun 5, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
196 changes: 172 additions & 24 deletions e2e/playwright/flow-tests.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ test('Basic sketch', async ({ page }) => {
// select a plane
await page.mouse.click(700, 200)

await expect(page.locator('.cm-content')).toHaveText(
await expect(u.codeLocator).toHaveText(
`const sketch001 = startSketchOn('XZ')`
)
await u.closeDebugPanel()
Expand All @@ -102,29 +102,25 @@ test('Basic sketch', async ({ page }) => {

const startXPx = 600
await page.mouse.click(startXPx + PUR * 10, 500 - PUR * 10)
await expect(page.locator('.cm-content'))
.toHaveText(`const sketch001 = startSketchOn('XZ')
await expect(u.codeLocator).toHaveText(`const sketch001 = startSketchOn('XZ')
|> startProfileAt(${commonPoints.startAt}, %)`)
await page.waitForTimeout(100)

await page.mouse.click(startXPx + PUR * 20, 500 - PUR * 10)
await page.waitForTimeout(100)

await expect(page.locator('.cm-content'))
.toHaveText(`const sketch001 = startSketchOn('XZ')
await expect(u.codeLocator).toHaveText(`const sketch001 = startSketchOn('XZ')
|> startProfileAt(${commonPoints.startAt}, %)
|> line([${commonPoints.num1}, 0], %)`)

await page.mouse.click(startXPx + PUR * 20, 500 - PUR * 20)
await expect(page.locator('.cm-content'))
.toHaveText(`const sketch001 = startSketchOn('XZ')
await expect(u.codeLocator).toHaveText(`const sketch001 = startSketchOn('XZ')
|> startProfileAt(${commonPoints.startAt}, %)
|> line([${commonPoints.num1}, 0], %)
|> line([0, ${commonPoints.num1}], %)`)
await page.waitForTimeout(100)
await page.mouse.click(startXPx, 500 - PUR * 20)
await expect(page.locator('.cm-content'))
.toHaveText(`const sketch001 = startSketchOn('XZ')
await expect(u.codeLocator).toHaveText(`const sketch001 = startSketchOn('XZ')
|> startProfileAt(${commonPoints.startAt}, %)
|> line([${commonPoints.num1}, 0], %)
|> line([0, ${commonPoints.num1}], %)
Expand Down Expand Up @@ -154,8 +150,7 @@ test('Basic sketch', async ({ page }) => {
await page.getByRole('button', { name: 'Constrain' }).click()
await page.getByRole('button', { name: 'Equal Length' }).click()

await expect(page.locator('.cm-content'))
.toHaveText(`const sketch001 = startSketchOn('XZ')
await expect(u.codeLocator).toHaveText(`const sketch001 = startSketchOn('XZ')
|> startProfileAt(${commonPoints.startAt}, %)
|> line([${commonPoints.num1}, 0], %, 'seg01')
|> line([0, ${commonPoints.num1}], %)
Expand Down Expand Up @@ -1533,7 +1528,7 @@ test('Can add multiple sketches', async ({ page }) => {
await u.openDebugPanel()

const center = { x: viewportSize.width / 2, y: viewportSize.height / 2 }
const { toSU, click00r, expectCodeToBe } = getMovementUtils({ center, page })
const { toSU, click00r } = getMovementUtils({ center, page })

await expect(
page.getByRole('button', { name: 'Start Sketch' })
Expand All @@ -1550,25 +1545,25 @@ test('Can add multiple sketches', async ({ page }) => {
let codeStr = "const sketch001 = startSketchOn('XY')"

await page.mouse.click(center.x, viewportSize.height * 0.55)
await expectCodeToBe(codeStr)
await expect(u.codeLocator).toHaveText(codeStr)
await u.closeDebugPanel()
await page.waitForTimeout(500) // TODO detect animation ending, or disable animation

await click00r(0, 0)
codeStr += ` |> startProfileAt(${toSU([0, 0])}, %)`
await expectCodeToBe(codeStr)
await expect(u.codeLocator).toHaveText(codeStr)

await click00r(50, 0)
codeStr += ` |> line(${toSU([50, 0])}, %)`
await expectCodeToBe(codeStr)
await expect(u.codeLocator).toHaveText(codeStr)

await click00r(0, 50)
codeStr += ` |> line(${toSU([0, 50])}, %)`
await expectCodeToBe(codeStr)
await expect(u.codeLocator).toHaveText(codeStr)

await click00r(-50, 0)
codeStr += ` |> line(${toSU([-50, 0])}, %)`
await expectCodeToBe(codeStr)
await expect(u.codeLocator).toHaveText(codeStr)

// exit the sketch, reset relative clicker
click00r(undefined, undefined)
Expand All @@ -1586,24 +1581,24 @@ test('Can add multiple sketches', async ({ page }) => {
await page.mouse.click(center.x + 30, center.y)
await page.waitForTimeout(500) // TODO detect animation ending, or disable animation
codeStr += "const sketch002 = startSketchOn('XY')"
await expectCodeToBe(codeStr)
await expect(u.codeLocator).toHaveText(codeStr)
await u.closeDebugPanel()

await click00r(30, 0)
codeStr += ` |> startProfileAt(${toSU([30, 0])}, %)`
await expectCodeToBe(codeStr)
await expect(u.codeLocator).toHaveText(codeStr)

await click00r(30, 0)
codeStr += ` |> line(${toSU([30 - 0.1 /* imprecision */, 0])}, %)`
await expectCodeToBe(codeStr)
codeStr += ` |> line(${toSU([30 + 0.1 /* imprecision */, 0])}, %)`
await expect(u.codeLocator).toHaveText(codeStr)

await click00r(0, 30)
codeStr += ` |> line(${toSU([0, 30])}, %)`
await expectCodeToBe(codeStr)
await expect(u.codeLocator).toHaveText(codeStr)

await click00r(-30, 0)
codeStr += ` |> line(${toSU([-30 + 0.1, 0])}, %)`
await expectCodeToBe(codeStr)
codeStr += ` |> line(${toSU([-30 - 0.1, 0])}, %)`
await expect(u.codeLocator).toHaveText(codeStr)

click00r(undefined, undefined)
await u.openAndClearDebugPanel()
Expand Down Expand Up @@ -4781,6 +4776,159 @@ test('Engine disconnect & reconnect in sketch mode', async ({ page }) => {
).not.toBeVisible()
})

test.describe('Testing Gizmo', () => {
const cases = [
{
testDescription: 'top view',
clickPosition: { x: 951, y: 385 },
expectedCameraPosition: { x: 800, y: -152, z: 4886.02 },
expectedCameraTarget: { x: 800, y: -152, z: 26 },
},
{
testDescription: 'bottom view',
clickPosition: { x: 951, y: 429 },
expectedCameraPosition: { x: 800, y: -152, z: -4834.02 },
expectedCameraTarget: { x: 800, y: -152, z: 26 },
},
{
testDescription: '+x view',
clickPosition: { x: 929, y: 417 },
expectedCameraPosition: { x: 5660.02, y: -152, z: 26 },
expectedCameraTarget: { x: 800, y: -152, z: 26 },
},
{
testDescription: '-x view',
clickPosition: { x: 974, y: 397 },
expectedCameraPosition: { x: -4060.02, y: -152, z: 26 },
expectedCameraTarget: { x: 800, y: -152, z: 26 },
},
{
testDescription: '+y view',
clickPosition: { x: 967, y: 421 },
expectedCameraPosition: { x: 800, y: 4708.02, z: 26 },
expectedCameraTarget: { x: 800, y: -152, z: 26 },
},
{
testDescription: '-y view',
clickPosition: { x: 935, y: 393 },
expectedCameraPosition: { x: 800, y: -5012.02, z: 26 },
expectedCameraTarget: { x: 800, y: -152, z: 26 },
},
] as const
for (const {
clickPosition,
expectedCameraPosition,
expectedCameraTarget,
testDescription,
} of cases) {
test(`check ${testDescription}`, async ({ page }) => {
const u = await getUtils(page)
await page.addInitScript(async (KCL_DEFAULT_LENGTH) => {
localStorage.setItem(
'persistCode',
`const part001 = startSketchOn('XZ')
|> startProfileAt([20, 0], %)
|> line([7.13, 4 + 0], %)
|> angledLine({ angle: 3 + 0, length: 3.14 + 0 }, %)
|> lineTo([20.14 + 0, -0.14 + 0], %)
|> xLineTo(29 + 0, %)
|> yLine(-3.14 + 0, %, 'a')
|> xLine(1.63, %)
|> angledLineOfXLength({ angle: 3 + 0, length: 3.14 }, %)
|> angledLineOfYLength({ angle: 30, length: 3 + 0 }, %)
|> angledLineToX({ angle: 22.14 + 0, to: 12 }, %)
|> angledLineToY({ angle: 30, to: 11.14 }, %)
|> angledLineThatIntersects({
angle: 3.14,
intersectTag: 'a',
offset: 0
}, %)
|> tangentialArcTo([13.14 + 0, 13.14], %)
|> close(%)
|> extrude(5 + 7, %)
`
)
}, KCL_DEFAULT_LENGTH)
await page.setViewportSize({ width: 1000, height: 500 })
await page.goto('/')
await u.waitForAuthSkipAppStart()
await page.waitForTimeout(100)
// wait for execution done
await u.openDebugPanel()
await u.expectCmdLog('[data-message-type="execution-done"]')
await u.sendCustomCmd({
type: 'modeling_cmd_req',
cmd_id: uuidv4(),
cmd: {
type: 'default_camera_look_at',
vantage: {
x: 3000,
y: 3000,
z: 3000,
},
center: {
x: 800,
y: -152,
z: 26,
},
up: { x: 0, y: 0, z: 1 },
},
})
await page.waitForTimeout(100)
await u.clearCommandLogs()
await u.sendCustomCmd({
type: 'modeling_cmd_req',
cmd_id: uuidv4(),
cmd: {
type: 'default_camera_get_settings',
},
})
await u.waitForCmdReceive('default_camera_get_settings')

await page.waitForTimeout(400)
await page.mouse.move(clickPosition.x, clickPosition.y)
await page.waitForTimeout(100)
await u.clearCommandLogs()
await page.mouse.click(clickPosition.x, clickPosition.y)
await u.waitForCmdReceive('default_camera_look_at')
await u.clearCommandLogs()

await u.sendCustomCmd({
type: 'modeling_cmd_req',
cmd_id: uuidv4(),
cmd: {
type: 'default_camera_get_settings',
},
})
Comment on lines +4896 to +4902
Copy link
Collaborator

@Irev-Dev Irev-Dev Jun 5, 2024

Choose a reason for hiding this comment

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

Making a not of this for as issue #2603

await u.waitForCmdReceive('default_camera_get_settings')
await page.waitForTimeout(400)

await Promise.all([
// position
expect(page.getByTestId('cam-x-position')).toHaveValue(
expectedCameraPosition.x.toString()
),
expect(page.getByTestId('cam-y-position')).toHaveValue(
expectedCameraPosition.y.toString()
),
expect(page.getByTestId('cam-z-position')).toHaveValue(
expectedCameraPosition.z.toString()
),
// target
expect(page.getByTestId('cam-x-target')).toHaveValue(
expectedCameraTarget.x.toString()
),
expect(page.getByTestId('cam-y-target')).toHaveValue(
expectedCameraTarget.y.toString()
),
expect(page.getByTestId('cam-z-target')).toHaveValue(
expectedCameraTarget.z.toString()
),
])
Comment on lines +4906 to +4927
Copy link
Collaborator

Choose a reason for hiding this comment

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

So this is definitely a bit uglier than the solution you originally had, but running an expect for each value, where what we give expect is a playwright locator i.e. page.getByTestId('cam-x-position') you get the benefit of playwright trying the locator multiple times, means it accounts for the browser being a bit slower some times and all manner of race conditions.

so I think this will make it more reliable, but I'm not sure because CI is going to be the ultimate judge of that, if it doesn't end up being useful than we can revert it.

})
}
})

test('Successful export shows a success toast', async ({ page }) => {
// FYI this test doesn't work with only engine running locally
// And you will need to have the KittyCAD CLI installed
Expand Down
8 changes: 2 additions & 6 deletions e2e/playwright/test-utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -162,12 +162,7 @@ export const getMovementUtils = (opts: any) => {
return ret.then(() => [last.x, last.y])
}

const expectCodeToBe = async (str: string) => {
await expect(opts.page.locator('.cm-content')).toHaveText(str)
await opts.page.waitForTimeout(100)
}

return { toSU, click00r, expectCodeToBe }
return { toSU, click00r }
}

export async function getUtils(page: Page) {
Expand Down Expand Up @@ -228,6 +223,7 @@ export async function getUtils(page: Page) {
.locator(locator)
.boundingBox()
.then((box) => ({ ...box, x: box?.x || 0, y: box?.y || 0 })),
codeLocator: page.locator('.cm-content'),
doAndWaitForCmd: async (
fn: () => Promise<void>,
commandType: string,
Expand Down
Loading
Loading