davidfig / pixi-cull

a library to visibly cull objects designed to work with pixi.js
MIT License
112 stars 15 forks source link

[Feature] cull() callback #18

Closed AdrienLemaire closed 3 years ago

AdrienLemaire commented 3 years ago

After culling non-visible sprites, I do other operations like calling stats() and queryCallback().

  // cull whenever the viewport moves
  useTick(() => {
    if (viewport.dirty) {
      const AABB = viewport.getVisibleBounds();
      cull.cull(AABB);
      // Also show/hide debug boxes on visible sprites
      cull.queryCallback(AABB, (sprite) => {
        if (sprite.children.length > 1) {
          const boxes = sprite.children[sprite.children.length - 1];
          if (boxes.visible !== showSpriteBoxes)
            boxes.visible = showSpriteBoxes;
        }
        return false;
      });
      // Update stats.js panel info
      const { total, culled } = cull.stats();
      CullPanel.update(culled, total);
      viewport.dirty = false;
    }
  });

When reading the code, I realize that the O(n^2) double-for loop operation is repeated 3 times (in cull(), queryCallback() and stats()).

Could we have a callback option to cull(), and maybe return stats including lastBuckets ? The children would be looped only once in that case, and my code would look like:

  // cull whenever the viewport moves
  useTick(() => {
    if (viewport.dirty) {
      const { total, culled } = cull.cull(
        viewport.getVisibleBounds(),
        // Also show/hide debug boxes on visible sprites
        (sprite) => {
          if (sprite.children.length > 1) {
            const boxes = sprite.children[sprite.children.length - 1];
            if (boxes.visible !== showSpriteBoxes)
              boxes.visible = showSpriteBoxes;
          }
          return false;
        }
      )
      // Update stats.js panel info
      CullPanel.update(culled, total);
      viewport.dirty = false;
    }
  });
davidfig commented 3 years ago

Adding the stats will result in additional looping to find the number of culled objects. Since that's mostly used for debug purposes, I'm going to leave it as is. I will add an optional callback for non-culled objects to the cull function.

davidfig commented 3 years ago

Published as v1.1.0.

AdrienLemaire commented 3 years ago

@davidfig I'm having issues installing that latest version

$ npm install pixi-cull@1.1.0 -E
npm ERR! code ERESOLVE
npm ERR! ERESOLVE unable to resolve dependency tree
npm ERR!
npm ERR! Found: pixi.js@6.0.0
npm ERR! node_modules/pixi.js
npm ERR!   peer pixi.js@">=4.6.0" from pixi-viewport@4.22.0
npm ERR!   node_modules/pixi-viewport
npm ERR!     pixi-viewport@"4.22.0" from the root project
npm ERR!   pixi.js@"6.0.0" from pixi.js-legacy@6.0.0
npm ERR!   node_modules/pixi.js-legacy
npm ERR!     pixi.js-legacy@"6.0.0" from the root project
npm ERR!
npm ERR! Could not resolve dependency:
npm ERR! pixi-cull@"1.1.0" from the root project
npm ERR!
npm ERR! Conflicting peer dependency: pixi.js@5.3.9
npm ERR! node_modules/pixi.js
npm ERR!   peer pixi.js@"^5.0.0-rc.2" from pixi-cull@1.1.0
npm ERR!   node_modules/pixi-cull
npm ERR!     pixi-cull@"1.1.0" from the root project
npm ERR!
npm ERR! Fix the upstream dependency conflict, or retry
npm ERR! this command with --force, or --legacy-peer-deps
npm ERR! to accept an incorrect (and potentially broken) dependency resolution.

Since you didn't create a git tag for v1.0.1, it's difficult to debug what has changed between the versions. Could you also add the callback param to the cull() type ? https://github.com/davidfig/pixi-cull/blob/master/%40types/index.d.ts#L93

davidfig commented 3 years ago

Hmm..I updated the types in v1.1.1.

I'm not sure what's going on with your npm install. I just installed piix.js pixi-viewport and pixi-cull to a clean project and I didn't get any errors. You might want to delete your lock file and node_modules directory and reinstall everything. That sometimes cleans stuff out.

AdrienLemaire commented 3 years ago

closing since this was completed. I ended up not using it because this function is called before setting object.visible=true, and I need to know this value in my callback.