New shader code about vispy HOT 13 CLOSED

enable_objects is pretty useless as it stands now since it is only used in draw_elements for the index buffer. We should activate/deactivate the index buffer when drawing and that's it.

1. it should be renamed to activate_object() and probably be made _vispy_private because we only use it internally
2. It is not useless and is used to activate the texture and vbo (in variables.py). The points is that it allows one to activate an object and make it automatically be de-activated when the program is deactivated.

Overloaded methods need docstring, at least for us to understand quickly what the method is doing.
assert isinstance(name, str) is pretty useless test since it is followed by an if/elis/else

Agreed

set_data seems a better name for set_var since a bunch of data can be set

I disagree. Consider this:

texture = oogl.Texture2D()
....
texture.set_data(my_array)
program.set_data(texture)   # What?

Apart from uniform scalars/vectors/matrices, you actually do not set any data. You associate/attach data.

Other possible names:

• attach() -> already used for shaders
• associate() -> a bit long
• set_attr() -> we only need it for attributes, but then users will wonder why there is no set_uniform
• set()
• set_var()
• set_item()
• other?

from vispy.

On Aug 28, 2013, at 11:17 AM, Almar Klein [email protected] wrote:

enable_objects is pretty useless as it stands now since it is only used in draw_elements for the index buffer. We should activate/deactivate the index buffer when drawing and that's it.

1. it should be renamed to activate_object() and probably be made _vispy_private because we only use it internally
2. It is not useless and is used to activate the texture and vbo (in variables.py). The points is that it allows one to activate an object and make it automatically be de-activated when the program is deactivated.

Yep, I see how this may be useful later for complex cascading of object, but this is not necessary for the elementbuffer and will maje things slower.

Overloaded methods need docstring, at least for us to understand quickly what the method is doing.
assert isinstance(name, str) is pretty useless test since it is followed by an if/elis/else

Agreed

set_data seems a better name for set_var since a bunch of data can be set

I disagree. Consider this:

texture = oogl.Texture2D()
....
texture.set_data(my_array)
program.set_data(texture) # What?

Apart from uniform scalars/vectors/matrices, you actually do not set any data. You associate/attach data.

"attach" is fine for me since this what we actually do (attach a shader or attach some data to the program).
Even better it is already used for shader indeed.

Other possible names:

• attach() -> already used for shaders
• associate() -> a bit long
• set_attr() -> we only need it for attributes, but then users will wonder why there is no set_uniform
• set()
• set_var()
• set_item()
• other?

---

Reply to this email directly or view it on GitHub:
#50 (comment)

from vispy.

Yep, I see how this may be useful later for complex cascading of object, but this is not necessary for the elementbuffer and will maje things slower.

It is necessary in my opinion. The ElementBuffer, VertexBuffer and Texture objects all need to be activated in order to use them. We could just activate them, but then they are never deactivated. The enable_object() is there to do this all automatically. There is no slowing down, because if we did not do this, the program would simply not work :)

from vispy.

"attach" is fine for me since this what we actually do (attach a shader or attach some data to the program). Even better it is already used for shader indeed.

Mmm ... but it might be confusing to use it both for shaders, uniforms and attributes. On the other hand, it does exactly what it says. What do @lcampagn and @rossant think?

from vispy.

I was saying:

def draw(…)

elembuffer.activate()
...
elembuffer.deactivate()

in this specific case (would the element buffer ever activates other objects ?)

On Aug 28, 2013, at 11:28 AM, Almar Klein [email protected] wrote:

Yep, I see how this may be useful later for complex cascading of object, but this is not necessary for the elementbuffer and will maje things slower.

It is necessary in my opinion. The ElementBuffer, VertexBuffer and Texture objects all need to be activated in order to use them. We could just activate them, but then they are never deactivated. The enable_object() is there to do this all automatically. There is no slowing down, because if we did not do this, the program would simply not work :)

---

Reply to this email directly or view it on GitHub:
#50 (comment)

from vispy.

Ah, right. So the element_buffer can indeed follow that pattern. But this still leaves vertex buffer and texture.

would the element buffer ever activates other objects ?

No, because all objects are agnostic about other objects, except for the Program, that is like the central unit that is aware of how things are connected.

from vispy.

On Wed, Aug 28, 2013 at 3:29 AM, Almar Klein [email protected]:

"attach" is fine for me since this what we actually do (attach a shader or
attach some data to the program). Even better it is already used for shader
indeed.

Mmm ... but it might be confusing to use it both for shaders, uniforms and
attributes. On the other hand, it does exactly what it says. What do
@lcampagn https://github.com/lcampagn and @rossanthttps://github.com/rossantthink?

I agree "attach" might be confusing. "set_var" makes the most sense to me,
because we are setting the contents of a GLSL variable.

from vispy.

Perhaps set_vars() is even better, and reserving the function only for dicts and structured arrays.

from vispy.

I like set_vars(**kwargs) and update_vars(dict_or_structured) since it follows dict conventions (and conceptually, we are configuring a namespace which is very much like a dict).

from vispy.

I left the **kwargs intact. So one can actually do:
prog.set_vars(a_dict) or prog.set_vars(**a_dict)
The latter can of course only be done with a real dict, whereas the first option also accepts a structured array or a structured VertexBuffer.

from vispy.

If everyone is happy, we can close this issue.

from vispy.

I'm happy :)

from vispy.

Closing issue. If not happy, reopen :)

from vispy.