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

GL ELEMENT_ARRAY_BUFFER_BINDING issue not working for VAO == 0 #3194

Closed
wants to merge 1 commit into from

Conversation

Slaw6820
Copy link

For array == 0 previous value should be restored instead of NULL assignment.

@Zorro666 Zorro666 self-requested a review January 2, 2024 12:19
Copy link
Collaborator

@Zorro666 Zorro666 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi

Thank you for the proposed fix.

Can you explain what problem you discovered and if possible share a program that recreates the problem?

Looking at the GL spec the implemented behaviour looks correct, an array value of zero should break the current VAO binding i.e.

glBindVertexArray binds the vertex array object with name array. array is the name of a vertex array object previously returned from a call to glGenVertexArrays, or zero to break the existing vertex array object binding.

@Slaw6820
Copy link
Author

Slaw6820 commented Jan 2, 2024

It is an issue I have encountered some time ago while working on app I cannot share. I haven't looked into spec, but it seems like VAO == 0 had some data bound. Without this fix, some geometry was not rendered at all.

@Zorro666
Copy link
Collaborator

Zorro666 commented Jan 2, 2024

Thank you for explaining your use case. Not a problem about not being able to share the application.
Do you happen to know what API (GL or GLES) it was using and which version of the API.
The GL specification I mentioned is for GL Core and the behaviour of glBindVertexArray is different in other versions of GL

@Slaw6820
Copy link
Author

Slaw6820 commented Jan 2, 2024

I'm almost sure it was GL 4.6 Compatibility

@Zorro666
Copy link
Collaborator

Zorro666 commented Jan 3, 2024

GL Compatibility makes sense : the behaviour of the implementation needs to be different when capturing GL Core or not.

GL Core behaves as the code is currently and for GLES & Compatibility it needs to be changed to something like you have in this PR.

@Slaw6820
Copy link
Author

Slaw6820 commented Jan 3, 2024

Sure. So, does the fix fit here? Renderdoc supports GL core mainly, right?
If yes, how would you like to see this fix to be implemented?

@Zorro666
Copy link
Collaborator

Zorro666 commented Jan 4, 2024

I was thinking about this and this would be easier if there was an issue i.e. bug report which would give a reproduction case to work through. I would say we need to keep the previous and the new code but with an if test. The if test should only use the old code on a core profile (or the if test can be inverted and use the new code for GLES and Compatibility)

I think it would be useful to have a GL test program that uses this feature and the GL test program can run in Core or Compatibility mode to show the two different code paths, ideally there would also be a GLES test program.

I don't think there is a major rush on this change (as there is no bug report to work from).
Let me look around for some GL test programs which could be used to verify the change.
I will also look for the best way to do the if check.

@Zorro666
Copy link
Collaborator

Zorro666 commented Jan 8, 2024

I have been investigating to find a way to do this without an if test, which should be possible.
The problem I am having is not having a reproduction case for the particular problem that you discovered to validate any change and then not being able to understand what is needed to be fixed. The correct fix might be this PR or it might be a smaller version or the fix might be in a different piece of code.

I have been using the gl_vao_0 test program, however so far I have not been able to create any problems when calling glBindVertexArray(0)

If you were able to start from that test program (or a different reproduction project) to create a capture that does not replay correctly then we can move forward with the PR to identify and understand the problem and be able to verify the required fix.

@Zorro666
Copy link
Collaborator

Zorro666 commented Feb 8, 2024

Hi

Closing the PR as it has been a month without any replies.
There looks to be a valid problem here but without a reproduction case then the correct code fix can't be identified.

@Zorro666 Zorro666 closed this Feb 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants