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

Instanced mesh example #2460

Merged
merged 4 commits into from
Apr 19, 2023
Merged

Instanced mesh example #2460

merged 4 commits into from
Apr 19, 2023

Conversation

brisvag
Copy link
Collaborator

@brisvag brisvag commented Mar 9, 2023

A surprisingly simple visual for arbitrarily oriented and colored meshes using instanced rendering.

I started on this as a way to have "true 3D" markers for solving #2453, but adding arbitrary orientations turned out to be relatively easy, and that's useful fo r stuff I'm working on :P

Basically, replace the mesh with a simple icosphere and you get your real pysically sized markers that work in both 2D and 3D. Notice how I applied a scaling transform to the visual in the examples, and the dinosaurs are being stretched correctly.

instanced_mesh.mp4

One thing that's missing (and likely hard to do) compared to the markers is the edge. As far as I know, the only 2 reasonable ways to do it are geometry shaders and multipass rendering?

Comment on lines 24 to 26
attribute vec3 transform_x;
attribute vec3 transform_y;
attribute vec3 transform_z;
Copy link
Contributor

Choose a reason for hiding this comment

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

are these vectors the column vectors of the matrix or the row vectors? would add a comment to the shader making this clear

Copy link
Contributor

Choose a reason for hiding this comment

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

i.e. are the values those that are required to compute x'/y'/z' (rows of matrix) or are they the new basis vectors (columns of matrix)?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Columns vectors (new basis vectors). The mat constructor takes inputs column by column.
Will add comment!

examples/scene/instanced_quad_visual.py Outdated Show resolved Hide resolved
@alisterburt
Copy link
Contributor

❤️ ❤️ ❤️

@brisvag
Copy link
Collaborator Author

brisvag commented Apr 18, 2023

@djhoese this would be nice to get in, but I don't know how to intepret that test failure. Something about discontinuous data being copied? How can I solve it?

@djhoese
Copy link
Member

djhoese commented Apr 18, 2023

Hm and you don't see this warning when you run it locally? Same version of python and numpy? Any idea if the warning is in vispy?

Comment on lines 72 to 74
self.transforms_x = gloo.VertexBuffer(transforms[..., 0], divisor=1)
self.transforms_y = gloo.VertexBuffer(transforms[..., 1], divisor=1)
self.transforms_z = gloo.VertexBuffer(transforms[..., 2], divisor=1)
Copy link
Member

Choose a reason for hiding this comment

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

My guess is this is the problem regarding the dis-contiguous data. By doing the indexing there you're now causing striding by 3 (for the 3 elements X/Y/Z) and the VertexBuffer underneath is complaining that the data isn't contiguous and has to be copied.

vispy/vispy/gloo/buffer.py

Lines 438 to 442 in ded8eb9

if c in [2, 3, 4]:
if not data.flags['C_CONTIGUOUS']:
logger.warning('Copying discontiguous data for struct '
'dtype:\n%s' % _last_stack_str())
data = data.copy()

Copy link
Member

Choose a reason for hiding this comment

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

So I think if you did a .copy() here when you do the [..., 0] style indexing then you'd at least avoid the warning. If you could find a way to structure the array as (3, M, N) then do indexing [0] then you'd also avoid the copy.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

True, it also happens locally. I think copy is fine, as this si just an example :)

@djhoese
Copy link
Member

djhoese commented Apr 18, 2023

Ok, passing now. Anything else before I do one last skim of the code and merge?

@brisvag
Copy link
Collaborator Author

brisvag commented Apr 18, 2023

Nope, I think it's good to go :)

@djhoese
Copy link
Member

djhoese commented Apr 19, 2023

I downloaded the generated .zip of the website and the image looks good. Thanks. Merging...

@djhoese djhoese merged commit 44afcb6 into vispy:main Apr 19, 2023
15 checks passed
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.

None yet

3 participants