CARVIEW |
Navigation Menu
-
Notifications
You must be signed in to change notification settings - Fork 625
WIP: Ripping gloo in two via GLIR #571
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
Conversation
Current status: broke the buffer class in two. It works with the examples that I tried. Is ready for a first quick review to see whether this corresponds with what others had in mind. |
logger.debug("GPU: Resizing buffer(%d bytes)" % self._nbytes) | ||
gl.glBufferData(self._target, self._nbytes, self._usage) | ||
self._buffer_size = self._nbytes | ||
pass |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should any / all of these be raise NotImplementedError
for safety? Maybe they should all be, and if a class wants a no-op, then they can explicitly override.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, these should simply be removed (and the calls to them as well), GLIR now takes care of this stuff.
This looks reasonable to me. I wonder if this will help prevent the rare-ish segfaults we're still getting on Travis. As long as the |
Just run all functions in the script that start with 'test_'. In this way when there is an error, I can just jump to PM debug mode in my IDE and inspect what's wong. Nose first runs all tests and then displays a traceback. I want to *go* there.
Also run in predetermined order, helps when re-running tests
ed38d5c
to
36f016e
Compare
Buffer class is implemente via glir. Still need to simplify the buffer class by removing local storage.
a995aa3
to
5916048
Compare
Whoah, tests pass. Please don't merge though, there is some ugly stuff in there to make it work. Next class to split in two: gloo.Program. I'll get my axe. |
@@ -109,7 +112,9 @@ def set_front_face(mode='ccw'): | |||
mode : str | |||
Can be 'cw' for clockwise or 'ccw' for counter-clockwise. | |||
""" | |||
gl.glFrontFace(_gl_attr(mode)) | |||
#gl.glFrontFace(_gl_attr(mode)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
remove?
LGTM apart from a few minor comments (notably dead commented code that should probably be removed). |
Addressed comments. There are two pending questions for @Eric89GXL see comments in code. |
These comments are now also addressed. Cyrille was right, I was under the impression that it was |
OK, ready for merging then! \o/ |
Maybe @lcampagn wants to have a look? |
Overall I am impressed that tests are passing and the API is almost exactly the same! However, broken examples: Most of these appear to be due to some problem with MeshVisual. |
Fixed examples. One was a quantum-bug; it disappeared when I ran with |
Looks like Travis is happy, other than the size test. Is this ready for merge from your end @almarklein? If so, I'm +1 for merge but I'll let @rossant accept responsibility for pushing the button :) |
I am ready :D |
(and really happy to have this about finished, it took quite some effort) |
Yeah, it's quite an impressive bit of work. If it really helps @rossant build clean/fast WebGL visualizations it will be incredibly valuable. |
Fantastic!! Much congrats to Almar. We've (slightly) reduced the code base, fixed quite a few bugs, and we have a more solid architecture IMO. And yes, the WebGL backend is coming soon hopefully, I was waiting for this PR to finish it... |
WIP: Ripping gloo in two via GLIR
Just a quick note: you need to put |
Oh I thought it was the other way around sorry.. Le jeudi 16 octobre 2014, Almar Klein notifications@github.com a écrit :
|
No problem :) |
This PR refactors gloo to break it in two pieces, one is the high level gloo interface, which generates GLIR commands. The other is the GLIR interpreter.
In the progress I will also fix some outstanding issues with gloo, as described in #464, like getting rid of local strorage and allow specifying uniforms/attributes before the source code is set.
Closes #464, #510, #450, #338, #407, #351
Overview of changes that this PR makes
All gloo objects:
Program:
active_uniforms
. Just a function to get info on all variables in shading code.variable.py
is gone. These objects served mainly for deferred setting of values. The GLIR command queue takes over this function.shaders.py
is gone. It turns out that shaders do not really have a function other then temporarily existsting to compile the code. Vispy programs should now have more GPU memory available. Just callprogram.set_shaders()
to set source code.Buffer:
Texture:
set_size
with completely different size and format.set_data
using any type and shape.Texture((100,100))
Framebuffer
Outstanding questions for GLIR spec
Work to do after this