search

wwwtyro / candygraph

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

Removed memory management mechanics around retain(). Nothing is auto-… #26

Closed wwwtyro closed 2 years ago

wwwtyro commented 2 years ago

…disposed any longer.

@flekschas @blubmin I've decided to remove the memory management mechanics around retain() and default everything to retained. My original purpose was to keep GPU memory management simple for one-off plots, but ultimately it feels awkward, confusing, and better-handled at a higher level of abstraction. I don't think this change requires code review (though certainly feel free to!), I mostly want to make sure I'm not forgetting something that would make this ex-feature a requirement or good idea. Thanks for any feedback!

flekschas commented 2 years ago

Thanks for the heads up! I am out on vacation but happy to take a look when I am back next week.

wwwtyro commented 2 years ago

I'm going to go ahead and merge this PR for now, but we can revisit the approach in the future if we discover issues. :slightly_smiling_face: :+1:

flekschas commented 2 years ago

Apologies, I totally forgot about the review. Thanks for simplifying the API! I like it and the code changes look good :)

wwwtyro commented 2 years ago

No worries at all, thanks for looking, glad it makes sense!