-
-
Notifications
You must be signed in to change notification settings - Fork 8
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
Add pixel getter #34
Add pixel getter #34
Conversation
match self.header.bpp { | ||
Bpp::Bits1 => RawDataSlice::<RawU1, LittleEndian>::new(row) | ||
.into_iter() | ||
.nth(p.x as usize) |
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.
Using nth() seems to be rather expensive for random pixel lookup.
In Bmp format it should be possible to lookup pixels in the raw data block in O(1) time.
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.
There is a specialized implementation of nth
which is O(1).
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.
Where?
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.
RawDataSlice
and the iterators are included in embedded-graphics
: https://github.com/embedded-graphics/embedded-graphics/blob/master/src/iterator/raw.rs
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.
Cool. Does that also count for the nth in line 100?
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.
Yes, Chunks
does also implements nth
: https://doc.rust-lang.org/src/core/slice/iter.rs.html#1501
There might be some performance gain by not using Chunks
, but that could be explored later. My focus for this PR was getting a working implementation not necessarily the fastest.
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.
Very nice. Looks good to me then.
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.
Beat me to it! Looks good to me, although perhaps adding a (not doc) comment to explain the performance in nth()
.
This PR adds pixel getters to
Bmp
andRawBmp
. It doesn't yet implement the newImagePixelGetter
trait (embedded-graphics/embedded-graphics#612) to stay compatible with e-g0.7
.