OK I got the basics down for lines. One caveat is that now there's no check to make sure that the buffer sizes match for data and colors for example.

Consider this:

lg = subplot.add_line(np.random.rand(100))
lg.data = np.random.rand(200)

Now the last 100 line points are just black since they're not defined. A user has to explicitly set the colors of the new vertices that extend beyond the previously defined buffer.

lg.colors = ["w"] * 200

Or they can use a uniform color for things like this.

ok for images I think easiest way is to create new world objects 🤔

EDIT: Yup this will be fine, I was initially worried that we'd have to re-add event handlers if we create a new world object, but I don't think that's necessary since ImageGraphic and ImageVolumeGraphic use a pygfx.Group as the world object, so we can just clear the group and add new image tiles.

EDIT again: Just tested with image click events, if an event handler is added to the graphic then when new tiles are created the graphic is still responsive to those events because it's the pygfx.Group that is handling the events and not the individual image tiles.

@clewis7 would like to get your review before I do the gc tests

📚 Docs preview built and uploaded! https://www.fastplotlib.org/ver/replaceable-buffers

I think we should make uniforms the default for all graphics 🤔.

Also good time to start a "performance tips" section of the docs. Replacing the buffer has a huge performance hit so you don't want do it all the time.

I found a memory leak, if old_buffer._wgpu_object.destroy() is not called then it lingers on in GPU VRAM even if the Binding object no longer exists.

Posted on pygfx: pygfx/pygfx#1264

For now I implemented manually destroying the GPUBuffer when it's replaced.

@clewis7 what do you think about:

For lines, default is:
uniform_colors = True

For scatters, default is:
uniform_colors = False

I think independent scatter colors is a more common usecase than lines. We can auto-determine it in the future based on the colors and cmap arg but I'll do that in the future.

EDIT: But then the API is inconsistent between lines and scatters :/

OK I got the basics down for lines. One caveat is that now there's no check to make sure that the buffer sizes match for data and colors for example.

Consider this:

lg = subplot.add_line(np.random.rand(100))
lg.data = np.random.rand(200)

Now the last 100 line points are just black since they're not defined. A user has to explicitly set the colors of the new vertices that extend beyond the previously defined buffer.

lg.colors = ["w"] * 200

Or they can use a uniform color for things like this.

I think this behavior is confusing. Perhaps it is better to just by default take the current color scheme and extend it to the new or fewer points.

@clewis7 what do you think about:

For lines, default is: uniform_colors = True

For scatters, default is: uniform_colors = False

I think independent scatter colors is a more common usecase than lines. We can auto-determine it in the future based on the colors and cmap arg but I'll do that in the future.

EDIT: But then the API is inconsistent between lines and scatters :/

Hmmm, I do agree that scatters are more likley to have individual colors. How much a performance reduction is it if you make the lines also have uniform_buffer=True as the default??

Maybe we can do an auto mode 🤔
And have it change dynamically too when the colors feature is modified.

Got "auto" color mode working for lines and scatter, and can switch between vertex and uniform modes and the corresponding buffers are auto created and the previous one removed. Need to test that garbage collection works when switching from vertex -> uniform.

I think I'm going to just implement this for colors for now. Will do scatter-specific markers, edge colors, and sizes later since I think expanding the data buffers isn't going to be too common. And when the user does expand the data buffer, they should also be aware that they should set the scatter point markers and other properties for the new points anyways. I'll work on on tests, testing this properly, make sure garbage collection works and then we merge because I need this for #971 . Dynamic display_windows require auto-replaceable buffers.

updated backend tests to account for color_mode and uniforms being default for everything, tests passing on my end

No clue why black show so many changes on github actions, I just blacked locally 🤷 . Maybe black changed things as usual, we can do that in another PR.

Need to update screenshot examples.

Added de-referencing tests and I think stuff works!

Automated tests for check that GPU VRAM is freed seems hard. Manually checking as I showed here seems like the best way 🤷 pygfx/pygfx#1264 (comment)

I tried using this: https://pypi.org/project/nvidia-ml-py/#description

But during offscreen tests it seems like GPU VRAM isn't being used 🤷 . I say let's figure this out later

I just need to add the following tests and do a few other small things:

• image, test w.r.t. the tile sizes for tests
• image, with examples gallery test
• lines, scatters, and collections with example gallery tests
• animation examples to show you can change buffer sizes but also note that it has performance drawbacks and should only be done when really necessary, or if you can take the performance hit when the buffer is replaced.
• note in the docs about performance drawbacks
• docstring on color_mode

OK so there's an issue with scatters (even with the "simple" mode that doesn't use markers or edge_colors) where I get a wgpu bind group error 😭 . The resource has been destroyed, so this manual destroying can cause issues. Will post to pygfx and figure out.

EDIT: ok seems like the dereferencing is clearing GPU VRAM, I guess it does it eventually, not sure what's up with pygfx/pygfx#1264 (comment)

Line and images seem to work, kept this running for 20 mins and it's still going.

Also watching nvidia-smi to see that there's no VRAM leak, after 20 mins no memory leak, goes up and back down as expected 😄

I think I might be able to get rid of the bindgroup issues with scatter by destroying the old buffer after we set the new buffer, instead of right now where it destroys first and then sets the new buffer.

ok interesting, without wgpu_obj.destroy(), seems like it's working for this example, but not the testing UI I made to check buffer gc 🤷‍♂️

I think tests should pass now! 😄

@clewis7 I'm guessing you won't be able to review today so I will merge this into #971 , let me know if you have any objections XD . You can review it later as part of #971

I think tests should pass now! 😄

@clewis7 I'm guessing you won't be able to review today so I will merge this into #971 , let me know if you have any objections XD . You can review it later as part of #971

No objections, can review later :D