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

Graphic Buffers Update #714

Closed
wants to merge 10 commits into from
Closed

Conversation

bremco
Copy link
Contributor

@bremco bremco commented Jul 26, 2021

The highly interdependent Issues this fixes:

  • The Vertex Array Object (VAO) for a render wasn't actually being (re)used, but regenerated each time. Now it's cached.
  • While (otherwise) vertex- and index-buffers where already cached for normal rendering, the index-buffer for a mesh was recreated each time only a part of the mesh (a range) needed to be drawn (for example, Layer and Simulation view in Cura). (NB: This was due to a workaround on our side, caused by an only partial implementation of a specific bit of the OpenGL API by the Python interface we're using.) This is now fixed.

Complications:

  • In order to show 'ranged'/partial meshes, implementing programs (like Cura) must now have shaders that take a draw-range uniform, which can be compared against the newly introduced 'vertex-index' attribute. (As we can't use the OpenGL API due to partial implementation, and we can't use the gl_primitiveID built-in due to support of legacy shaders.) See also Graphic Buffers Update Cura#10188, the front-end implementation of this PR.

See also this related discussion #664 earlier.

I'm not sure why we're using VAO's when so much seems to be recreated each render, but this way at least the VAO itself wouldn't also be recreated even if the objects aren't.
Still TODO: Once the RenderBatch isn't recreated each time a slightly different render is made, VertexArrayObjects will actually be used, instead of just pretended to be used.
Also still TODO: any model will render as if the base color is matte dark grey. All the other rendering (like for 'lay-flat by face' or the blue model outline or the tool-arrows and such) and all the rendering _given that the cube is matte grey_ (like 'the error polka dot pattern' and the 'below the buildplate inversion') will still work properly. So everything works _except_ for the color and light-highlights. Why?
VAO includes shader uniforms, which where needed. So now, instead of cacheing per mesh alone, cache per mesh _per shader_. Currently the doesn't work 100% yet if different parts of a mesh are rendered with different settings, such as in the simulation view. Also need to check if the X-Ray view was supposed to be opaque or if that's another thing that doesn't work 100% yet (is usable though just not as pretty as remembered).
Instead of re-uploading the mesh each time the range changes, handle the range in the shaders with the new draw-range parameters.
@nallath
Copy link
Member

nallath commented Jul 26, 2021

Do you also have before / after profiling? I'm curious what the gains are :)

@bremco
Copy link
Contributor Author

bremco commented Jul 26, 2021

@nallath Right, was one of the things I meant to do as well :-)
Not sure I can get that done today though.

(P.S. Even if there are no true gains I'd argue this is more correct, but that's harder to justify from a 'risk' perspective. Also technically upping the SDK with my shader shenanigans.)

UM/View/RenderBatch.py Outdated Show resolved Hide resolved
UM/View/RenderBatch.py Outdated Show resolved Hide resolved
UM/View/RenderBatch.py Outdated Show resolved Hide resolved
@bremco
Copy link
Contributor Author

bremco commented Jul 26, 2021

Lesson for next time: Run the tests after I've merged with main as well.

bremco and others added 2 commits July 26, 2021 15:46
Typing edition.

Co-authored-by: Jaime van Kessel <[email protected]>
Seems to have fixed the test as well. Not sure if it was ever a valid test if it can't create a VAO there.
@bremco
Copy link
Contributor Author

bremco commented Jul 26, 2021

(... actually I don't think I did run the tests. I was confused with last week, when I had to set them up again on my work lappy.)

In any case, please try again, should pass now!

UM/View/RenderBatch.py Outdated Show resolved Hide resolved
Fixes type laziness, types should work harder!
@bremco
Copy link
Contributor Author

bremco commented Jul 26, 2021

I still didn't do a proper benchmark, but playing around a bit with some printouts of the average time of the render of the SimulationView (where I expect most of the time-win will be) it seems that the newer code is about 20% faster.

UM/View/RenderBatch.py Outdated Show resolved Hide resolved
@bremco
Copy link
Contributor Author

bremco commented Aug 2, 2021

When trying to benchmark this, I noticed lineMeshVertexCount (the entire layer version, not the polygon one), which I now use in SimulationPass, is quite a slow function compared to what was there before.

I need to set up some caching or something, or this PR will feel incomplete (even though it's already a bit faster). A lot of SimilationPass could be imporved, but let's try to keep it a bit smaller for now.

@rburema
Copy link
Member

rburema commented Aug 9, 2021

Closing this in favour of #721 due to laptop & account related shenanigans.

@rburema rburema closed this Aug 9, 2021
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.

4 participants