ByoungSooPark/modeling-app
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Fix CPU-driven churn once Text-to-CAD toast appears in the app (#3523)

Browse Source 
* Dispose of requestAnimationFrame loop when component unmounts

* Only run requestAnimationFrame loop when mouse is on canvas

* Better animation loop disposal on canvas mouseout

* A snapshot a day keeps the bugs away! 📷🐛 (OS: windows-latest)

* Text-to-cad test flakiness

* A snapshot a day keeps the bugs away! 📷🐛 (OS: windows-latest)

* Re-run CI

* Remove arbitrary timeout which may cause us to miss the toast on a fast-running test

* Remove a couple more arbitrary timeouts in text-to-cad tests

* Remove all the arbitrary 5s awaits from these tests

---------

Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
This commit is contained in:
Frank Noirot
2024-08-19 09:38:47 -04:00
committed by Jess Frazelle
parent d916c79874
commit b044f6faef
2 changed files with 67 additions and 39 deletions
Download Patch File Download Diff File Expand all files Collapse all files

46
e2e/playwright/text-to-cad-tests.spec.ts
View File

@ -29,15 +29,13 @@ test.describe('Text-to-CAD tests', () => {
)
await expect(submittingToastMessage).toBeVisible()
await page.waitForTimeout(5000)
const generatingToastMessage = page.getByText(
`Generating parametric model...`
)
await expect(generatingToastMessage).toBeVisible()
await expect(generatingToastMessage).toBeVisible({ timeout: 10000 })
const successToastMessage = page.getByText(`Text-to-CAD successful`)
await expect(successToastMessage).toBeVisible()
await expect(successToastMessage).toBeVisible({ timeout: 15000 })
await expect(page.getByText('Copied')).not.toBeVisible()
@ -96,15 +94,13 @@ test.describe('Text-to-CAD tests', () => {
)
await expect(submittingToastMessage).toBeVisible()
await page.waitForTimeout(5000)
const generatingToastMessage = page.getByText(
`Generating parametric model...`
)
await expect(generatingToastMessage).toBeVisible()
await expect(generatingToastMessage).toBeVisible({ timeout: 10000 })
const successToastMessage = page.getByText(`Text-to-CAD successful`)
await expect(successToastMessage).toBeVisible()
await expect(successToastMessage).toBeVisible({ timeout: 15000 })
await expect(page.getByText('Copied')).not.toBeVisible()
@ -116,13 +112,12 @@ test.describe('Text-to-CAD tests', () => {
// Find the toast.
// Look out for the toast message
await expect(submittingToastMessage).toBeVisible()
await page.waitForTimeout(5000)
await expect(generatingToastMessage).toBeVisible()
await expect(generatingToastMessage).toBeVisible({ timeout: 10000 })
// Expect 2 success toasts.
await expect(successToastMessage).toHaveCount(2)
await expect(successToastMessage).toHaveCount(2, {
timeout: 15000,
})
await expect(page.getByText('a 2x4 lego')).toBeVisible()
await expect(page.getByText('a 2x6 lego')).toBeVisible()
})
@ -145,15 +140,13 @@ test.describe('Text-to-CAD tests', () => {
)
await expect(submittingToastMessage).toBeVisible()
await page.waitForTimeout(5000)
const generatingToastMessage = page.getByText(
`Generating parametric model...`
)
await expect(generatingToastMessage).toBeVisible()
await expect(generatingToastMessage).toBeVisible({ timeout: 10000 })
const successToastMessage = page.getByText(`Text-to-CAD successful`)
await expect(successToastMessage).toBeVisible()
await expect(successToastMessage).toBeVisible({ timeout: 15000 })
// Hit copy to clipboard.
const rejectButton = page.getByRole('button', { name: 'Reject' })
@ -317,11 +310,9 @@ test.describe('Text-to-CAD tests', () => {
// Look out for the toast message
await expect(submittingToastMessage).toBeVisible()
await page.waitForTimeout(5000)
await expect(generatingToastMessage).toBeVisible({ timeout: 10000 })
await expect(generatingToastMessage).toBeVisible()
await expect(successToastMessage).toBeVisible()
await expect(successToastMessage).toBeVisible({ timeout: 15000 })
})
test('sending a bad prompt fails, can ignore toast, can start over from command bar', async ({
@ -390,11 +381,9 @@ test.describe('Text-to-CAD tests', () => {
// Look out for the toast message
await expect(submittingToastMessage).toBeVisible()
await page.waitForTimeout(5000)
await expect(generatingToastMessage).toBeVisible({ timeout: 10000 })
await expect(generatingToastMessage).toBeVisible()
await expect(successToastMessage).toBeVisible()
await expect(successToastMessage).toBeVisible({ timeout: 15000 })
await expect(page.getByText('Copied')).not.toBeVisible()
@ -447,16 +436,13 @@ test.describe('Text-to-CAD tests', () => {
)
await expect(submittingToastMessage).toBeVisible()
await page.waitForTimeout(1000)
const generatingToastMessage = page.getByText(
`Generating parametric model...`
)
await expect(generatingToastMessage).toBeVisible()
await page.waitForTimeout(5000)
await expect(generatingToastMessage).toBeVisible({ timeout: 10000 })
const successToastMessage = page.getByText(`Text-to-CAD successful`)
await expect(successToastMessage).toBeVisible()
await expect(successToastMessage).toBeVisible({ timeout: 15000 })
await expect(page.getByText(promptWithNewline)).toBeVisible()
})

60
src/components/ToastTextToCad.tsx
View File

@ -6,7 +6,7 @@ import { PATHS } from 'lib/paths'
import toast from 'react-hot-toast'
import { sep } from '@tauri-apps/api/path'
import { TextToCad_type } from '@kittycad/lib/dist/types/src/models'
import { useEffect, useRef, useState } from 'react'
import { useCallback, useEffect, useRef, useState } from 'react'
import {
Box3,
Color,
@ -122,10 +122,40 @@ export function ToastTextToCadSuccess({
}) {
const wrapperRef = useRef<HTMLDivElement | null>(null)
const canvasRef = useRef<HTMLCanvasElement | null>(null)
const animationRequestRef = useRef<number>()
const [hasCopied, setHasCopied] = useState(false)
const [showCopiedUi, setShowCopiedUi] = useState(false)
const modelId = data.id
const animate = useCallback(
({
renderer,
scene,
camera,
controls,
isFirstRender = false,
}: {
renderer: WebGLRenderer
scene: Scene
camera: OrthographicCamera
controls: OrbitControls
isFirstRender?: boolean
}) => {
if (
!wrapperRef.current ||
!(isFirstRender || animationRequestRef.current)
)
return
animationRequestRef.current = requestAnimationFrame(() =>
animate({ renderer, scene, camera, controls })
)
// required if controls.enableDamping or controls.autoRotate are set to true
controls.update()
renderer.render(scene, camera)
},
[]
)
useEffect(() => {
if (!canvasRef.current) return
@ -133,7 +163,6 @@ export function ToastTextToCadSuccess({
const renderer = new WebGLRenderer({ canvas, antialias: true, alpha: true })
renderer.setSize(CANVAS_SIZE, CANVAS_SIZE)
renderer.setPixelRatio(Math.min(window.devicePixelRatio, 2))
renderer.setAnimationLoop(animate)
const scene = new Scene()
const ambientLight = new DirectionalLight(new Color('white'), 8.0)
@ -156,13 +185,6 @@ export function ToastTextToCadSuccess({
return
}
function animate() {
requestAnimationFrame(animate)
// required if controls.enableDamping or controls.autoRotate are set to true
controls.update()
renderer.render(scene, camera)
}
loader.parse(
buffer,
'',
@ -213,6 +235,8 @@ export function ToastTextToCadSuccess({
camera.updateProjectionMatrix()
controls.update()
// render the scene once...
renderer.render(scene, camera)
},
// called when loading has errors
function (error) {
@ -222,8 +246,26 @@ export function ToastTextToCadSuccess({
}
)
// ...and set a mouseover listener on the canvas to enable the orbit controls
canvasRef.current.addEventListener('mouseover', () => {
renderer.setAnimationLoop(() =>
animate({ renderer, scene, camera, controls, isFirstRender: true })
)
})
canvasRef.current.addEventListener('mouseout', () => {
renderer.setAnimationLoop(null)
if (animationRequestRef.current) {
cancelAnimationFrame(animationRequestRef.current)
animationRequestRef.current = undefined
}
})
return () => {
renderer.dispose()
if (animationRequestRef.current) {
cancelAnimationFrame(animationRequestRef.current)
animationRequestRef.current = undefined
}
}
}, [])