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

Shader applied to Spine node not working in official v. 4.0 #234

Closed
etsek opened this issue Oct 15, 2020 · 21 comments · Fixed by #236 or #246
Closed

Shader applied to Spine node not working in official v. 4.0 #234

etsek opened this issue Oct 15, 2020 · 21 comments · Fixed by #236 or #246
Milestone

Comments

@etsek
Copy link

etsek commented Oct 15, 2020

  • cocos2d-x version: cocos2d-x-4.0
  • devices test on: iPhone, Win32
  • developing environments
    • NDK version:
    • Xcode version: 12.0, 11.7
    • VS version: 2019
    • browser type and version:

Steps to Reproduce:

  1. Add a predefined shader to any skeletonNode in cpp-tests. For instance in SpineTest.cpp, add the following code in CoinExample::init() right before the call to scheduleUpdate(). While the sprite is properly rendered in grayscale, the Spine node in unaffected. In versions 3.17.x this functionality worked as expected.

auto program = backend::Device::getInstance()->newProgram(
positionTextureColor_vert,
grayScale_frag);
auto programState = new backend::ProgramState(program);
skeletonNode->setProgramState(programState);

auto sprite = Sprite::create("Images/grossini.png");
sprite->setPosition( Vec2(_contentSize.width/2 +100, _contentSize.height/2) );
sprite->setProgramState(programState);
addChild(sprite);

Shaders applied to Spine nodes do now work anymore in official v. 4.0, this is a known issue documented in https://discuss.cocos2d-x.org/t/cocos2d-4-0-create-shader-for-spine-animation/51480.

A relevant PR has been created at cocos2d/cocos2d-x#20584

@halx99
Copy link
Collaborator

halx99 commented Oct 15, 2020

Good news

@halx99
Copy link
Collaborator

halx99 commented Oct 15, 2020

Does engine-x works?
Does the official v4 caused by Custom Shader?

@etsek
Copy link
Author

etsek commented Oct 15, 2020

Engine-x behavior is identical to official v4, i.e. shaders are not working when applied to Spine nodes.
I'm not sure the issue is caused exclusively by custom shaders, in the above code snippet I'm using a predefined shader.

@halx99
Copy link
Collaborator

halx99 commented Oct 15, 2020

ok, I will check later

@halx99
Copy link
Collaborator

halx99 commented Oct 16, 2020

I have checked the official spine runtimes v4 implementation, it's not support custom programstate currently

@halx99 halx99 added this to the 1.0 milestone Oct 16, 2020
@etsek
Copy link
Author

etsek commented Oct 16, 2020

I think there are two issues here, one has to do with the Spine runtime currently not supporting custom program state (for which Spine developers need to merge the PR fix) and the other with the fact that batching is altogether disabled for custom program types in the Cocos2d-x engine code (CCTrianglesCommand.cpp).

@halx99 halx99 linked a pull request Oct 16, 2020 that will close this issue
3 tasks
@halx99
Copy link
Collaborator

halx99 commented Oct 16, 2020

Yes, so I create a PR to re-enable custom program state batch draw

@halx99
Copy link
Collaborator

halx99 commented Oct 16, 2020

The PR may only improve batch draw and all program was compiled just in use time, for spine, you may needs send issue to official spine-runtimes: https://github.com/EsotericSoftware/spine-runtimes

@etsek
Copy link
Author

etsek commented Oct 16, 2020

There's already a PR for that issue: EsotericSoftware/spine-runtimes#1763

@halx99 halx99 reopened this Oct 16, 2020
@halx99
Copy link
Collaborator

halx99 commented Oct 16, 2020

spine-custom-shader.zip
with the latest master of engine-x, the custom shader should works, see:
image

The test case is set spine animation HSV: https://github.com/c4games/engine-x/blob/master/tests/cpp-tests/Classes/SpineTest/SpineTest.cpp#L337

@halx99
Copy link
Collaborator

halx99 commented Oct 16, 2020

And I also send a PR to spine-runtimes: https://github.com/EsotericSoftware/spine-runtimes/pull/1787/files

@etsek
Copy link
Author

etsek commented Oct 16, 2020

Thanks! @halx99, I pulled the latest from master and applied your Spine runtime patch and now shaders work beautifully on Spine nodes. I think this issue is considered closed unless you want to wait for the Spine team to merge your PR.

@rh101
Copy link
Contributor

rh101 commented Oct 22, 2020

Out of curiosity, have you tested displaying more than one of the same spine animation on the screen, but each spine animation uses a different custom shader or different shader uniform values? For instance, with the HSV test case, display multiple spine animations, each having the custom HSV shader with different HSV value set for each one. Does it work?

@halx99
Copy link
Collaborator

halx99 commented Oct 22, 2020

I will check later

@halx99
Copy link
Collaborator

halx99 commented Oct 22, 2020

@rh101 I have checked, it's display same because two spine animation merged to 1 draw batch, so needs to implement custom shader like your's setBatchId

@rh101
Copy link
Contributor

rh101 commented Oct 22, 2020

Fair enough. I was hoping there was a better way to handle the batching without using the IDs, but I can't think of any other way at the moment.

@halx99
Copy link
Collaborator

halx99 commented Oct 23, 2020

@rh101 Hi, I use uniformID to diff final matID, see: #246

@halx99 halx99 linked a pull request Oct 23, 2020 that will close this issue
@rh101
Copy link
Contributor

rh101 commented Oct 23, 2020

Wouldn't XXH32 be processing intensive depending on the data contained in the vertex and uniform buffers?

void ProgramState::updateUniformID()
{
#ifdef CC_USE_METAL
    XXH32_reset(_uniformHashState, 0);
    XXH32_update(_uniformHashState, _vertexUniformBuffer, _vertexUniformBufferSize);
    XXH32_update(_uniformHashState, _fragmentUniformBuffer, _fragmentUniformBufferSize);
    _uniformID = XXH32_digest(_uniformHashState);
#else
    _uniformID = XXH32(_vertexUniformBuffer, _vertexUniformBufferSize, 0);
#endif
#endif
}
}

I've never used it, so just wondering if it would be slow the larger the buffers are.

@halx99
Copy link
Collaborator

halx99 commented Oct 23, 2020

I think the uniform buffer size depends on shader complexity and usually it small than 256 bytes, the XXH32 so fast could be ignore, I checked the full FBO test case, the fps still fast than official v4 in my machine

  • adxe (30fps)
  • official-v4(27fps)

@halx99
Copy link
Collaborator

halx99 commented Oct 23, 2020

We also can provide setUniformID manually for user to avoid calculate uniformID with uniform buffer automatically, what do you think about?

@rh101
Copy link
Contributor

rh101 commented Oct 23, 2020

We also can provide setUniformID manually for user to avoid calculate uniformID with uniform buffer automatically, what do you think about?

That would be a good idea, in case the performance is affected by the XXH32 calculation.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants