davidfig / pixi-viewport

A highly configurable viewport/2D camera designed to work with pixi.js
https://davidfig.github.io/pixi-viewport/
MIT License
1.05k stars 175 forks source link

React Pixi + Viewport and issues with v5 #438

Closed sergioisidoro closed 5 months ago

sergioisidoro commented 1 year ago

I'm trying to port the simplest version of the React Pixi Viewport from here:

https://codesandbox.io/s/react-pixi-viewport-9ngfd and https://codepen.io/inlet/pen/yLVmPWv

According to the latest breaking change from v5 interaction: props.app.renderer.plugins.interaction should be events: props.app.renderer.events

I've tried a couple of things but could not get anything to work. I wonder if something else has recently changed in v5? On 4.38.0 of pixi-viewport things still work ok. Documentation on the new usage of the events is also quite scarce, with most examples still pointing to interactions (including official docs)

Disclaimer, I'm super new to Pixi.js.

Reproduction:

    "@pixi/react": "^7.0.3",
    "pixi-viewport": "^5.0.1",
    "pixi.js": "^7.2.2",

https://codesandbox.io/s/react-pixi-viewport-forked-5gt5bl?file=/src/Viewport.tsx:712-720 (Note how changing the pixi-viewport to 4.38 fixes the issue)

Edit: Full trace:

react-reconciler.development.js:9860 The above error occurred in the <Viewport> component:

    at Viewport
    at Viewport (http://localhost:3000/static/js/bundle.js:366:66)

Consider adding an error boundary to your tree to customize error handling behavior.
Visit https://reactjs.org/link/error-boundaries to learn more about error boundaries.
logCapturedError @ react-reconciler.development.js:9860
update.callback @ react-reconciler.development.js:9893
callCallback @ react-reconciler.development.js:5094
commitUpdateQueue @ react-reconciler.development.js:5115
commitLayoutEffectOnFiber @ react-reconciler.development.js:15019
commitLayoutMountEffects_complete @ react-reconciler.development.js:16366
commitLayoutEffects_begin @ react-reconciler.development.js:16352
commitLayoutEffects @ react-reconciler.development.js:16290
commitRootImpl @ react-reconciler.development.js:18966
commitRoot @ react-reconciler.development.js:18825
performSyncWorkOnRoot @ react-reconciler.development.js:18225
flushSyncCallbacks @ react-reconciler.development.js:2936
(anonymous) @ react-reconciler.development.js:17756
pixi_viewport.js:1138 Uncaught TypeError: Cannot read properties of undefined (reading 'domElement')
    at $.addListeners (pixi_viewport.js:1138:1)
    at new $ (pixi_viewport.js:1135:1)
    at new ht (pixi_viewport.js:1343:1)
    at Object.create (PixiViewport.tsx:18:1)
    at createElement (element.js:60:1)
    at completeWork (react-reconciler.development.js:13604:1)
    at completeUnitOfWork (react-reconciler.development.js:18739:1)
    at performUnitOfWork (react-reconciler.development.js:18711:1)
    at workLoopSync (react-reconciler.development.js:18609:1)
    at renderRootSync (react-reconciler.development.js:18577:1)
addListeners @ pixi_viewport.js:1138
$ @ pixi_viewport.js:1135
ht @ pixi_viewport.js:1343
create @ PixiViewport.tsx:18
createElement @ element.js:60
completeWork @ react-reconciler.development.js:13604
completeUnitOfWork @ react-reconciler.development.js:18739
performUnitOfWork @ react-reconciler.development.js:18711
workLoopSync @ react-reconciler.development.js:18609
renderRootSync @ react-reconciler.development.js:18577
recoverFromConcurrentError @ react-reconciler.development.js:17958
performSyncWorkOnRoot @ react-reconciler.development.js:18204
flushSyncCallbacks @ react-reconciler.development.js:2936
(anonymous) @ react-reconciler.development.js:17756
pixi_viewport.js:1349 Uncaught TypeError: Cannot read properties of undefined (reading 'update')
    at ht.update (pixi_viewport.js:1349:1)
    at options.noTicker.tickerFunction [as fn] (pixi_viewport.js:1343:1)
    at TickerListener.emit (TickerListener.ts:71:1)
    at _Ticker.update (Ticker.ts:427:1)
    at _tick (Ticker.ts:129:1)

(Note that Viewport component is my own wrapper of pixi-viewport)

sergioisidoro commented 1 year ago

The issue seems to be related to this change: https://github.com/davidfig/pixi-viewport/commit/ee8f2dd0a45812cd44644ee3afe4665a90141d8d#diff-ddb343831b1574d25562089f63f1222cde16e19c841c1a400e5e5d0f1ad176f0R54

https://pixijs.download/release/docs/PIXI.EventSystem.html#domElement

The DOM element to which the root event listeners are bound. This is automatically set to the renderer's view. Default Value: undefined

Automatically, but I'm not really sure when (or if) that happens.

PS: Passing a brand new instance ofEventSystem also does not work events: new PIXI.EventSystem(props.app.renderer)

lazharichir commented 1 year ago

Have you tried simply importing import { EventSystem } from "@pixi/events"; without using it directly?

And in your viewport's instantiator, use events: app.renderer.events, (or props... for you).

Without the import, it wasn't working for me (vanillajs, not in React) and adding the import solved it.

lazharichir commented 1 year ago

@sergioisidoro i've setup react pixi (first time) and so this fixed it:

const PixiComponentViewport = PixiComponent("Viewport", {
  create: (props: PixiComponentViewportProps) => {

    // Install EventSystem, if not already
    // (PixiJS 6 doesn't add it by default)
    if (!("events" in props.app.renderer))
      props.app.renderer.addSystem(PIXI.EventSystem, "events");

    const { width, height } = props;
    const { ticker } = props.app;
    const { events } = props.app.renderer;

    const viewport = new PixiViewport({
      screenWidth: width,
      screenHeight: height,
      worldWidth: width,
      worldHeight: height,
      ticker: ticker,
      events: events,
    });

    viewport
      .drag()
      .pinch()
      .wheel()
      .clamp({ direction: "all" })
      .clampZoom({ minScale: 0.5, maxScale: 1 })
      .decelerate();

    return viewport;
  },
});
LouisGusta commented 1 year ago

I'm having some problems with the migration too, I updated the interaction, the viewport is working but when I go to the next page, it gives me a destruction error.

on my error it fails to access 'next' inside let listener = this._head.next at Ticker.ts

in the same error he also complains about the destroy(t) function

my project is: https://github.com/MatzeStudios/mazeonline/blob/mouse-fix/frontend/src/components/GameScreen/index.js

ben4d85 commented 1 year ago

I have tried the approach from @lazharichir and can confirm that this allows me to use pixi-viewport with pixi-react.

I think manually adding the event system is key (even if there is no error message related to it). It would be great if this could be addressed.

When navigating away from the page that contains my pixi-viewport app, I can confirm that I also receive the error mentioned by @LouisGusta, namely:

Cannot read properties of null (reading 'next') at _Ticker.remove

Also see this thread, where the problem is discussed as well: https://github.com/davidfig/pixi-viewport/issues/441#issuecomment-1571265457

Please can this be looked into? As is, this problem prevents me from using pixi-viewport in production.

Balinth commented 1 year ago

Hey all, I am still struggling with this, managed to work around all errors, see the full Viewport.tsx react wrapper on the bottom.

The main finding, is that the ticker is already destroyed by the Stage, and there is fortunately a noTicker option that prevents Viewport from trying to destroy it again. I am no js developer, so I have no idea what would be the actual good solution, but I would expect destroy methods to be idempotent, and not fail if called on an already destroyed object. (or rather, in this case, Ticler.remove not fail, if _head is already null)

import React, {forwardRef} from "react";
import * as PIXI from "pixi.js";
import {PixiComponent, useApp} from "@pixi/react";
import {Viewport as PixiViewport} from "pixi-viewport";
import {EventSystem} from "@pixi/events";
import {Container as PixiContainer} from "@pixi/display";

export interface ViewportProps {
    width: number;
    height: number;
    children?: React.ReactNode;
}

export interface PixiComponentViewportProps extends ViewportProps {
    app: PIXI.Application;
}

const PixiComponentViewport = PixiComponent("Viewport", {
    create: (props: PixiComponentViewportProps) => {

        // comes from github issue: https://github.com/davidfig/pixi-viewport/issues/438
        // Install EventSystem, if not already
        // (PixiJS 6 doesn't add it by default)

        const events = new EventSystem(props.app.renderer);
        events.domElement = props.app.renderer.view as any;

        const viewport = new PixiViewport({
            screenWidth: props.app.screen.width,
            screenHeight: props.app.screen.height,
            worldWidth: props.width * 2,
            worldHeight: props.height * 2,
            ticker: props.app.ticker,
            events: events,
        });
        viewport.drag().pinch().wheel().clampZoom({
            minWidth: 1,
            minHeight: 1,
            maxWidth: 10000,
            maxHeight: 10000,
        });

        return viewport;
    },

    willUnmount: (instance: PixiViewport, parent: PixiContainer) => {
        // workaround because the ticker is already destroyed by this point by the stage
        instance.options.noTicker = true;
        instance.destroy({children: true, texture: true, baseTexture: true})

    }
});

const Viewport = forwardRef(
    (props: ViewportProps, ref: React.Ref<PixiViewport>) => {
        const app = useApp();
        return <PixiComponentViewport ref={ref} app={app} {...props} />;
    }
);

export default Viewport;

This is with versions pixi/react 7.0.3 pixi.js 7.2.3 pixi-viewport 5.0.1 react 18.2

ben4d85 commented 1 year ago

I have tried the suggestion from @Balinth, i.e. adding willUnmount as shown in his code sample. I can confirm that the following error no longer appears:

Uncaught TypeError: Cannot read properties of null (reading 'next') at _Ticker.remove (webpack-internal:///(app-client)/./node_modules/@pixi/ticker/lib/Ticker.mjs:83:31) at ht.destroy (webpack-internal:///(app-client)/./node_modules/pixi-viewport/dist/pixi_viewport.js:1367:74) at _removeChild (webpack-internal:///(app-client)/./node_modules/@pixi/react/dist/index.es-dev.js:23304:11)

However, I now get a different error:

Uncaught TypeError: Cannot read properties of null (reading 'removeEventListener') at $.destroy (webpack-internal:///(app-client)/./node_modules/pixi-viewport/dist/pixi_viewport.js:1166:45) at ht.destroy (webpack-internal:///(app-client)/./node_modules/pixi-viewport/dist/pixi_viewport.js:1367:246) at ht.willUnmount (webpack-internal:///(app-client)/./src/app/[lang]/map/pixi/PixiMap.tsx:235:18)

Update: I realised that I only changed willUnmount to the code from @Balinth, but I overlooked that his code also uses a different approach to the EventSystem. See my next answer below.

Balinth commented 1 year ago

Didn't see that one yet. How / when is that happening for you?

smaylovkemal commented 1 year ago

@Balinth I have the same error as @ben4d85 . How can I fix it?

Heilemann commented 1 year ago

Here is mine, which is currently working (but throwing me for a loop about wheel-to-zoom not working as expected, but that's another matter).

import { PixiComponent, useApp } from '@pixi/react'
import { Viewport as PixiViewport } from 'pixi-viewport'
import * as PIXI from 'pixi.js'
import { ReactNode } from 'react'

export interface ViewportProps {
    children?: ReactNode
}

export interface PixiComponentViewportProps extends ViewportProps {
    app: PIXI.Application
}

const PixiViewportComponent = PixiComponent('Viewport', {
    create: ({ app }: PixiComponentViewportProps) => {
        // fix from https://github.com/davidfig/pixi-viewport/issues/438
        const events = new PIXI.EventSystem(app.renderer)
        events.domElement = app.renderer.view as any

        const viewport = new PixiViewport({
            events: events,
            ticker: app.ticker,
            allowPreserveDragOutside: true,
            // passiveWheel: false,
        })

        viewport
            .drag({
                clampWheel: false,
            })
            .wheel({
                percent: 0.1,
                trackpadPinch: true,
            })
            .decelerate()
            .clampZoom({
                minScale: 0.25,
                maxScale: 5,
            })

        viewport.on('wheel', e => {
            console.log(e)
        })

        return viewport
    },
    willUnmount: (viewport: PixiViewport) => {
        // @ts-ignore Hack to get around pixi-viewport bug when destroying
        viewport.options.noTicker = true
        viewport.destroy({ children: true, texture: true, baseTexture: true })
    },
})

const Viewport = props => <PixiViewportComponent app={useApp()} {...props} />

export default Viewport
ben4d85 commented 1 year ago

I have tried the code samples shared above and found the following.

(A) Suggestion from @lazharichir - does not work for me.

import { EventSystem } from '@pixi/events';

if (!("events" in props.pixiApp.renderer)) {
    props.pixiApp.renderer.addSystem(EventSystem, 'events');
}
const { events } = props.pixiApp.renderer;

PS: Another test that could potentially be done here is to use import * as PIXI from 'pixi.js' instead, with PIXI.EventSystem.

(B) Suggestion from @Balinth - does work for me.

import { EventSystem } from '@pixi/events';

const events = new EventSystem(props.pixiApp.renderer);
events.domElement = props.pixiApp.renderer.view as any;

(C) Suggestions from @Heilemann - does work for me.

import * as PIXI from 'pixi.js'

const events = new PIXI.EventSystem(props.pixiApp.renderer);
events.domElement = props.pixiApp.renderer.view as any;

Update: I just upgraded to:

and had some problems with the above. So now I am using:

import { EventSystem } from "pixi.js";

const events = new EventSystem(props.pixiApp.renderer);
events.domElement = props.pixiApp.renderer.view as any;
rob-myers commented 11 months ago

I got things working by directly patching pixi-viewport and @pixi/react, and adapting the above approach.

"@pixi/react": "^7.1.1",
"pixi-viewport": "^5.0.2",
"pixi.js": "^7.3.2",

@pixi/react patch:

--- a/node_modules/@pixi/react/dist/index.es-dev.js
+++ b/node_modules/@pixi/react/dist/index.es-dev.js
 import PropTypes from 'prop-types';
+import '@pixi/events';

and/or similarly in node_modules/@pixi/react/dist/index.{es,cjs,cjs-dev}.js.

pixi-viewport patch:

--- a/node_modules/pixi-viewport/dist/pixi_viewport.js
+++ b/node_modules/pixi-viewport/dist/pixi_viewport.js
   destroy() {
-    this.viewport.options.events.domElement.removeEventListener("wheel", this.wheelFunction);
+    this.viewport.options.events.domElement?.removeEventListener("wheel", this.wheelFunction);
   }

and/or similarly in node_modules/pixi-viewport/dist/pixi_viewport.umd.cjs.

// Viewport.tsx
import React from "react";
import type PIXI from "pixi.js";
import { PixiComponent, useApp } from "@pixi/react";
import { Viewport as PixiViewport } from "pixi-viewport";

const PixiComponentViewport = PixiComponent("Viewport", {
  create: ({ app }: PixiComponentViewportProps) => {
    app.renderer.events.domElement = app.renderer.view as any;

    const viewport = new PixiViewport({
      passiveWheel: false,
      events: app.renderer.events,
      ticker: app.ticker,
    });
    viewport.drag({
      wheel: false,
    }).wheel({
      wheelZoom: true,
    }).pinch().clampZoom({
      maxScale: 4,
      minScale: 1/4,
    });
    return viewport;
  },
  willUnmount: (viewport: PixiViewport) => {
    viewport.options.noTicker = true; // fix the `ticker` option above
    viewport.destroy({ children: true });
  },
});

export default function Viewport(props: ViewportProps) {
  const app = useApp();
  return <PixiComponentViewport app={app} {...props} />;
}

export interface ViewportProps {
  children?: React.ReactNode;
}

export interface PixiComponentViewportProps extends ViewportProps {
  app: PIXI.Application;
}

Then we can do e.g.

return (
    <Stage>
      <Viewport>
        <Sprite
          x={250}
          y={250}
          anchor={[0.5, 0.5]}
          interactive={true}
          image="https://s3-us-west-2.amazonaws.com/s.cdpn.io/693612/IaUrttj.png"
          pointerdown={(e) => {
            console.log("pointerdown", e);
          }}
        />
      </Viewport>
    </Stage>
);

To support <Sprite texture={foo} />, following a comment in https://github.com/pixijs/pixi-react/issues/452 I changed my webpack config as follows:

    resolve: {
      alias: {
        'pixi.js': path.resolve(__dirname, 'node_modules/pixi.js/dist/cjs/pixi.min.js'),
        '@pixi/react': path.resolve(__dirname, `node_modules/@pixi/react/dist/index.cjs${process.env.NODE_ENV === 'production' ? '' : '-dev'}.js`),
        'pixi-viewport': path.resolve(__dirname, 'node_modules/pixi-viewport/dist/pixi_viewport.umd.cjs'),
      },
    }
OxCom commented 8 months ago

@rob-myers thanks for idea. I found that such way does not optimal for me and there is my alternative approach.

As I could see the domElement from events removed before willUnmount. In this case we could store domElement and inject it temporary before destroy call and cleanup it later.

// ...

class PixiViewportPatch extends PixiViewport {
    private renderedDOMElement?: HTMLElement;

    constructor(options: IViewportOptions) {
        super(options);

        this.lockDOMElement();
    }

    lockDOMElement() {
        this.renderedDOMElement = this.options.events.domElement;

        return this;
    }

    patchEvents() {
        if (this.renderedDOMElement) {
            this.options.events.domElement = this.renderedDOMElement;
        }
    }

    releaseDOMElement() {
        this.renderedDOMElement = undefined;
    }
}

const PixiComponentViewport = PixiComponent("Viewport", {
    create: (props: PixiComponentViewportProps) => {
        const {events} = props.app.renderer;

        const instance = new PixiViewportPatch({
            // ...
            events: events,
        });

        // ...

        return instance;
    },
    willUnmount: (instance: PixiViewportPatch) => {
        // workaround because the ticker is already destroyed by this point by the stage
        instance.options.noTicker = true;
        // workaround DOMElement: the domElement has been removed from events before willUnmount()
        instance.patchEvents();
        instance.destroy({children: true, texture: true, baseTexture: true});
        // workaround DOMElement: restore changes after patch
        instance.releaseDOMElement();
    }
});

const Viewport = forwardRef<null, ViewportProps>(function Viewport(props: ViewportProps, ref: React.Ref<PixiViewportPatch>) {
    const app = useApp();

    return <PixiComponentViewport ref={ref} app={app} {...props} />;
});

export default Viewport;
schematical commented 7 months ago

@OxCom wow, that solved a somewhat unrelated problem I was having where the zoom and center was resetting pretty much every time I setState on anything. It was driving me crazy. That last bit where you wrap the original custom component with that const app = useApp(); instead of explicitly passing in the app from the parent might have been the real solution.