brianzinn / react-babylonjs

React for Babylon 3D engine
https://brianzinn.github.io/react-babylonjs/
818 stars 105 forks source link

Issue getting scene.render to work properly. #181

Closed bscott711 closed 2 years ago

bscott711 commented 2 years ago

I'm using the following snippet in this project to control the renderLoop so it only renders on demand. It works well in the playground and in non-react version i had been working on. But it doesn't seem to be working properly in this scenario as the GPU is pegged at 30% rather than 0 when the scene is not being moved. Any help on how I am using it incorrectly is appreciated!

`` var renderLoop = function () { camera.update(); if (cameraChanged) { scene.executeWhenReady(() => cameraChanged = true); cameraChanged = !cameraChanged; scene.render(true, true); } }; engine.runRenderLoop(renderLoop); // Camera position camera.onViewMatrixChangedObservable.add(() => { cameraChanged = true; });

brianzinn commented 2 years ago

I think it's because cameraChanged is being brought in by closure and is always true. Try changing it to:

import { useRef } from 'react';
const cameraChanged = useRef(true);

...

if (cameraChange.current) {
 // render scene
 cameraChanged.current = false;
}

camera.onViewMatrixChangedObservable.add(() => {
   cameraChanged.current = true;
});
bscott711 commented 2 years ago

I appear to be breaking some hook rules if I do that (no surprise there :joy: ) since the function I am using is both a nested function and simple js.

brianzinn commented 2 years ago

Also the way you are using the project you are better off with https://github.com/brianzinn/babylonjs-hook It doesn't have the renderer, but you are doing everything imperatively.

brianzinn commented 2 years ago

I would add ability to pass in own render loop to this library as it would be useful for any viewers or other libraries that want to save resources when nothing is happening in the scene - like a viewer. Things like animations, etc. would not be visible and that is OK for many scenarios. Let me know if you are interested and we can work on a solution.

dennemark commented 2 years ago

For reference: https://forum.babylonjs.com/t/what-is-the-correct-way-to-render-on-demand/17256/28

Time for a new storybook entry? This component can be placed into scene and allows optional hardware scaling. Thanks for the hint on using useRef with cameraChanged! I think I tried to implement this once, but did not fully succeed. Or my issue was, that this component only listens to camera changed observable, however, I also would need to trigger rerender on other things, like:

import {useEffect, useState, useRef} from 'react'
import {useScene} from "react-babylonjs"
import { Camera } from '@babylonjs/core/Cameras'

const RenderOnCameraChange = ({hardwareScaling}:{hardwareScaling?: number}) => {
    const scene = useScene()
    const cameraChanged = useRef("render");
    const hardwareScalingFallback = useRef(1);

    const [camera, setCamera] = useState<Camera | undefined>(undefined);

    useEffect(()=>{
        if(scene){
            scene.activeCamera && setCamera(scene.activeCamera)
            const updateCamera = scene.onActiveCameraChanged.add(() => {
                scene.activeCamera && setCamera(scene.activeCamera)})
            return ()=>{
                scene.onActiveCameraChanged.remove(updateCamera)
            }
        }
    },[scene])

    useEffect(()=>{
        if(scene && camera){
            const viewChanged = camera.onViewMatrixChangedObservable.add(()=>{
                cameraChanged.current = "render";
            })
            return () => {
                camera.onViewMatrixChangedObservable.remove(viewChanged);
            }
        }
    },[scene, camera])

    useEffect(()=>{

        if(scene && camera){
            const engine = scene.getEngine();

            hardwareScalingFallback.current = engine.getHardwareScalingLevel()
            //RENDER LOOP
            var renderLoop = function () {
                camera.update();
                if (cameraChanged.current === "render") {
                    // render scene - optionally with hardware scaling
                    if(hardwareScaling){
                        hardwareScaling && engine.setHardwareScalingLevel(hardwareScaling);
                        cameraChanged.current = "reset";
                    }else{
                        cameraChanged.current = "norender"
                    }
                    scene.render(true, true)
                }else if(cameraChanged.current === "reset"){
                    // render scene with old hardwareScaling value
                    hardwareScaling && engine.setHardwareScalingLevel(hardwareScalingFallback.current);
                    scene.render(true, true)
                    cameraChanged.current = "norender"
                }
            };
            engine.stopRenderLoop();
            engine.runRenderLoop(renderLoop);
            return () => {
                engine.stopRenderLoop();
                engine.runRenderLoop(()=>{scene.render()});
            }
        }
    },[scene, camera])

    return <></>;
} 

export default RenderOnCameraChange;
brianzinn commented 2 years ago

I think loading of model (and meshes) can be handled with: https://doc.babylonjs.com/typedoc/classes/babylon.scene#onnewmeshaddedobservable

I suspect for lights moving would need to listen first: https://doc.babylonjs.com/typedoc/classes/babylon.scene#onnewlightaddedobservable

I didn't see from API easy way to get lights themselves can be listened in on - even without changing position, direction, intensity, changing shadow maps, etc.

What you have is enough for most uses cases as the camera matrix changing also catches intertial movements on the camera, etc. In 4.0 I want to break out the additional utilities like the Html that you added as well as Model, Skybox and something like this would be really useful as part of library where community can participate as opposed to a recipe. First step looks like we need better access to render loop or maybe it is enough to have access to a render callback. What you have there is a great start. I'm wondering if having access to override this part of code would suffice: https://github.com/brianzinn/react-babylonjs/blob/master/src/Engine.tsx#L71

dennemark commented 2 years ago

Those observables are useful! But I think I once tried to use the newMeshAddedObservable and sometimes it seems to trigger too late, maybe because within the mesh textures etc. need to be loaded first.

Concerning the overwrite, I guess it limits a few options. I.e. one cannot observe changing of active camera outside the Engine component. This would probably need to be handled within the render functoin. Not sure if this is so convenient. But for other use cases it might be enough.

In 4.0 I want to break out the additional utilities like the Html that you added as well as Model, Skybox and something like this would be really useful as part of library where community can participate as opposed to a recipe.

👍

brianzinn commented 2 years ago

@bscott711 - were you able to find a resolution for this? if an extension point can be added for better control of render loop that is the way to go. I think it would be nice to be able to opt-out of rendering also when the canvas scrolls out of view. It's not a stretch to have an opt-in mechanism that would combine IntersectionObserver (recently added ResizeObserver to this library) with listening to some additional events to reduce rendering as noted above. Perhaps even some hook like a useEffect with a callback to restore.

bscott711 commented 2 years ago

Hey @brianzinn thanks for the follow up. I had to punt for now with where it's at, like most things I do it's probably 85% there :facepalm: thanks for those suggestions, when time allows I will get back into it and try to implement something like that.

brianzinn commented 2 years ago

I'm working on a new Engine FC that will handle better rendering logic and will look at passing in a custom method that will override what is current happening (like a template method). I still need to compare with what @dennemark has above to really look at which way is easier to extend and more intuitive. This code is not tested - just quickly converted to FC as one way to go:

import { Engine } from '@babylonjs/core/Engines/engine.js'
import { EngineOptions, ThinEngine } from '@babylonjs/core/Engines/thinEngine.js'
import { Observable } from '@babylonjs/core/Misc/observable.js'
import { Nullable } from '@babylonjs/core/types.js'
import React, { useRef, useEffect, ReactNode, MutableRefObject } from 'react'
import { EngineCanvasContext, EngineCanvasContextType } from './hooks/engine'

export type RenderOptions = {
  /**
   * Observes visibility and does not render scene when no pixels of canvas are visible
   * Defaults to false, so you need to opt-in
   */
  whenVisibleOnly?: boolean
}

const useCanvasObserver = (canvasRef: MutableRefObject<Nullable<HTMLCanvasElement>>, shouldRenderRef: MutableRefObject<boolean>, threshold: number = 0) => {
  const callbackFn: IntersectionObserverCallback = (entries) => {
    const [entry] = entries;
    shouldRenderRef.current = entry.isIntersecting;
    console.log('should render updating:', shouldRenderRef.current);
  }

  useEffect(() => {
    if (canvasRef.current === null) {
      return;
    }
    const observer = new IntersectionObserver(callbackFn, { threshold });
    observer.observe(canvasRef.current);

    return () => {
      if (canvasRef.current) {
        observer.unobserve(canvasRef.current);
      }
    }

  }, [canvasRef, threshold]);
}

export type EngineProps = {
  engineCanvasContext?: EngineCanvasContextType
  shadersRepository?: string
  engineOptions?: EngineOptions
  antialias?: boolean
  enableOfflineSupport?: boolean
  adaptToDeviceRatio?: boolean
  renderOptions?: RenderOptions

  /**
   * Attach resize event when canvas resizes (window resize may not occur).
   * Defaults to true, so you need to opt-out.
   */
  observeCanvasResize?: boolean

  /**
   * By default touch-action: 'none' will be on the canvas.  Use this to disable.
   */
  touchActionNone?: boolean
  /**
   * Useful if you want to attach CSS to the canvas by css #id selector.
   */
  canvasId?: string
  canvasStyle?: any
  width?: number
  height?: number

  // onCreated?: (engine: Engine) => void
} & { children?: ReactNode | undefined } & React.CanvasHTMLAttributes<HTMLCanvasElement>

const ReactBabylonjsEngine: React.FC<EngineProps> = (props: EngineProps, context?: any) => {
  const engine = useRef<Nullable<Engine>>(null);
  const resizeObserver = useRef<Nullable<ResizeObserver>>(null);

  const onBeforeRenderLoopObservable = useRef<Observable<Engine>>(new Observable<Engine>());
  const onEndRenderLoopObservable = useRef<Observable<Engine>>(new Observable<Engine>());

  const canvasRef = useRef<Nullable<HTMLCanvasElement>>(null);
  const shouldRenderRef = useRef(true);

  const renderOptions: RenderOptions = props.renderOptions ?? {};

  useEffect(() => {
    if (canvasRef.current === null) {
      return;
    }

    engine.current = new Engine(
      canvasRef.current,
      props.antialias === true ? true : false, // default false
      props.engineOptions,
      props.adaptToDeviceRatio === true ? true : false // default false
    )

    // TODO: this prop should be in a dependency and moved out of useEffect.
    if (renderOptions.whenVisibleOnly === true) {
      // NOTE: the shouldRender usage needs to be updated when more render logic is added (ie: camera project matrix change observable, etc.)
      useCanvasObserver(canvasRef, shouldRenderRef, 0)
    }

    engine.current.runRenderLoop(() => {
      if (shouldRenderRef.current === false) {
        return;
      }
      if (onBeforeRenderLoopObservable.current.hasObservers()) {
        onBeforeRenderLoopObservable.current.notifyObservers(engine.current!)
      }

      // TODO: here is where you could access your own render method
      engine.current!.scenes.forEach((scene) => {
        scene.render()
      })

      if (onEndRenderLoopObservable.current.hasObservers()) {
        onEndRenderLoopObservable.current.notifyObservers(engine.current!)
      }
    })

    if (props.observeCanvasResize !== false && window?.ResizeObserver) {
      resizeObserver.current = new ResizeObserver(() => {
        engine.current!.resize()
      })
      resizeObserver.current.observe(canvasRef.current);
    }

    engine.current.onContextLostObservable.add((eventData: ThinEngine) => {
      console.warn('context loss observable from Engine: ', eventData)
    })

    const onResizeWindow = () => {
      if (engine.current) {
        engine.current.resize()
      }
    }

    window.addEventListener('resize', onResizeWindow);

    return () => {
      window.removeEventListener('resize', onResizeWindow);

      if (resizeObserver.current !== null) {
        resizeObserver.current.disconnect()
        resizeObserver.current = null
      }

      if (engine.current !== null) {
        engine.current!.dispose()
        engine.current = null
      }
    }

  }, [canvasRef.current]);

  let { touchActionNone, width, height, canvasStyle, canvasId, ...rest } = props

  let opts: any = {}

  if (touchActionNone !== false) {
    opts['touch-action'] = 'none'
  }

  if (width !== undefined && height !== undefined) {
    opts.width = width
    opts.height = height
  }

  if (canvasId) {
    opts.id = canvasId
  }

  if (canvasStyle) {
    opts.style = canvasStyle
  }

  // TODO: this.props.portalCanvas does not need to render a canvas.
  return (
    <EngineCanvasContext.Provider value={{ engine: engine.current, canvas: canvasRef.current }}>
      <canvas {...opts} ref={canvasRef}>
        {engine.current !== null && props.children}
      </canvas>
    </EngineCanvasContext.Provider>
  )
}

export default ReactBabylonjsEngine
brianzinn commented 2 years ago

This is all on the main branch, but not out on NPM. I will implement with my new documentation to make sure it's working as expected, but there is an opt-in mechanism to skip render when the canvas is not visible (using intersection observer).

I can add a way to not render at all as an opt-in and then calling scene.render(...) can be the responsibility of the caller.

voronp commented 2 years ago

can we add prop like isPaused to Engine and stop render if it is true?

brianzinn commented 2 years ago

@voronp - that is a definite possibility. There was a shouldRenderRef added when the IntersectionObserver was added. I think it makes sense to tie into that logic, but I am unsure how they would work together if that is the case. ie: "pause" rendering then it seems would take precedence over Canvas visibility always. Here is the relevant code: https://github.com/brianzinn/react-babylonjs/blob/master/packages/react-babylonjs/src/Engine.tsx#L122

Were you wanting to propose a PR?

voronp commented 2 years ago

@brianzinn ok, so created pr with demo in storybooks https://github.com/brianzinn/react-babylonjs/pull/223

brianzinn commented 2 years ago

closing from inactivity - there is the fix from voronp, but also I added the opt-in prop whenVisibleOnly that will only render when a portion of the canvas is visible to demonstrate that extensibility. please re-open if your question remains unanswered.