Skip to content
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

allow to specify the framebuffer from the outside. #111

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

simon-budig
Copy link

This pull request makes it possible to hand in a pointer to the framebuffer memory, so that it does not get allocated by the library itself.

This makes it possible for the application, where in the memory the framebuffer is located exactly.

Also this change makes the "pixels" member public.

To the best of my knowledge this is a backward-compatible change. I use it in https://github.com/simon-budig/flame/blob/master/flame.ino

@houston47d
Copy link

Your change makes the assumptions that a user will always specify pix or not specify pix. Should they mix the two, it will result in memory leak or memory corruption (depending on which way it goes) due to trying to free a buffer previously provided or not freeing a buffer previously allocated. While it seems unlikely someone would do it intentionally, it would be pretty easy to do accidentally by missing the second parameter to updateType. I would suggest keeping a flag indicating where the current buffer is from.
Not sure why you want to make pixels a public member. The getPixels() function will return it, and I bet the linker will in-line it so that it is just like referencing it as a public member from a speed perspective. Best to leave it private.

@simon-budig
Copy link
Author

The important part is to be able to make the library work on memory allocated outside the library. This is important for situations where you have a lot of pixels and not much RAM available. In that case it is easy to allocate the memory framebuffer statically and hand it in to the library. If the memory is allocated dynamically (via malloc within the library) you have no control over where it is placed exactly, and it might even fail if the memory is fragmented due to other things that happened in the program earlier.

@simon-budig
Copy link
Author

regarding making pixels public: I originally did that change to the library in 2014, when getPixels() did not exist. So that aspect might very well be obsolete by now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants