Open alecjacobson opened 2 years ago
From the comments, looks like this is known issue
https://github.com/nmwsharp/polyscope/blob/3b28e0243194b6633de02617100f6f139af65a56/src/surface_mesh.cpp#L921
calling refresh()
seems to trigger lots of unnecessary things like recompiling shaders etc.
Yes, for sure. These timings match my experience as well.
For this immediate issue, the solution is just a small reorganization of the code in SurfaceMesh
's prepareProgram()
and preparePickProgram()
functions so that we can repopulate a buffer without recompiling shaders or making other spurious openGL calls. (Similar fixes are needed in the other structures, I believe)
More deeply, Polyscope has a bunch of small design flaws which stop it from being ideal for animation code right now (the main cause for which is that I personally don't write much animation code!). In particular:
GLEngine
is architected such that openGL data buffers are immutably tied to the program. You can't re-allocate or swap a buffer without recompiling the shader program, and every object in Polyscope has a distinct shader program, even if the shader is the same. This is totally unnecessary and also leads to lots of unnecessary data duplication in GPU buffers. We should rework the GLEngine
to decouple buffers from programs, and let programs swap on to different buffers (how openGL is actually meant to work...).userCallback
means that Polyscope now 'owns' the program flow, and this is counter to the philosophy that Polyscope should be uninvasive, and should not take ownership of the program. We should offer another option, where users can call a polyscope::frameTick()
function that executes one iteration of the main loop, which would make it easy to drop Polyscope in to existing animation code.getObjectClickLocation()
to make this easier.I think these can all be implemented transparently behind the scenes without any breaking changes. It would be great to bundle a bunch of these fixes in to an "animation" update or something.
I would love to see this addressed. I haven't tried using Polyscope for animation myself, but if what Alec says is true it would be unusable for my needs. I could ask a student to work on this and create a pull request, but I wanted to check if there isn't any progress on this already from your end so we wouldn't be duplicating work.
Was this issue fixed in #206? It seems like there was a total rewrite of the renderer, and it mentions cached shaders.
Hi! Please give it a try :) there have been multiple improvements merged which dramatically speed this up since Alec's original post.
In particular, we now persist GPU buffers that don't actually change, rather than needlessly transferring extra data. Also, I fixed a bunch of places where shader programs were getting unnecessarily recompiled (although there may be more hiding). And as @lasagnaphil noticed, the shader programs are now cached anyway, massively reducing repeated program creation cost when it does happen.
Looking forward to the support of interaction with the scene. I think it will make Polyscope suitable in more applications.
I'm wondering if I'm using the
updateVertexPositions
anduserCallback
correctly. Currently I'm getting around260 ms/f (4 FPS)
when I update the positions of a mesh with 262,144 faces inside the draw loop callback.If I run a similar animation in the libigl viewer I'm getting
19.9 ms/f (50 FPS)
. In that scenario I'm usingset_vertices
andcallback_pre_draw
.Here's a video screenshot on my intel macbook pro running catalina:
https://user-images.githubusercontent.com/2241689/147182765-78b15f28-a1ce-4fc6-abf0-f6b44af2728e.mov
( Turning off the polyscope groundplane doesn't seem to change anything. I'm guessing the slowdown is happening in
updateVertexPositions
)