Hard FPS lost when movin meshes

There is actually one box at a time on the scene :stuck_out_tongue: The one in the middle.

It looks like that’s not a repro of the issue then. The problem only arise when there are more obstacles right?

@blake @Alecell Didn’t try to comment the line at line 53 yet though, pardon me :smiley:
EDIT: @Alecell ok actually what’s the point of not removing the observer here? That’s the case I was asking about. You must clean up your observers because they will stack up.

He is not removing the observers so they stack up and the system obviously dies :cry:

2 Likes

But there are only few meshes. It must work with BJS 4.2.1. I’ll dig deeper :sunglasses:

1 Like

But I still don’t understand why it would be wanted to keep running code that updates a disposed mesh’s position and checks it for intersections before calling dispose again… Isn’t the solution just to not comment that line out and remove the observer? I don’t understand what problem commenting out that line is trying to solve I guess…

@Alecell I’ve found a couple of flaws in your code. Fixing. However I already rewrote your code so it is not dropping FPS anymore:

obstacles.ts

import { Mesh, MeshBuilder, Nullable, Observer, Scene } from "@babylonjs/core";
import { IObstacles } from "./types";

export class Obstacles implements IObstacles {
  private prevTime = new Date().getTime();
  private currentTime = new Date().getTime();
  private trySpawnTime = new Date().getTime();
  private trySpawnTimeout: number = 0;

  constructor(private scene: Scene, private player: Mesh) {}

  private static _Obstacles: Mesh[] = [];
  private static _Observer: Nullable<Observer<Scene>>;

  public static Start(scene: Scene, player: Mesh) {
    Obstacles._Observer = scene.onBeforeRenderObservable.add(() => {
      Obstacles._Obstacles.forEach((obstacle, idx) => {
        const animationRatio = scene.getAnimationRatio();
        obstacle.position.x += -0.05 * animationRatio;

        if (player.intersectsMesh(obstacle)) {
          obstacle.dispose(true);
          Obstacles._Obstacles.splice(idx, 1);
        }
      });
    });
  }

  public static End(scene: Scene) {
    // remove observer
    if (Obstacles._Observer) {
      scene.onBeforeRenderObservable.remove(Obstacles._Observer);
    }
  }

  private createObstacle() {
    const obstacle = this.generateObstacle();

    obstacle.position.z = 3.55;
    obstacle.position.y = -0.75;
    obstacle.position.x = 1.5;

    Obstacles._Obstacles.push(obstacle);
  }

  
  private canSpawn(minDelay: number, maxDelay: number) {
    const isMaxDelay = this.currentTime - this.prevTime > maxDelay;
    const isMinDelay = this.currentTime - this.prevTime > minDelay;
    const canTry = this.currentTime - this.trySpawnTime > this.trySpawnTimeout;
    let shouldSpawn = false;

    if (canTry) {
      this.trySpawnTime = this.currentTime;

      if (isMinDelay) {
        shouldSpawn = Math.random() > 0.5;

        if (isMaxDelay) shouldSpawn = true;
      }
    }

    if (shouldSpawn) this.prevTime = this.currentTime;

    return shouldSpawn;
  }

  private generateObstacle() {
    return MeshBuilder.CreateBox("obstacle", {
      width: 0.1,
      height: Math.random(),
      depth: 1,
    });
  }


  spawnWithDelay(minDelay: number, maxDelay: number) {
    this.currentTime = new Date().getTime();
    this.trySpawnTimeout = Math.floor(minDelay / 4);

    if (this.canSpawn(minDelay, maxDelay)) {
      this.createObstacle();
    }
  }
}

index.tsx
Insert at line 30:

  Obstacles.Start(scene, player)
3 Likes

Here you go! :vulcan_salute: Live long and prosper!

catrun.zip (1.2 MB)

EDIT: Feel free to ask after you have examined the code.

EDIT2: I’ve just solved the loading of the landscape. I don’t know what’s your purpose with it, but your code was throwing an exception and didn’t load it at all.

There wasn’t a point exactly on don’t remove the obstacle nor the observable I was just testing things when I fall on this error and I wanted to understand it :sweat_smile:

But why when I put the onBeforeRenderObservable on the constructor I still with the error? I mean, adding it on the constructor wasn’t suppose to avoid the stacking, adding only one observer? :thinking:

(sorry for the late response, I was on a meeting)

No worries, we are enjoying helping the lost souls here :stuck_out_tongue:

Getting of the animation ratio should be moved one level up. Anyway in all the examples I saw until now you’re the first one who uses this method. Congratulations!

Many demos are not playable on higher framerates because the dudes are not using it. :sunglasses: I use it in all my projects as well.

1 Like

Hey @roland I didn’t provided the Solution badge because the biggest question was more of a why things got the FPS lost, you said that was because of the huge stacking of the onBeforeRenderObservable but if I do the example below I’m not stacking them, but got the error anyway.

Do you know why? :thinking:

Here I add the onBeforeRenderObservable inside the constructor and I generate the object only one time, so, despite the several code mistakes, I believe I’m not stacking, am I?

import { Mesh, MeshBuilder, Scene } from '@babylonjs/core';
import { IObstacles } from './types';

export class Obstacles implements IObstacles {
  private prevTime = new Date().getTime();
  private currentTime = new Date().getTime();
  private trySpawnTime = new Date().getTime();
  private trySpawnTimeout: number = 0;
  private obstacles: Mesh[] = []

  constructor(
    private scene: Scene, 
    private player: Mesh,
  ) {
    this.scene.onBeforeRenderObservable.add(() => {
      const animationRatio = this.scene.getAnimationRatio();

      this.obstacles.forEach(obstacle => {
        obstacle.position.x += -0.05 * animationRatio;

        if (this.player.intersectsMesh(obstacle)) {
          obstacle.dispose(true);
        }
      })
    });
  }

  private canSpawn(minDelay: number, maxDelay: number) {
    const isMaxDelay = this.currentTime - this.prevTime > maxDelay;
    const isMinDelay = this.currentTime - this.prevTime > minDelay;
    const canTry = this.currentTime - this.trySpawnTime > this.trySpawnTimeout;
    let shouldSpawn = false;

    if (canTry) {
      this.trySpawnTime = this.currentTime;

      if (isMinDelay) {
        shouldSpawn = Math.random() > 0.5;
        
        if (isMaxDelay) shouldSpawn = true;
      }
    }

    if (shouldSpawn) this.prevTime = this.currentTime;

    return shouldSpawn;
  }

  private generateObstacle() {
    return MeshBuilder.CreateBox('obstacle', { width: 0.1, height: Math.random(), depth: 1 })
  }

  private createObstacle() {
    const obstacle = this.generateObstacle();
  
    obstacle.position.z = 3.55;
    obstacle.position.y = -0.75;
    obstacle.position.x = 1.5;

    this.obstacles.push(obstacle);
  }

  spawnWithDelay(minDelay: number, maxDelay: number) {
    this.currentTime = new Date().getTime();
    this.trySpawnTimeout = Math.floor(minDelay / 4);

    if (this.canSpawn(minDelay, maxDelay)) {
      this.createObstacle();
    }
  }
}

EDIT: I forgot to mention that, on this example also, if I comment obstacle.position.x += -0.05 * animationRatio; the FPS don’t drop too

Ok, I get the point. However a Half solution badge would be cool :smiley:
Can you make a video like I did where I can see the issue? This is running using your code from the previous post.

https://misc.babylonjs.xyz/catrun.mkv

I think Roland nailed it, you need to remove the disposed obstacle from the obstacles array like he did, otherwise you’ll keep checking it for intersections, each frame checking intersections against all the disposed meshes. And in the first version if you don’t remove the observer then it keeps checking for intersections against the old meshes as well (Roland nailed that part early on too I think). :slight_smile:

Hmm still don’t get why the FPS doesn’t drop when not moving the obstacles thou, if you’re still spawning new ones… :thinking:

1 Like

Thanks @Blake! :slight_smile:

@Alecell
The second thing that if you don’t remove the obstacle from the array but you dispose() the mesh you will perform operations on a disposed mesh.

However I can’t see any performance issues running @Alecell 's current code.

1 Like

Hey @roland I made the video you suggested, I hope you could understand my poor english speech :sweat_smile:

(I upload the video on youtube cause I’m not able to post my video here, also watch it directly on youtube, the resolution right here is very low I don’t know why)

EDIT: I forgot to mention that with spacebar we can jump, even jumping the obstacles (they don’t disappear) the FPS still drop

1 Like

Hey @Alecell!
I just saw the video. I am going to try to run the game on my Raspberry Pi4 so I’ll match your OS and to rule out the possibility that there is actually an fps drop on my HW as well however it’s powerful enough to play the scene smoothly despite the performance loss. I’ll get back to you shortly. Your English is perfect :blush: better than mine :joy: the cat in the background was cute :heart_eyes_cat:

1 Like

Try this:

image

EDIT: This is temporary, we need to do it another way I am just curious whether this helps.

1 Like

It just works!!!

Also tested if I jump all the obstacles (without the splice), and doing this the FPS don’t drop too!

What an interesting thing, maybe something happens on the babylonjs that if I try to update a disposed object it leads to that FPS loss :thinking:

Thank you very much @roland!!

Also thank you @Blake

You guys help me a lot :grin:

2 Likes

The quick hack I’ve provided has a flaw. It works now because you have only 1 obstacle on the scene at a time. If you add more it will skip one obstacle in the forEach loop because splice() removes the array entry and reindexes the array immediatelly.

  1. you have to use a for loop from the last entry to the first, this way it will not skip any entries

or

  1. you can use this solution which I like better. Solution 1 will be a little bit more performant but I’m not really keen on classic for loops and you’ll end up with a few obstacles on scene anyway and it really doesn’t matter in this case.
  constructor(private scene: Scene, private player: Mesh) {
    const toRemove: number[] = [];
    this.scene.onBeforeRenderObservable.add(() => {
      const animationRatio = this.scene.getAnimationRatio();

      this.obstacles.forEach((obstacle, idx) => {
        obstacle.position.x += -0.05 * animationRatio;

        if (this.player.intersectsMesh(obstacle)) {
          toRemove.push(idx);
          obstacle.dispose(true);
        }
      });

      this.obstacles = this.obstacles.filter(
        (_, i) => toRemove[i] === undefined
      );
      toRemove.length = 0;
    });
  }

A disposed mesh is simply disposed and you can’t rely on a disposed mesh or any disposed object to behave normally. So this is expected. That’s what I was writing about earlier and that’s what @blake was trying to remind of.

There is a mesh.isDisposed() function available so if you are not sure whether your mesh is disposed at the time you are accessing it for any reason, use it.

Glad to help dude! @blake thanks for participating!

:vulcan_salute:

3 Likes