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

Reset VertexAttribDivisor even if None #2583

Merged
merged 3 commits into from Apr 17, 2024
Merged

Conversation

aganders3
Copy link
Contributor

Fixes #2581 - see additional discussion in that issue.

My hypothesis here is that the attr_handle is reused between the text and instanced mesh visuals, but the divisor is not properly (re)set. The single mesh does not interfere because it does not use more than handle 0, which is okay because the divisor is the same for this attribute (a_position).

Here is some debug output from GlirProgram._pre_draw showing the attribute name, handle, and divisor value. You can see the overlapping values from the Text and InstancedMesh visuals.

drawing <Text at 0x1340d6dd0>
a_color 4 None
a_position 1 None
a_texcoord 0 None
a_rotation 2 None
a_pos 3 None

drawing <Mesh at 0x1340e5310>
a_position 0 None

drawing <InstancedMesh at 0x1340e5410>
a_position 0 None
u_base_color 1 1
transform_x 5 1
transform_y 4 1
transform_z 2 1
shift 3 1

cc @brisvag @rzmearns

Copy link
Member

@djhoese djhoese left a comment

Choose a reason for hiding this comment

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

I think I understand the idea. Looks good enough to me.

Copy link
Collaborator

@brisvag brisvag left a comment

Choose a reason for hiding this comment

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

Basically: OpenGL will reuse stuff like the attribute handle between visuals, and if you don't explicitly set everything you need to the right value, you end up with whatever was set elsewhere before.
LGTM!

@aganders3
Copy link
Contributor Author

Shoot - the test failure looks legit. I don't know much about osmesa but will take a look today.

@aganders3
Copy link
Contributor Author

aganders3 commented Apr 16, 2024

Okay I think it's just a backend support issue, so I think e27e089 will at least fix the tests.

I'm not aware of a standard way of checking for support in glir.py, so I'm open to suggestion here. Should it just raise an exception if any attributes have a divisor and it's not available? If instances > 1 it will raise AttributeError: module 'vispy.gloo.gl' has no attribute 'glDrawArraysInstanced' just after this anyway.

I now understand why if divisor is not None was there to begin with :).

@brisvag
Copy link
Collaborator

brisvag commented Apr 16, 2024

This looks fine to me, at least it's a lot clearer what went wrong!

@brisvag brisvag merged commit 9b0f589 into vispy:main Apr 17, 2024
15 checks passed
@aganders3 aganders3 deleted the fix-2581 branch April 17, 2024 14:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Having problems with TextVisuals and InstancedMesh
3 participants