davidfig / pixi-cull

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

Sprites & BItmapTexts that are rotated are made invisible too soon and visible not early enough #2

Open NikkiKoole opened 5 years ago

NikkiKoole commented 5 years ago

http://nikkikoole.github.io/culltest/ https://github.com/NikkiKoole/nikkikoole.github.io/blob/master/culltest/main.js

Per sprite and bitmap text i also draw a rectangle for its bounds. Those are culled correctly, but the sprites themselves arent.

I also tried it with the latest release from a day ago, also i am on pixi 5rc2 but see the same behaviour on 4.8.

Or am i just overlooking something obvious alltogether?

ps. when all the sprites and bitmaptexts arent rotated its working correctly.

edit:

just using the demo from you readme file: add

sprite.rotation = Math.random() * Math.PI*2;

where the red boxes are generated and you see the behaviour too: dragging around you see some boxes being deleted from the view too soon.

zakjan commented 4 years ago

Indeed, code for calculating AABB is handling scaling, but missing rotation at: https://github.com/davidfig/pixi-cull/blob/master/code/simple.js#L167-L171 https://github.com/davidfig/pixi-cull/blob/master/code/spatial-hash.js#L206-L211

@davidfig Would adding code for handling rotation of all objects be feasible for performance? Such as https://www.asawicki.info/news_1376_calculating_aabb_of_a_rotated_2d_rectangle

davidfig commented 4 years ago

Yes, performance would be fine; it's what pixi already does (although they use matrices).

An alternative is to use pixi's getBoundingBox(), which already includes rotations and skewing. My original version of pixi-cull used that function, but after I struggled unsuccessfully to get it working, I ended up switching to my own simple bounding box calculation. I didn't need rotations, which is why I omitted it.

Happy to accept a PR if you want to add rotations to the calculations. I don't think it takes into account anchors either, so if it's important to you, feel free to add it.

zakjan commented 4 years ago

I got a working code with rotation, but it's missing other transformations.

  const rotPivot = new PIXI.Point(object.x + object.pivot.x, object.y + object.pivot.y);
  const rotSin = Math.sin(object.rotation);
  const rotCos = Math.cos(object.rotation);

  const points = [
    new PIXI.Point(object.x, object.y),
    new PIXI.Point(object.x + object.width, object.y),
    new PIXI.Point(object.x, object.y + object.height),
    new PIXI.Point(object.x + object.width, object.y + object.height),
  ];
  const pointsTransformed = points.map(point => {
    return new PIXI.Point(
      ((point.x - rotPivot.x) * rotCos - (point.y - rotPivot.y) * rotSin) + rotPivot.x,
      ((point.x - rotPivot.x) * rotSin + (point.y - rotPivot.y) * rotCos) + rotPivot.y
    );
  });

  const bounds = new PIXI.Bounds();
  pointsTransformed.map(point => bounds.addPoint(point));
  const aabb = bounds.getRectangle();

  object.AABB = aabb;
  object.dirty = true;

Did you mean getBounds method? It would be ideal to use a native PIXI method, but there are lots of complains about this method. When I tried it, sometimes it returned exactly 1/2 of the expected values, or it was completely off, no idea why.

Maybe we could somehow apply world+localTransform to the 4 corner points?