davidfig / pixi-cull

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

SpatialHash loses followed sprite and is always dirty #17

Closed AdrienLemaire closed 3 years ago

AdrienLemaire commented 3 years ago

Video demonstrating the issues: https://youtu.be/RPK06B6LeMo

Related code: ```ts const Viewport: React.FC = ({ worldWidth, worldHeight, children }) => { // Create a new pixi-viewport instance const viewport = useMemo(() => { const newViewport = new PixiViewport({ // the interaction module is important for wheel to work properly when // renderer.view is placed or scaled interaction: app.renderer.plugins.interaction, // FIXME: The viewport breaks if trying to use innerWidth/innerHeight screenHeight: document.documentElement.clientHeight, screenWidth: document.documentElement.clientWidth, ticker: app.ticker, worldHeight, worldWidth, }); // worldWidth and worldHeight are originally calculated from the tiled map as follows // worldWidth = tilewidth * width, worldHeight = tileheight * width const fitHeight = newViewport.findFitHeight(worldHeight); const fitWidth = newViewport.findFitWidth(worldWidth); const scale = Math.min(fitHeight, fitWidth); setMinScale(scale); newViewport // .clamp({ direction: "all" }) // stop the viewport from moving beyond your boundaries. .setZoom(0.1) .snap(worldWidth / 2, worldHeight / 2, { time: 0 }) .animate({ // also works with snapZoom // setZoom and fitWorld work without easing callbackOnComplete: () => setAnimationCompleted(true), ease: "easeInOutSine", scale, time: DELAY_MAP, }) .wheel() .decelerate(); return newViewport; }, [app, worldWidth, worldHeight]); // Reactify pixi-viewport const ViewportComponent = useMemo( () => PixiComponent("Viewport", { create: () => viewport }), [viewport] ); // cutting offscreen shapes const cull = useMemo(() => { // const newCull = new Cull.Simple({dirtyTest: false}); // newCull.addList(viewport.children); const newCull = new Cull.SpatialHash({ dirtyTest: true, }); newCull.addContainer(viewport); newCull.cull(viewport.getVisibleBounds()); return newCull; }, [viewport]); // cull whenever the viewport moves let notDirty = 0; useTick(() => { if (viewport.dirty) { notDirty = 0; cull.cull(viewport.getVisibleBounds()); const { total, culled } = cull.stats(); CullPanel.update(culled, total); viewport.dirty = false; } else notDirty += 1; if (notDirty > 1) console.log("viewport tick not dirty"); }); useEffect(() => { // Trigger the character follow once the map animation is completed if (animationCompleted && characterRef.current) { const sprite = characterRef.current; viewport .animate({ callbackOnComplete: () => { viewport.follow(sprite.parent, { acceleration: 1 }); // At this point, all tiles should be available in the world, so we // start sorting the sprites inside the Viewport every frame app.ticker.add(sortViewport); }, position: sprite.parent.position, scale: 1, time: DELAY_ZOOM, }) .pinch() // Allow zooming out on mobiles with 2 fingers // maxScale > 1 will display seams around tiles .clampZoom({ maxScale: 2, minScale }); } }, [viewport, animationCompleted, minScale, app.ticker, sortViewport]); return {children}; ```

Since I'm working on a huge map with 20000+ sprites, cull.Simple isn't enough (FPS drops from 60 to ~20 when not moving).

First issue: When changing the culling from Simple to SpatialHash, the sprite that is followed by viewport is culled out when (I guess) it gets out of the default 1000x1000 cull cell.

Second issue: Whenever I call cull.cull() && viewport.dirty = false, the next tick is false, and the following one is always back to true. When I'm not moving and nothing is moving on the screen, I'm expecting the viewport.dirty flag to stay false. Note that if I press on the screen and keep the followed sprite's velocity to 0, viewport.dirty do stays false as long as my finger is pressing the screen.

Do you have suggestions on how to resolve these issues ?

davidfig commented 3 years ago

First issue: When changing the culling from Simple to SpatialHash, the sprite that is followed by viewport is culled out when (I guess) it gets out of the default 1000x1000 cull cell.

You need to make sure that moving sprites's bounding boxes are updated. By default they are not (for performance reasons). Set sprite.dirty = true to update the bounding box on sprites that are moving, rotating, etc.

Second issue: Whenever I call cull.cull() && viewport.dirty = false, the next tick is false, and the following one is always back to true. When I'm not moving and nothing is moving on the screen, I'm expecting the viewport.dirty flag to stay false. Note that if I press on the screen and keep the followed sprite's velocity to 0, viewport.dirty do stays false as long as my finger is pressing the screen.

Umm...I'm not sure what's causing the viewport to become dirty. The viewport only sets itself dirty when there's a user interaction. My guess is that this is a bug in your code. Let me know if you discover that the viewport is doing something wrong.

AdrienLemaire commented 3 years ago

You need to make sure that moving sprites's bounding boxes are updated. By default they are not (for performance reasons). Set sprite.dirty = true to update the bounding box on sprites that are moving, rotating, etc.

I see ! Thanks, one issue resolved.

Umm...I'm not sure what's causing the viewport to become dirty. The viewport only sets itself dirty when there's a user interaction. My guess is that this is a bug in your code. Let me know if you discover that the viewport is doing something wrong.

Roger that, will try to debug that

AdrienLemaire commented 3 years ago

@davidfig as you can see in the code shared above, I call .snap() only once after creating the viewport, and I call .follow() only once in the animate callbackOnComplete. But I can see that at each animationFrame, after I cull and set viewport.dirty = false, the Snap.update() method is called once, and the Follow.update() method is called 3 times, even if no action is done on the screen and no sprite is moving.

Do you understand why this happen ? Also, why aren't there any check to verify that the center has changed before calling moveCenter ?

I wonder if this issue is related to https://github.com/davidfig/pixi-viewport/issues/81 (with a snap/follow conflict instead of a clamp/follow one)

davidfig commented 3 years ago

snap() is ongoing (think of it as snap back). you need to pass options.removeOnComplete = true to make it a one time call. There will be a conflict between snap and follow since both want to move the viewport. That might be what's causing the problem.

follow.update() should only be called once every loop. I'm not sure why it's being called 3x. That might be a bug if you're seeing that.

moveCenter and moveCorner do not check whether there is a change before changing viewport's position. Not sure if it's worth checking since there is little cost in changing the viewport's position.

AdrienLemaire commented 3 years ago

Not sure if it's worth checking since there is little cost in changing the viewport's position.

The cost is in toggling the dirty flag, which retriggers pixi-cull :)

davidfig commented 3 years ago

Ah! Yes, you are right. Let me add that check.

davidfig commented 3 years ago

I added the check to moveCorner and moveCenter in v4.22.0.

AdrienLemaire commented 3 years ago

@davidfig adding {removeOnComplete: true} seems to have solved the snap&follow issue. Thank you very much !