Render issue on cocos2d-x-3.17

Render issue on cocos2d-x-3.17
0
#1

After upgrading to cocos-2d-x 3.17 from 3.14.1 graphics issue (flickering artifacts) was encountered on some android devices (LG G Pad 8.3 OS Android 5.0).
Probably this issue appeared on cocos-2d-x-3.15 or cocos-2d-x-3.16.

Screenshots attached

#2

It seems shader issue.

#3

I got the LG G Pad 8.3 with Android 4.4.2 and I was able to reproduce this bug. I figured out that bug appears with DrawNode usage.

#4

On LG G Pad 8.3 Configuration::getInstance()->supportsShareableVAO() returns true
Disabling using of VAO in DrawNode fixes this issue.

#5

Could you create issue with simple test case how to reproduce it?

#6

I tested the “Node: Draw” test cases. All tests worked fine on LG G Pad 8.3. I will try to reproduce this issue in cpp-tests project.

#7

I read cocos2d-x code and found some issues.

  1. CCDrawNode.cpp:315 (DrawNode::onDraw(), DrawNode::onDrawGLLine(), DrawNode::onDrawGLPoint()):
    This code updates array buffer IN ANOTHER VAO, because it executing before GL::bindVAO(_vao);
if (_dirty)
    {
        glBindBuffer(GL_ARRAY_BUFFER, _vbo);
        glBufferData(GL_ARRAY_BUFFER, sizeof(V2F_C4B_T2F)*_bufferCapacity, _buffer, GL_STREAM_DRAW);
        
        _dirty = false;
    }

When I moved this code after GL::bindVAO(_vao); flickering issues on LG G Pad 8.3 OS gone away!

  1. CCVertexIndxBuffer.cpp:225 (void IndexBuffer::recreateVBO() const)
    This is index buffer (element array buffer), but GL_ARRAY_BUFFER constant used instead GL_ELEMENT_ARRAY_BUFFER

  2. CCMaterial.cpp:175 (bool Material::parseTechnique(Properties* techniqueProperties))

else if (strcmp(name, "renderState") == 0)
       {
         //why parseRenderState(this, space), not parseRenderState(technique, space);?
           parseRenderState(this, space);
       }

Technique pass is RenderState subclass and can have own renderState block

#8

Unfortunately I don’t know enough about the workings of OpenGL, so I can’t comment, but in case they are bugs in the code, then would you please create a separate issue in github for each of those items?

#9

@zhangxm seems that this bug will interest you.

#10

@direct3d9

  1. i am not sure if it is a problem, because the buffer is referred by VAO, so i think it will be detected by the correct VAO. As it is described in the wiki
Note that VAOs do not copy, freeze or store the contents of the referenced buffers - if you change any of the data in the buffers referenced by an existing VAO, those changes will be seen by users of the VAO.
  1. Yep, i think you are right. Good catch. Will you please send a pull request to fix it?
  2. I agree with you. I don’t know why it does like this.
2 Likes
#11

After reading that line, I am interpreting it differently, specifically the part about:

...if you change any of the data in the buffers referenced by an existing VAO...

In the current implementation, the buffer is created prior to the binding of the VAO, so it’s not yet referenced by the VAO:

if (_dirty)
{
    glBindBuffer(GL_ARRAY_BUFFER, _vbo);
    glBufferData(GL_ARRAY_BUFFER, sizeof(V2F_C4B_T2F) * _bufferCapacity, _buffer, GL_STREAM_DRAW);

    _dirty = false;
}

if (Configuration::getInstance()->supportsShareableVAO())
{
    GL::bindVAO(_vao);
}
else
{
    GL::enableVertexAttribs(GL::VERTEX_ATTRIB_FLAG_POS_COLOR_TEX);

    glBindBuffer(GL_ARRAY_BUFFER, _vbo);
    // vertex
    glVertexAttribPointer(GLProgram::VERTEX_ATTRIB_POSITION, 2, GL_FLOAT, GL_FALSE, sizeof(V2F_C4B_T2F), (GLvoid *)offsetof(V2F_C4B_T2F, vertices));
    // color
    glVertexAttribPointer(GLProgram::VERTEX_ATTRIB_COLOR, 4, GL_UNSIGNED_BYTE, GL_TRUE, sizeof(V2F_C4B_T2F), (GLvoid *)offsetof(V2F_C4B_T2F, colors));
    // texcoord
    glVertexAttribPointer(GLProgram::VERTEX_ATTRIB_TEX_COORD, 2, GL_FLOAT, GL_FALSE, sizeof(V2F_C4B_T2F), (GLvoid *)offsetof(V2F_C4B_T2F, texCoords));
}

Now, as @direct3d9 mentioned, changing the order in which this occurs is actually having an impact on the output, since it fixes the flickering issue for him. That can only mean that the order is significant, so it matters. OpenGL sample code relating to VAO always has the exact same order, where glBindVertexArray() is always called before glBindBuffer(), so perhaps there is something to this.

This would be the corrected order in DrawNode::onDraw():

if (Configuration::getInstance()->supportsShareableVAO())
{
    GL::bindVAO(_vao);
    if (_dirty)
    {
        glBindBuffer(GL_ARRAY_BUFFER, _vbo);
        glBufferData(GL_ARRAY_BUFFER, sizeof(V2F_C4B_T2F) * _bufferCapacity, _buffer, GL_STREAM_DRAW);

        _dirty = false;
    }
}
else
{
    if (_dirty)
    {
        glBindBuffer(GL_ARRAY_BUFFER, _vbo);
        glBufferData(GL_ARRAY_BUFFER, sizeof(V2F_C4B_T2F) * _bufferCapacity, _buffer, GL_STREAM_DRAW);

        _dirty = false;
    }

    GL::enableVertexAttribs(GL::VERTEX_ATTRIB_FLAG_POS_COLOR_TEX);

    glBindBuffer(GL_ARRAY_BUFFER, _vbo);
    // vertex
    glVertexAttribPointer(GLProgram::VERTEX_ATTRIB_POSITION, 2, GL_FLOAT, GL_FALSE, sizeof(V2F_C4B_T2F), (GLvoid *)offsetof(V2F_C4B_T2F, vertices));
    // color
    glVertexAttribPointer(GLProgram::VERTEX_ATTRIB_COLOR, 4, GL_UNSIGNED_BYTE, GL_TRUE, sizeof(V2F_C4B_T2F), (GLvoid *)offsetof(V2F_C4B_T2F, colors));
    // texcoord
    glVertexAttribPointer(GLProgram::VERTEX_ATTRIB_TEX_COORD, 2, GL_FLOAT, GL_FALSE, sizeof(V2F_C4B_T2F), (GLvoid *)offsetof(V2F_C4B_T2F, texCoords));
}
#12

This flickering articfacts are presents ONLY if shareable VAO is enabled. Changing buffer object contents BEFORE calling of GL::bindVAO(_vao) may changes buffers in ANOTHER VAO, that bound earlier.

#13

buffer is bound to VAO in DrawNode::setupBuffer(). I think the flickering is because some opengl driver doesn’t implement correctly. And move the codes after binding VAO will not break other things, so i think it is ok to move updating buffer data after binding VAO.

@direct3d9 i will send PR to fix them if you will not send one.

2 Likes
#14

buffer initially bound to VAO in DrawNode::setupBuffer(), but rebound into another VAO in DrawNode::onDraw(), DrawNode:: onDrawGLLine() and DrawNode:: onDrawGLPoint(), because another VAO may be current before DrawNode::onDraw() execution.

Please send 3 PRs:
1 - move buffer updating code after GL::bindVAO(_vao)

2 - Fix buffer type in CCVertexIndxBuffer.cpp:225 (void IndexBuffer::recreateVBO() const)
This is index buffer (element array buffer), but GL_ARRAY_BUFFER constant used instead GL_ELEMENT_ARRAY_BUFFER. (see above)

3 - Fix parseRenderState() for technique (see above)

#15

VAO will bind VBO when glVertexAtrribPointer is invoked, only bind VBO(except GL_ELEMENT_ARRAY_BUFFER) will not change VAO’s state. You can refer to the url for detail explanation. If it has problem, then why it only has problem on LG Pad? And how did you move the codes? We should update VBO in both using VAO or not.

I will send PRs for other 2 issues.

#16

Fixed in PR.

Fixed in PR.

#17

These fixes, (and any fixes in general) are done in the opengl version or the metal version? or metal will get them when its fully merged?

#18

Some fixes will be sent to metal support branch, some are not, because they do not exist in metal branch.