Fixed an issue where the shader would come off when run setTexture.

[HirokiIshimori]

Summary

Fix issue #20600 , When I set a shader in Sprite and execute setTexture, the shader comes off.

Event

1. After doing setProgramState as below ...

sprite->setProgramState(programState);

2. Try calling setTexture.

sprite->setTexture(filePath);

Then the shader will not run.

Why I submitted a Pull Request.

Currently, it can be dealt with by calling setProgramState immediately after calling setTexture as shown below.

sprite->setTexture(filePath);
sprite->setProgramState(mProgramState);

But developers need to explicitly keep ProgramState, and I think it's a redundant implementation.
So I submitted a Pull Request.

[halx99]

I think, check does custom shader is better

[HirokiIshimori]

Does a custom shader solve this problem?

[halx99]

Does a custom shader solve this problem?

Please refer to this commit: axmolengine/axmol@27bdca3

[HirokiIshimori]

Thank you very much. I pushed a fix commit.
6e3d316

[rh101]

The added condition _programState->getProgram()->getProgramType() == backend::ProgramType::POSITION_TEXTURE_COLOR has no effect if it's the same, but also it won't correctly set the ETC1 shader if the new texture is of format ETC1:

    if (_programState == nullptr || _programState->getProgram()->getProgramType() == backend::ProgramType::POSITION_TEXTURE_COLOR) {
        const auto isETC1 = texture && texture->getAlphaTextureName();
        setProgramState((isETC1) ? backend::ProgramType::ETC1 : backend::ProgramType::POSITION_TEXTURE_COLOR);
    }

because setProgramState checks if it's the same type, and returns if it is, so it changes nothing:

void Sprite::setProgramState(unsigned type)
{
    if(_programState != nullptr &&
       _programState->getProgram()->getProgramType() == type)
        return;
    
    auto* program = backend::Program::getBuiltinProgram(type);
    auto programState = new (std::nothrow) backend::ProgramState(program);
    setProgramState(programState);
    CC_SAFE_RELEASE_NULL(programState);
}

This would probably work better, and what @halx99 may have meant:

 if (_programState == nullptr || _programState->getProgram()->getProgramType() < backend::ProgramType::CUSTOM_PROGRAM) {
        const auto isETC1 = texture && texture->getAlphaTextureName();
        setProgramState((isETC1) ? backend::ProgramType::ETC1 : backend::ProgramType::POSITION_TEXTURE_COLOR);
    }

It would then correctly set the ETC1 or POSITION_TEXTURE_COLOR shader depending on the passed in texture, and it won't affect sprites with custom shaders.

Otherwise, you can explicitly check for the ETC1 and POSITION_TEXTURE_COLOR shaders, in case a non-custom shader is being used on the sprite that isn't one of them:

if (_programState == nullptr || _programState->getProgram()->getProgramType() == backend::ProgramType::POSITION_TEXTURE_COLOR || 
    _programState->getProgram()->getProgramType() == backend::ProgramType::ETC1) {
    const auto isETC1 = texture && texture->getAlphaTextureName();
    setProgramState((isETC1) ? backend::ProgramType::ETC1 : backend::ProgramType::POSITION_TEXTURE_COLOR);
}

[halx99]

Yes, @rh101 's code is better

[HirokiIshimori]

Thank you for your advice.

Is it okay with such a fix?

Suggested change

	if (_programState == nullptr || _programState->getProgram()->getProgramType() == backend::ProgramType::POSITION_TEXTURE_COLOR) {
	const auto isETC1 = texture && texture->getAlphaTextureName();
	setProgramState((isETC1) ? backend::ProgramType::ETC1 : backend::ProgramType::POSITION_TEXTURE_COLOR);
	}
	if (_programState == nullptr || _programState->getProgram()->getProgramType() == backend::ProgramType::POSITION_TEXTURE_COLOR ||
	_programState->getProgram()->getProgramType() == backend::ProgramType::ETC1) {
	const auto isETC1 = texture && texture->getAlphaTextureName();
	setProgramState((isETC1) ? backend::ProgramType::ETC1 : backend::ProgramType::POSITION_TEXTURE_COLOR);
	}

[rh101]

Yes that would work fine, since it covers all cases.

[HirokiIshimori]

@halx99 @rh101 Thank you.