[WEB-Canvas] Sprite setColor generates cache canvas and create chaos in updating logic

[yi]

details about this problem as following:

100% reproducible, only happen in canvas rendering.

Steps to reproduce:

1. create a new cc.Sprite instance (var node)
2. node.setSpritFrame(frame) setting sprite frame from a sprite sheet to the node
3. node.setColor(colorA) setting color to that node
4. node.setColor(colorB) change the color of that node
5. then the whole sprite sheet texture will be draw on that node.

Cause:

https://github.com/cocos2d/cocos2d-html5/blob/develop/cocos2d/core/sprites/CCSprite.js#L964

because a cc.Sprite doesn't keep the instance of given sprite frame, it lost the clipping rect when rebuilding node._rect

Root cause:

I would suggest to decoupling the current implementation between RenderCMD and cc.Node. The current tough coupling between these two is likely to spawn bug. I suggest to make RenderCMD a pure stateless helper, as well as providing unified interface for cc.Node(and child classes) to call the helper and do the rendering job.

[pandamicro]

Hi, @yi Thanks for your suggestion, here is my thought about this problem

You are right, the core problem is that we are mixing cache logic with render command, it was originally due to performance concern. But finally it has caused us too much bugs, and I agree with you that we need to refactor this part of engine.

I think we can abstract the render command to more basic level to solve this problem, instead of sprite render command, we should do:

1. Make Texture render command, with texture, texture rect
2. Canvas cache is just a type of Texture, texture can be an Image element or a Canvas element
3. Sprite should save several render command that can be switched from one to another, maybe original render command and current render command is good enough, but maybe not.

@yi @VisualSJ What do you think ?

[yi]

@pandamicro +1, absolutely agree!

By the way, after we have a stateless Texture render command, there will be no need to maintain an instance of _renderCmd in every node instance. Oh yeah! will be less code and simplier structure.

Even the texture render helper just need to init once when booting the framework, and then every instance of Node could simply call methods provided by the helper, regardless weither is rendering on a canvas element or via webgl. (that's the good side of using require)

[VisualSJ]

https://github.com/cocos2d/cocos2d-html5/pull/3115 this pr is separated Texture's update logic.

[pandamicro]

VisualSJ have improved the texture's logic Future improvement will be scheduled to next version

[zilongshanren]

@pandamicro Any updates?

[pandamicro]

cocos2d/cocos2d-html5#3115 was a proper update for this, future improvements rely on refactorization of renderer, so I will close this issue. Then we will come up with new issues for renderer