Ryuma-Hachisu commented 8 years ago

oldshaderのではなくnewshaderの方でretainのし忘れがあり、それが原因でカラーブレンドのアニメの時に落ちています。

手元では直しましましたが修正されてないみたいなので、報告します。

CustomSpriteのcocos2d::GLProgramState* _shaderProgramState; です。

こちら

void CustomSprite::changeShaderProgram(bool useCustomShaderProgram)
{
    if (useCustomShaderProgram != _useCustomShaderProgram)
    {
        if (useCustomShaderProgram)
        {
            cocos2d::GLProgram *shaderProgram = getCustomShaderProgram();
            if (shaderProgram == nullptr)
            {
                // Not use custom shader.
                shaderProgram = _defaultShaderProgram;
                useCustomShaderProgram = false;
            }
#if OLDSHADER_USE
            this->setGLProgram(shaderProgram);
#else           
            if (_shaderProgramState == 0)
            {
                _shaderProgramState = cocos2d::GLProgramState::create(shaderProgram);
            }
            this->setGLProgramState(_shaderProgramState);
#endif

に起きまして、 this->setGLProgramState(_shaderProgramState);と

node側にセットしています。この際node側で当然retainしているので一見大丈夫なように見えますが、 _shaderProgramState側ではretainをしていないのにもかかわらず。

CustomSprite::~CustomSprite()
{
    //エフェクトクラスがある場合は解放する
    if (refEffect)
    {
        delete refEffect;
        refEffect = 0;
    }
#if OLDSHADER_USE
#else
    if (_shaderProgramState)
    {
        _shaderProgramState->release();
        _shaderProgramState = 0;
    }
#endif
}

とデストラクタでreleaseされちゃっており、node側でも当然セットされてるのでそちらでも解放され二十解放されてしまってます。

のでクラス側でも、保持する場合はcocos2d-xのルールとしてはretainをするようにしなければなりません。

if (_shaderProgramState == 0)
{
        _shaderProgramState = cocos2d::GLProgramState::create(shaderProgram);
                _shaderProgramState->retain();
}

が正しい動作になります。

DKAlpha commented 8 years ago

ご報告ありがとうございます。こちら確認しまして反映したいと思います。

DKAlpha commented 8 years ago

こちらの対応をおこないましたV1.3.3予定のコードをdevelopブランチにアップいたしました。合わせてカラーブレンドの再現性向上対応も行いました。https://github.com/SpriteStudio/SS5PlayerForCocos2d-x/issues/53

お手数ではございますがこちらで動作を確認いただければと思います。

Ryuma-Hachisu commented 8 years ago

developを今軽くレビューしましたが、 void CustomSprite::changeShaderProgram(bool useCustomShaderProgram)の

if OLDSHADER_USE

else

        //ディフォルトシェーダープログラムを設定すると今まで使っていたシェーダプログラムは解放される
        _shaderProgramState = 0;

endif

の部分ですが、以前入れていた _shaderProgramState->release(); が逆に今度は必要になります。retainしているので0だけ突っ込むとリークします。

_shaderProgramState->release(); //ディフォルトシェーダープログラムを設定すると今まで使っていたシェーダプログラムは解放される _shaderProgramState = 0;

にするべきだと思います

Ryuma-Hachisu commented 8 years ago

また、動作確認しましたがスプライトのアルファが0の部分のピクセルが黒くなっていました。ピクセルシェーダーを確認した所、

if((u_partblend==1)||(u_partblend==3)) \n\ { \n\ pixel.rgb *= pixel.a ; \n\ }
になっているので、アルファが少ないものほど(0.02などの)真っ黒になっているとものと思います。またシェーダーを拝見しましたが、論理文が多すぎです。

シェーダーというかGPUは論理系の処理は非常に弱く、数字演算には強いCPUロジックになっています。物によってはですがif文を排除するだけで50%程度高速化するなどの影響力を持つほどです。

なのでこういう方法をおすすめします。極力if文を防ぐためにシェーダーを分散する。（設定によって扱うシェーダーを変える）

なんにしても現状のものだと黒くなってしまい問題ですので修正もよろしくお願いします。

DKAlpha commented 8 years ago

ご確認ありがとうございます。まずカスタムシェーダーについてのご指摘を整理すると

void CustomSprite::changeShaderProgram(bool useCustomShaderProgram)
{
    if (useCustomShaderProgram != _useCustomShaderProgram)
    {
        if (useCustomShaderProgram)
        {
            cocos2d::GLProgram *shaderProgram = getCustomShaderProgram();
            if (shaderProgram == nullptr)
            {
                // Not use custom shader.
                shaderProgram = _defaultShaderProgram;
                useCustomShaderProgram = false;
            }
#if OLDSHADER_USE
            this->setGLProgram(shaderProgram);
#else           
            if (_shaderProgramState == 0)
            {
                _shaderProgramState = cocos2d::GLProgramState::create(shaderProgram);
                _shaderProgramState->retain();　//この行を追加
            }
            this->setGLProgramState(_shaderProgramState);
#endif          
            _useCustomShaderProgram = useCustomShaderProgram;
        }
        else
        {
            this->setGLProgram(_defaultShaderProgram);
            _useCustomShaderProgram = false;
                       //_shaderProgramStateに0を入れていた部分の削除
        }
    }
}

この様に変更する事でエラーが発生する様な事はなくなった、という事でよろしいでしょうか？

次にシェーダーについてですが、使用している画像のフォーマットをお教えいただけますでしょうか。これは例えば32bitPNGであれば、読み込んだテクスチャはプリマルチアルファとして管理されておりますので、透明度はカラー値に乗算されます。なので完全透明＝黒（R=0、G=0、B=0）となる様にシェーダーを記述しましたが、カラー値の黒が黒として表示される原因から考える必要がある様に思います。

またこちらのシェーダー内での分岐

    if((u_partblend==1)||(u_partblend==3))                            \n\
    {                                                                 \n\
        pixel.rgb *= pixel.a;                                         \n\
    }                                                                 \n\

はパーツのブレンド方法が乗算または減算の場合、かつカラーブレンドを使用した場合（カラーブレンドを使用した場合にこのシェーダーを使用する、パーツのブレンド方法のみの変更ではシェーダーは使用されない）に通る事になり限定的に行われる処理となります。黒く表示されるパーツのブレンド方法とカラーブレンドの組み合わせはどのようになっていますでしょうか？

シェーダーの分岐による負荷については把握しております。今後の参考にさせていただきます。よろしくお願いいたします。

SpriteStudio / SS5PlayerForCocos2d-x

v1.3.2 oldshaderのではなくnewshaderの方でretainし忘れ #54

if OLDSHADER_USE

else

endif