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

* 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 GitHub
parent 337f828aa4
commit 37c6730c02
3 changed files with 67 additions and 39 deletions

Binary file not shown.

Before

Width:  |  Height:  |  Size: 34 KiB

After

Width:  |  Height:  |  Size: 34 KiB

View File

@ -31,15 +31,13 @@ test.describe('Text-to-CAD tests', () => {
) )
await expect(submittingToastMessage).toBeVisible() await expect(submittingToastMessage).toBeVisible()
await page.waitForTimeout(5000)
const generatingToastMessage = page.getByText( const generatingToastMessage = page.getByText(
`Generating parametric model...` `Generating parametric model...`
) )
await expect(generatingToastMessage).toBeVisible() await expect(generatingToastMessage).toBeVisible({ timeout: 10000 })
const successToastMessage = page.getByText(`Text-to-CAD successful`) 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() await expect(page.getByText('Copied')).not.toBeVisible()
@ -101,15 +99,13 @@ test.describe('Text-to-CAD tests', () => {
) )
await expect(submittingToastMessage).toBeVisible() await expect(submittingToastMessage).toBeVisible()
await page.waitForTimeout(5000)
const generatingToastMessage = page.getByText( const generatingToastMessage = page.getByText(
`Generating parametric model...` `Generating parametric model...`
) )
await expect(generatingToastMessage).toBeVisible() await expect(generatingToastMessage).toBeVisible({ timeout: 10000 })
const successToastMessage = page.getByText(`Text-to-CAD successful`) 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() await expect(page.getByText('Copied')).not.toBeVisible()
@ -121,13 +117,12 @@ test.describe('Text-to-CAD tests', () => {
// Find the toast. // Find the toast.
// Look out for the toast message // Look out for the toast message
await expect(submittingToastMessage).toBeVisible() await expect(submittingToastMessage).toBeVisible()
await expect(generatingToastMessage).toBeVisible({ timeout: 10000 })
await page.waitForTimeout(5000)
await expect(generatingToastMessage).toBeVisible()
// Expect 2 success toasts. // 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 2x4 lego')).toBeVisible()
await expect(page.getByText('a 2x6 lego')).toBeVisible() await expect(page.getByText('a 2x6 lego')).toBeVisible()
}) })
@ -150,15 +145,13 @@ test.describe('Text-to-CAD tests', () => {
) )
await expect(submittingToastMessage).toBeVisible() await expect(submittingToastMessage).toBeVisible()
await page.waitForTimeout(5000)
const generatingToastMessage = page.getByText( const generatingToastMessage = page.getByText(
`Generating parametric model...` `Generating parametric model...`
) )
await expect(generatingToastMessage).toBeVisible() await expect(generatingToastMessage).toBeVisible({ timeout: 10000 })
const successToastMessage = page.getByText(`Text-to-CAD successful`) const successToastMessage = page.getByText(`Text-to-CAD successful`)
await expect(successToastMessage).toBeVisible() await expect(successToastMessage).toBeVisible({ timeout: 15000 })
// Hit copy to clipboard. // Hit copy to clipboard.
const rejectButton = page.getByRole('button', { name: 'Reject' }) const rejectButton = page.getByRole('button', { name: 'Reject' })
@ -319,11 +312,9 @@ test.describe('Text-to-CAD tests', () => {
// Look out for the toast message // Look out for the toast message
await expect(submittingToastMessage).toBeVisible() await expect(submittingToastMessage).toBeVisible()
await page.waitForTimeout(5000) await expect(generatingToastMessage).toBeVisible({ timeout: 10000 })
await expect(generatingToastMessage).toBeVisible() await expect(successToastMessage).toBeVisible({ timeout: 15000 })
await expect(successToastMessage).toBeVisible()
}) })
test('sending a bad prompt fails, can ignore toast, can start over from command bar', async ({ test('sending a bad prompt fails, can ignore toast, can start over from command bar', async ({
@ -391,11 +382,9 @@ test.describe('Text-to-CAD tests', () => {
// Look out for the toast message // Look out for the toast message
await expect(submittingToastMessage).toBeVisible() await expect(submittingToastMessage).toBeVisible()
await page.waitForTimeout(5000) await expect(generatingToastMessage).toBeVisible({ timeout: 10000 })
await expect(generatingToastMessage).toBeVisible() await expect(successToastMessage).toBeVisible({ timeout: 15000 })
await expect(successToastMessage).toBeVisible()
await expect(page.getByText('Copied')).not.toBeVisible() await expect(page.getByText('Copied')).not.toBeVisible()
@ -448,16 +437,13 @@ test.describe('Text-to-CAD tests', () => {
) )
await expect(submittingToastMessage).toBeVisible() await expect(submittingToastMessage).toBeVisible()
await page.waitForTimeout(1000)
const generatingToastMessage = page.getByText( const generatingToastMessage = page.getByText(
`Generating parametric model...` `Generating parametric model...`
) )
await expect(generatingToastMessage).toBeVisible() await expect(generatingToastMessage).toBeVisible({ timeout: 10000 })
await page.waitForTimeout(5000)
const successToastMessage = page.getByText(`Text-to-CAD successful`) const successToastMessage = page.getByText(`Text-to-CAD successful`)
await expect(successToastMessage).toBeVisible() await expect(successToastMessage).toBeVisible({ timeout: 15000 })
await expect(page.getByText(promptWithNewline)).toBeVisible() await expect(page.getByText(promptWithNewline)).toBeVisible()
}) })

View File

@ -5,7 +5,7 @@ import { isDesktop } from 'lib/isDesktop'
import { PATHS } from 'lib/paths' import { PATHS } from 'lib/paths'
import toast from 'react-hot-toast' import toast from 'react-hot-toast'
import { TextToCad_type } from '@kittycad/lib/dist/types/src/models' import { TextToCad_type } from '@kittycad/lib/dist/types/src/models'
import { useEffect, useRef, useState } from 'react' import { useCallback, useEffect, useRef, useState } from 'react'
import { import {
Box3, Box3,
Color, Color,
@ -121,10 +121,40 @@ export function ToastTextToCadSuccess({
}) { }) {
const wrapperRef = useRef<HTMLDivElement | null>(null) const wrapperRef = useRef<HTMLDivElement | null>(null)
const canvasRef = useRef<HTMLCanvasElement | null>(null) const canvasRef = useRef<HTMLCanvasElement | null>(null)
const animationRequestRef = useRef<number>()
const [hasCopied, setHasCopied] = useState(false) const [hasCopied, setHasCopied] = useState(false)
const [showCopiedUi, setShowCopiedUi] = useState(false) const [showCopiedUi, setShowCopiedUi] = useState(false)
const modelId = data.id 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(() => { useEffect(() => {
if (!canvasRef.current) return if (!canvasRef.current) return
@ -132,7 +162,6 @@ export function ToastTextToCadSuccess({
const renderer = new WebGLRenderer({ canvas, antialias: true, alpha: true }) const renderer = new WebGLRenderer({ canvas, antialias: true, alpha: true })
renderer.setSize(CANVAS_SIZE, CANVAS_SIZE) renderer.setSize(CANVAS_SIZE, CANVAS_SIZE)
renderer.setPixelRatio(Math.min(window.devicePixelRatio, 2)) renderer.setPixelRatio(Math.min(window.devicePixelRatio, 2))
renderer.setAnimationLoop(animate)
const scene = new Scene() const scene = new Scene()
const ambientLight = new DirectionalLight(new Color('white'), 8.0) const ambientLight = new DirectionalLight(new Color('white'), 8.0)
@ -155,13 +184,6 @@ export function ToastTextToCadSuccess({
return return
} }
function animate() {
requestAnimationFrame(animate)
// required if controls.enableDamping or controls.autoRotate are set to true
controls.update()
renderer.render(scene, camera)
}
loader.parse( loader.parse(
buffer, buffer,
'', '',
@ -212,6 +234,8 @@ export function ToastTextToCadSuccess({
camera.updateProjectionMatrix() camera.updateProjectionMatrix()
controls.update() controls.update()
// render the scene once...
renderer.render(scene, camera)
}, },
// called when loading has errors // called when loading has errors
function (error) { function (error) {
@ -221,8 +245,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 () => { return () => {
renderer.dispose() renderer.dispose()
if (animationRequestRef.current) {
cancelAnimationFrame(animationRequestRef.current)
animationRequestRef.current = undefined
}
} }
}, []) }, [])