pixijs / pixi-react

Write PIXI apps using React declarative style
https://pixijs.io/pixi-react/
MIT License
2.25k stars 173 forks source link

Feature Request: Texture/Sprite lifecycle ergonomics with useEffect #446

Open laino opened 1 year ago

laino commented 1 year ago

Description

Currently doing something like this does not work*:

function Test(props: any) {
    const [texture, setTexture] = useState<PIXI.Texture>();

    useEffect(() => {
        const canvas = document.createElement('canvas');

        // ...

        const texture = PIXI.Texture.from(canvas);

        setTexture(texture);

        return () => {
            texture.destroy(true); // <== problematic line
            setTexture(null);
        };
    }, [props]);

    if (!texture) {
        return null;
    }

    return <Sprite texture={texture} x={props.x} y={props.y}/>;
}

The reason being that after the cleanup function destroys the texture, it is still used by the Sprite shortly after. It's probably not specific to such an use-case, but likely anything that involves a texture being destroyed in a cleanup function.

It would be great for ergonomics if this could be made to work. If it is impossible due to constraints of React, having an utility function that takes a callback which is executed when it is safe to destroy unused resources would go a long way:

cleanup(() => texture.destroy(true))

* This is stack trace caused by destroying the texture without wrapping it in a setTimeout or similar:

Uncaught TypeError: orig is null
    calculateVertices Sprite.mjs:77
    _calculateBounds Sprite.mjs:138
    calculateBounds Container.mjs:176
    calculateBounds Container.mjs:182
    calculateBounds Container.mjs:182
    getBounds DisplayObject.mjs:67
    getLocalBounds DisplayObject.mjs:95
    getLocalBounds Container.mjs:200
    createDisplayComponent display-component.tsx:216
    applyProps index.es-dev.js:487
    commitUpdate index.es-dev.js:23454
    commitMutationEffectsOnFiber index.es-dev.js:17515
    recursivelyTraverseMutationEffects index.es-dev.js:17399
    commitMutationEffectsOnFiber index.es-dev.js:17472
    recursivelyTraverseMutationEffects index.es-dev.js:17399
    commitMutationEffectsOnFiber index.es-dev.js:17419
    recursivelyTraverseMutationEffects index.es-dev.js:17399
    commitMutationEffectsOnFiber index.es-dev.js:17727
    recursivelyTraverseMutationEffects index.es-dev.js:17399
    commitMutationEffectsOnFiber index.es-dev.js:17458
    recursivelyTraverseMutationEffects index.es-dev.js:17399
    commitMutationEffectsOnFiber index.es-dev.js:17727
    recursivelyTraverseMutationEffects index.es-dev.js:17399
    commitMutationEffectsOnFiber index.es-dev.js:17727
    recursivelyTraverseMutationEffects index.es-dev.js:17399
    commitMutationEffectsOnFiber index.es-dev.js:17558
    commitMutationEffects index.es-dev.js:17369
    commitRootImpl index.es-dev.js:20420
    commitRoot index.es-dev.js:20292
    performSyncWorkOnRoot index.es-dev.js:19693
    flushSyncCallbacks index.es-dev.js:4417
    ensureRootIsScheduled index.es-dev.js:19224
    ensureRootIsScheduled index.es-dev.js:19216
    scheduleUpdateOnFiber index.es-dev.js:19104
    updateContainer index.es-dev.js:22489
    componentDidUpdate index.es-dev.js:23910
    React 8
    workLoop scheduler.development.js:266
    flushWork scheduler.development.js:239
    performWorkUntilDeadline scheduler.development.js:533
jdnichollsc commented 9 months ago

Did you include that new canvas element in the DOM? 🤔

laino commented 9 months ago

No. It's being used to do some off-screen rendering with a 2D context and putting the result into a texture.

That really doesn't matter though - I could be getting that texture whichever way and end up with the same problem.