-
Notifications
You must be signed in to change notification settings - Fork 2k
Fix duplicate rendering issue caused by failure to restore ibCount during batch merging #18721
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
Conversation
…ring batch merging
Code Size Check Report
Interface Check ReportThis pull request does not change any public interfaces ! |
@@ -429,10 +428,10 @@ CC_FORCE_INLINE void Batcher2d::handleMiddlewareDraw(RenderEntity* entity, Rende | |||
// check for merge draw | |||
auto enableBatch = !entity->getUseLocal(); | |||
if (enableBatch && _currTexture == texture && _currMeshBuffer == meshBuffer && !_currEntity->getUseLocal() && material->getHash() == _currMaterial->getHash() && drawInfo->getIndexOffset() == _currDrawInfo->getIndexOffset() + _currDrawInfo->getIbCount() && layer == _currLayer) { | |||
auto ibCount = _currDrawInfo->getIbCount(); | |||
_currDrawInfo->setIbCount(ibCount + drawInfo->getIbCount()); | |||
_currMiddlewareIbCount += drawInfo->getIbCount(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What will happen if handleMiddlewareDraw
is called multiple times with the same object?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It will cause duplicate rendering because the ibCount inside _currDrawInfo has been changed, but there is no place to revert the setting
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think duplicated rendering is acceptable. The program is correct, right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This will cause the shaded portion to be incorrect. #18704
@cocos-robot run test cases |
@qiuguohua, Please check the result of
Task Details
|
@qiuguohua, Please check the result of
Task Details |
Re: #
Changelog
#18704
Continuous Integration
This pull request:
Compatibility Check
This pull request: