Merge pull request #744 from Tresjs/refactor/loop-ready

fix(loop/useTresReady): add setReady

src/composables/useTresContextProvider/index.ts

@@ -212,9 +212,11 @@ export function useTresContextProvider({
 
   const { on: onTresReady, cancel: cancelTresReady } = useTresReady(ctx)!
 
+  ctx.loop.setReady(false)
+  ctx.loop.start()
   onTresReady(() => {
     emit('ready', ctx)
-    ctx.loop.start()
+    ctx.loop.setReady(true)
   })
 
   onUnmounted(() => {

src/core/loop.test.ts

@@ -1,11 +1,12 @@
 import { afterEach, beforeEach, it } from 'vitest'
+import type { TresContext } from '../composables/useTresContextProvider'
 import { createRenderLoop } from './loop'
 
 let renderLoop
 
 describe('createRenderLoop', () => {
   beforeEach(() => {
-    renderLoop = createRenderLoop()
+    renderLoop = createRenderLoop({} as TresContext)
   })
   afterEach(() => {
     renderLoop.stop()
@@ -117,4 +118,157 @@ describe('createRenderLoop', () => {
 
     expect(executionOrder).toEqual(['before', 'fbo', 'render', 'after'])
   })
+
+  describe('`stop`, `start`, `pause`, `resume` call order', () => {
+    it('does not trigger a callback on `start()` unless `stop()`ped', () => {
+      const callbackBefore = vi.fn()
+      const callbackRender = vi.fn()
+      const callbackAfter = vi.fn()
+      renderLoop.register(callbackBefore, 'before')
+      renderLoop.register(callbackRender, 'render')
+      renderLoop.register(callbackAfter, 'after')
+      renderLoop.start()
+      expect(callbackBefore).toBeCalledTimes(1)
+      expect(callbackRender).toBeCalledTimes(1)
+      expect(callbackAfter).toBeCalledTimes(1)
+
+      renderLoop.start()
+      renderLoop.start()
+      renderLoop.start()
+      renderLoop.start()
+      expect(callbackBefore).toBeCalledTimes(1)
+      expect(callbackRender).toBeCalledTimes(1)
+      expect(callbackAfter).toBeCalledTimes(1)
+
+      renderLoop.stop()
+      renderLoop.start()
+      expect(callbackBefore).toBeCalledTimes(2)
+      expect(callbackRender).toBeCalledTimes(2)
+      expect(callbackAfter).toBeCalledTimes(2)
+    })
+
+    it('can `start()` even if `resume()`d while `stop()`ped', () => {
+      const callbackBefore = vi.fn()
+      const callbackRender = vi.fn()
+      const callbackAfter = vi.fn()
+      renderLoop.register(callbackBefore, 'before')
+      renderLoop.register(callbackRender, 'render')
+      renderLoop.register(callbackAfter, 'after')
+      renderLoop.stop()
+      renderLoop.resume()
+      expect(callbackBefore).toBeCalledTimes(0)
+      expect(callbackRender).toBeCalledTimes(0)
+      expect(callbackAfter).toBeCalledTimes(0)
+
+      renderLoop.start()
+      expect(callbackBefore).toBeCalledTimes(1)
+      expect(callbackRender).toBeCalledTimes(1)
+      expect(callbackAfter).toBeCalledTimes(1)
+    })
+
+    it('`isActive.value` is `true` only if both `start()`ed and `resume()`d, regardless of call order', () => {
+      const callbackBefore = vi.fn()
+      const callbackRender = vi.fn()
+      const callbackAfter = vi.fn()
+      renderLoop.register(callbackBefore, 'before')
+      renderLoop.register(callbackRender, 'render')
+      renderLoop.register(callbackAfter, 'after')
+
+      const { start, stop, resume, pause } = renderLoop
+
+      // NOTE: stop, pause | stop, resume | start, resume
+      // NOTE: stop, pause
+      stop()
+      pause()
+      expect(renderLoop.isActive.value).toBe(false)
+      // NOTE: stop, resume
+      resume()
+      expect(renderLoop.isActive.value).toBe(false)
+      // NOTE: start, resume
+      start()
+      expect(renderLoop.isActive.value).toBe(true)
+
+      // NOTE: stop, pause | start, pause | start, resume
+      // NOTE: stop, pause
+      stop()
+      pause()
+      expect(renderLoop.isActive.value).toBe(false)
+      // NOTE: start, pause
+      start()
+      expect(renderLoop.isActive.value).toBe(false)
+      // NOTE: start, resume
+      resume()
+      expect(renderLoop.isActive.value).toBe(true)
+
+      // NOTE: start, resume | start, pause | start, resume
+      // NOTE: start, resume
+      resume()
+      start()
+      expect(renderLoop.isActive.value).toBe(true)
+      // NOTE: start, pause
+      pause()
+      expect(renderLoop.isActive.value).toBe(false)
+      // NOTE: start, resume
+      resume()
+      expect(renderLoop.isActive.value).toBe(true)
+
+      // NOTE: start, resume | stop, resume | start, resume
+      // NOTE: start, resume
+      resume()
+      start()
+      expect(renderLoop.isActive.value).toBe(true)
+      // NOTE: stop, resume
+      stop()
+      expect(renderLoop.isActive.value).toBe(false)
+      // NOTE: start, resume
+      start()
+      expect(renderLoop.isActive.value).toBe(true)
+
+      // NOTE: make some random calls
+      const ons = [start, resume]
+      const offs = [stop, pause]
+      const onsAndOffs = [start, stop, resume, pause]
+      const TEST_COUNT = 100
+
+      for (let i = 0; i < TEST_COUNT; i++) {
+        const ARRAY_COUNT = 25 + Math.floor(Math.random() * 10)
+        const _offs = Array.from({ length: ARRAY_COUNT }).fill(0).map(() => choose(offs))
+        _offs.forEach(fn => fn())
+        expect(renderLoop.isActive.value).toBe(false)
+        shuffle(ons)
+        ons.forEach(fn => fn())
+        expect(renderLoop.isActive.value).toBe(true)
+      }
+
+      for (let i = 0; i < TEST_COUNT; i++) {
+        const ARRAY_COUNT = 25 + Math.floor(Math.random() * 10)
+        const _onsAndOffs = Array.from({ length: ARRAY_COUNT }).fill(0).map(() => choose(onsAndOffs))
+        _onsAndOffs.forEach(fn => fn())
+        shuffle(offs)
+        offs[0]()
+        expect(renderLoop.isActive.value).toBe(false)
+        shuffle(ons)
+        ons.forEach(fn => fn())
+        expect(renderLoop.isActive.value).toBe(true)
+      }
+    })
+  })
 })
+
+function choose(array: any[]) {
+  const i = Math.floor(Math.random() * array.length)
+  return array[i]
+}
+
+function shuffle(array: any[]) {
+  let currentIndex = array.length
+  while (currentIndex !== 0) {
+    const randomIndex = Math.floor(Math.random() * currentIndex)
+    currentIndex--;
+    [array[currentIndex], array[randomIndex]] = [
+      array[randomIndex],
+      array[currentIndex],
+    ]
+  }
+  return array
+};

src/core/loop.ts

@@ -40,11 +40,15 @@ export interface RendererLoop {
   isActive: Ref<boolean>
   isRenderPaused: Ref<boolean>
   setContext: (newContext: Record<string, any>) => void
+  setReady: (isReady: boolean) => void
 }
 
 export function createRenderLoop(): RendererLoop {
+  let isReady = true
+  let isStopped = true
+  let isPaused = false
   const clock = new Clock(false)
-  const isActive = ref(false)
+  const isActive = ref(clock.running)
   const isRenderPaused = ref(false)
   let animationFrameId: number
   const loopId = MathUtils.generateUUID()
@@ -53,6 +57,8 @@ export function createRenderLoop(): RendererLoop {
   const subscriberRender = createPriorityEventHook<LoopCallbackWithCtx>()
   const subscribersAfter = createPriorityEventHook<LoopCallbackWithCtx>()
 
+  _syncState()
+
   // Context to be passed to callbacks
   let context: Record<string, any> = {}
 
@@ -76,29 +82,31 @@ export function createRenderLoop(): RendererLoop {
   }
 
   function start() {
-    if (!isActive.value) {
-      clock.start()
-      isActive.value = true
-      loop()
-    }
+    // NOTE: `loop()` produces side effects on each call.
+    // Those side effects are only desired if `isStopped` goes
+    // from `true` to `false` below.  So while we don't need
+    // a guard in `stop`, `resume`, and `pause`, we do need
+    // a guard here.
+    if (!isStopped) { return }
+    isStopped = false
+    _syncState()
+    loop()
   }
 
   function stop() {
-    if (isActive.value) {
-      clock.stop()
-      cancelAnimationFrame(animationFrameId)
-      isActive.value = false
-    }
+    isStopped = true
+    _syncState()
+    cancelAnimationFrame(animationFrameId)
   }
 
-  function pause() {
-    clock.stop()
-    isActive.value = false
+  function resume() {
+    isPaused = false
+    _syncState()
   }
 
-  function resume() {
-    clock.start()
-    isActive.value = true
+  function pause() {
+    isPaused = true
+    _syncState()
   }
 
   function pauseRender() {
@@ -110,6 +118,10 @@ export function createRenderLoop(): RendererLoop {
   }
 
   function loop() {
+    if (!isReady) {
+      animationFrameId = requestAnimationFrame(loop)
+      return
+    }
     const delta = clock.getDelta()
     const elapsed = clock.getElapsedTime()
     const snapshotCtx = {
@@ -145,6 +157,19 @@ export function createRenderLoop(): RendererLoop {
     animationFrameId = requestAnimationFrame(loop)
   }
 
+  function _syncState() {
+    const shouldClockBeRunning = !isStopped && !isPaused
+    if (clock.running !== shouldClockBeRunning) {
+      if (!clock.running) {
+        clock.start()
+      }
+      else {
+        clock.stop()
+      }
+    }
+    isActive.value = clock.running
+  }
+
   return {
     loopId,
     register: (callback: LoopCallbackFn, stage: 'before' | 'render' | 'after', index) => registerCallback(callback, stage, index),
@@ -157,5 +182,6 @@ export function createRenderLoop(): RendererLoop {
     isRenderPaused,
     isActive,
     setContext,
+    setReady: (b: boolean) => isReady = b,
   }
 }