Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Talk about renderChildren again. #4369

Closed
finscn opened this issue Oct 17, 2017 · 7 comments
Closed

Talk about renderChildren again. #4369

finscn opened this issue Oct 17, 2017 · 7 comments
Labels
🙏 Feature Request Community request for new features, APIs, packages.

Comments

@finscn
Copy link
Contributor

finscn commented Oct 17, 2017

One year ago , I created a PR #3108 .

I want to add the standalone methods about rendering & updating children.
The reason is that user could override them for controlling which sprite/how render the sprite by themself .
example:

There is a DisplayObject-Tree:

containerA
|_____ spriteA
|_____ spriteB
|_____ spriteC
|_____ subContainerA
|_____ ... some other sprites else ...

Sometimes , I want to decide that render which sprite/how render the sprite by myself, like this :

if ( condition1 ) {
    spriteA.renderWebGL(renderer);
    spriteB.renderWebGL(renderer);
} else if ( condition2 ) {
    spriteB.renderWebGL(renderer);
    spriteC.renderWebGL(renderer);
    spriteA.renderWebGL(renderer);
} else  if ( condition3 )  {
    spriteC.renderWebGL(renderer);
    subContainerA.renderWebGL(renderer);
} else {
    subContainerA.renderWebGL(renderer);
}

The logic above can't be solved by children.sort.
If we extract the method about rendering children , user could override the renderChildrenWebGL & renderChildrenCanvas, it's easy.
there is another use case for custom remove :

container.renderChildrenWebGL = function(renderer) {
    // In one game loop , the render is the last thing about an entity normally.
    // So we could remove the entity in render function.
    var i = 0,
        len = this.children.length;
    while (i < len) {
        var child = this.children[i];
        if (child._toRemove) {
            len--;
            this.removeChildAt(i, child);
            continue;
        }
        child.renderWebGL(renderer);
        i++;
    }
};

If no standalone renderChildren , users have to override renderWebGL , renderCanvas, renderAdvancedWebGL , it's complex , in especial renderAdvancedWebGL.
And if the logic in Container.prototype.renderWebGL/renderAdvancedWebGL changed , users have to re-override them again.

But the logic of renderChildrenWebGL & renderChildrenCanvas is very clear , and they won't be
changed.

One year ago , @englercj refused my PR , the reason is performance .
In the past year , I've tested many games (more than 20 games , and more than 100 test-cases) , I found this PR is not the hotspot of performance, even never affected performance.
(BTW, I found the updateTransformChildren is needless , I and my friends never used it).

In the real game , we could use many sprites , but the count of container is not big at the same time( in one game tick), so the 1 function-call won't take the performance down.

So I hope you could think the PR again, please, thank you.

finscn pushed a commit to finscn/pixi.js that referenced this issue Oct 17, 2017
Extract standalone `renderChildren` method from `render`
more detail see : pixijs#4369
@ivanpopelyshev
Copy link
Collaborator

Yeah, agree on renderWebGL, we certainly have to decouple it somehow.

@ivanpopelyshev
Copy link
Collaborator

ivanpopelyshev commented Oct 17, 2017

renderChildrenWebGL , remove child in render is VERY bad idea, we need special phase for it for sure. even updateTransform isn't good for it.

I agree that something must be done there but your solution is not enough :(

@finscn
Copy link
Contributor Author

finscn commented Oct 17, 2017

@ivanpopelyshev , I know the example about remove children is very bad . But the performance of JS is worse.
when there are many children (example in a particle system , more than 10000 children), use a standalone phase for removing , we have to use one more for/while-loop , it's so bad.

So , when I use JS to create a game , for performance , I would do some bad things.

OK, whatever , the example about remove children is not the key point of this PR.

@themoonrat themoonrat added 🙏 Feature Request Community request for new features, APIs, packages. Renderer: WebGL labels Jul 5, 2018
@themoonrat
Copy link
Member

@finscn Is this an issue you wish to keep open, or would you like to raise a new issue with another plan of attack? Thanks!

@finscn
Copy link
Contributor Author

finscn commented Jul 5, 2018

@themoonrat , I don't know anything about v5 and pixi-display / pixi-layer repo , so I can't answer your question now.

@ivanpopelyshev
Copy link
Collaborator

@finscn im working on a way to override pixi container methods in a plugin, and your input is significant. Lets close this one, I'll address it later :)

@lock
Copy link

lock bot commented Jul 5, 2019

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@lock lock bot locked and limited conversation to collaborators Jul 5, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
🙏 Feature Request Community request for new features, APIs, packages.
Projects
None yet
Development

No branches or pull requests

3 participants