-
Notifications
You must be signed in to change notification settings - Fork 50
graphic features refactor #473
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
Working on this, Graphic features should have no access to |
92c562d
to
9be017c
Compare
ok so |
I should double check to make sure we don't get any off by 1 errors in the slicing and with |
Ideas for event adding: # graphic base class
Graphic:
def add_event_handler(func, "event-name") Where "event-name" is either a feature name or pygfx event name (ex: "click" etc.). And emitted feature events use a |
@almarklein I've arrived at this parser that takes the key used to index the buffer and auto determines the offset & size to mark for upload, do you think this would be useful to have directly in pygfx? snippet: def _update_range(self, key: int | slice | np.ndarray[int | bool] | list[bool | int] | tuple[slice, ...]):
"""
Uses key from slicing to determine the offset and
size of the buffer to mark for upload to the GPU
"""
# number of elements in the buffer
upper_bound = self.value.shape[0]
if isinstance(key, tuple):
# if multiple dims are sliced, we only need the key for
# the first dimension corresponding to n_datapoints
key: int | np.ndarray[int | bool] | slice = key[0]
if isinstance(key, int):
# simplest case
offset = key
size = 1
elif isinstance(key, slice):
# parse slice
start, stop, step = key.indices(upper_bound)
# account for backwards indexing
if (start > stop) and step < 0:
offset = stop
else:
offset = start
# slice.indices will give -1 if None is passed
# which just means 0 here since buffers do not
# use negative indexing
offset = max(0, offset)
# number of elements to upload
# this is indexing so do not add 1
size = abs(stop - start)
elif isinstance(key, (np.ndarray, list)):
if isinstance(key, list):
# convert to array
key = np.array(key)
if not key.ndim == 1:
raise TypeError(key)
if key.dtype == bool:
# convert bool mask to integer indices
key = np.nonzero(key)[0]
if not np.issubdtype(key.dtype, np.integer):
# fancy indexing doesn't make sense with non-integer types
raise TypeError(key)
if key.size < 1:
# nothing to update
return
# convert any negative integer indices to positive indices
key %= upper_bound
# index of first element to upload
offset = key.min()
# number of elements to upload
# add 1 because this is direct
# passing of indices, not a start:stop
size = np.ptp(key) + 1
else:
raise TypeError(key)
self.buffer.update_range(offset=offset, size=size) |
I need to do a thing in the tests where we just draw frames on demand to test that the right things are in |
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.
Started reviewing things...
Things with lines appear to be working well - shared buffers, event handling
Images are a bit wonking - updating data, changing cmap, etc.
+============+=============+===============================================+ | ||
| type | str | "colors" - name of the event | | ||
+------------+-------------+-----------------------------------------------+ | ||
| graphic | Graphic | graphic instance that the event is from | |
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.
| graphic | Graphic | graphic instance that the event is from | | |
| graphic | Graphic | graphic instance that emitted the event | |
Thought from Billy: make the simplest use cases default kwargs, ex: uniform_colors for line and scatter. |
TODO: need to fix HLUT in |
if self._cmap_values is not None: | ||
if not isinstance(self._cmap_values, np.ndarray): | ||
raise TypeError |
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.
In a previous example with lines we had setting the cmap_values
from a list, with this change we would no longer be supporting
for now, I changed the example to just cast the list to an array
see if this can reopen Edit: apparently not |
See #511 , I accidentally merged this into main 🤦♂️