search

wwwtyro / candygraph

Fast by default, flexible 2D plotting library.
Other
435 stars 11 forks source link

Feature/clear cache on dispose #36

Open flekschas opened 2 years ago

flekschas commented 2 years ago

This PR addresses #34 by exposing functions to clear the command, coordinate-scope, and composite cache.

While technically the library user could call these methods, it seems like it'd put quite a huge burden on them to learn when and how to clear the cache most efficiently. Internally, we kind of know then whenever a primitive or composite Renderable is disposed, it can't be rendered anymore and so we could safely removed it's cached draw commands.

Therefore, I've changed all Primitive, Composite, and CoordinateSystem instances to automatically remove themselves from the cache upon disposal. This means, upon properly disposing resources, the memory leak is fixed automatically. A downside is that this is a breaking API change because in order to auto-clear the cache, the resources need to have a reference to the candygraph instance. Another downside of this coupling might be that one cannot reuse resources across candygraph instances but I am not sure this was even possible before because we already coupled the resources via the regl instance used by the candygraph instance.

Let me know what you think of this. Happy to adjust it of course :)

wwwtyro commented 2 years ago

Thanks @flekschas! I'll read through this over the week and try to finish it up on the weekend. :)

wwwtyro commented 2 years ago

Still working through this, but wanted to give you a heads up that this seems to have broken the Logarithmic Y-Axis plot in the examples: image

Also the last plot in the tutorial (another log plot).

wwwtyro commented 2 years ago

This looks good to me, thanks @flekschas! I think we just need to fix whatever broke with the plots and then land it.

flekschas commented 2 years ago

Thanks for the review! I'll take a look today or tomorrow to find out what broke the plots.

flekschas commented 2 years ago

@wwwtyro I fixed the log scale issue. I tried to be too smart by caching the coords-based commands using coord.kind. However, I didn't think of the fact that these two coordinate systems can come in 2 * 2^2 = 8 different flavors. Using coord.glsl as the coord-specific cache key fixes the issue:

Screen Shot 2022-05-23 at 11 58 43 AM
wwwtyro commented 2 years ago

Awesome, thanks, I'll go over it again soon.