Use iterators in interface send methods (#106)

* Use iterators in interface send methods

* Optimise set_pixel in graphics mode

* Dedupes byte/bit index calculation
* Replaces a slice index operation with `get_mut`, eliding bounds
checks (I think - the call tree is pretty hairy)
* Replaces if/else bit set/clear op with bit ops. See here for asm
output: https://godbolt.org/z/YRSanf

* Changelog entry

Author: jamwaffles

CHANGELOG.md

@@ -9,6 +9,7 @@
 ### Changed
 
 - [#105](https://github.com/jamwaffles/ssd1306/pull/105) Reduce flash usage by around 400 bytes by replacing some internal `unwrap()`s with `as` coercions.
+- [#106](https://github.com/jamwaffles/ssd1306/pull/106) Optimise internals by using iterators to elide bounds checks. Should also speed up `GraphicsMode` (and `embedded-graphics` operations) with a cleaned-up `set_pixel`.
 
 ## [0.3.0-alpha.4] - 2020-02-07
 

src/interface/i2c.rs

@@ -50,18 +50,16 @@ where
         // 8.1.5.2 5) b) in the datasheet
         writebuf[0] = 0x40;
 
-        for chunk in buf.chunks(16) {
-            let chunklen = chunk.len();
+        buf.chunks(16).try_for_each(|c| {
+            let chunk_len = c.len();
 
             // Copy over all data from buffer, leaving the data command byte intact
-            writebuf[1..=chunklen].copy_from_slice(&chunk[0..chunklen]);
+            writebuf[1..=chunk_len].copy_from_slice(c);
 
             self.i2c
-                .write(self.addr, &writebuf[..=chunklen])
-                .map_err(Error::Comm)?;
-        }
-
-        Ok(())
+                .write(self.addr, &writebuf[0..=chunk_len])
+                .map_err(Error::Comm)
+        })
     }
 
     fn send_bounded_data(
@@ -76,39 +74,39 @@ where
             return Ok(());
         }
 
-        let mut writebuf: [u8; 17] = [0; 17];
-
-        // Divide by 8 since each row is actually 8 pixels tall
-        let height = ((lower_right.1 - upper_left.1) / 8) as usize;
-
-        let starting_page = (upper_left.1 / 8) as usize;
+        // Write buffer. Writes are sent in chunks of 16 bytes plus DC byte
+        let mut writebuf: [u8; 17] = [0x0; 17];
 
         // Data mode
         // 8.1.5.2 5) b) in the datasheet
         writebuf[0] = 0x40;
 
-        let mut page_offset = starting_page * disp_width;
-
-        for _ in 0..=height {
-            let start_index = page_offset + upper_left.0 as usize;
-            let end_index = page_offset + lower_right.0 as usize;
-
-            page_offset += disp_width;
-
-            let sub_buf = &buf[start_index..end_index];
-
-            for chunk in sub_buf.chunks(16) {
-                let chunklen = chunk.len();
-
-                // Copy over all data from buffer, leaving the data command byte intact
-                writebuf[1..=chunklen].copy_from_slice(&chunk[0..chunklen]);
+        // Divide by 8 since each row is actually 8 pixels tall
+        let num_pages = ((lower_right.1 - upper_left.1) / 8) as usize + 1;
 
-                self.i2c
-                    .write(self.addr, &writebuf[..=chunklen])
-                    .map_err(Error::Comm)?;
-            }
-        }
+        // Each page is 8 bits tall, so calculate which page number to start at (rounded down) from
+        // the top of the display
+        let starting_page = (upper_left.1 / 8) as usize;
 
-        Ok(())
+        // Calculate start and end X coordinates for each page
+        let page_lower = upper_left.0 as usize;
+        let page_upper = lower_right.0 as usize;
+
+        buf.chunks(disp_width)
+            .skip(starting_page)
+            .take(num_pages)
+            .map(|s| &s[page_lower..page_upper])
+            .try_for_each(|c| {
+                c.chunks(16).try_for_each(|c| {
+                    let chunk_len = c.len();
+
+                    // Copy over all data from buffer, leaving the data command byte intact
+                    writebuf[1..=chunk_len].copy_from_slice(c);
+
+                    self.i2c
+                        .write(self.addr, &writebuf[0..=chunk_len])
+                        .map_err(Error::Comm)
+                })
+            })
     }
 }

src/interface/spi.rs

@@ -57,22 +57,20 @@ where
         self.dc.set_high().map_err(Error::Pin)?;
 
         // Divide by 8 since each row is actually 8 pixels tall
-        let height = ((lower_right.1 - upper_left.1) / 8) as usize;
+        let num_pages = ((lower_right.1 - upper_left.1) / 8) as usize + 1;
 
+        // Each page is 8 bits tall, so calculate which page number to start at (rounded down) from
+        // the top of the display
         let starting_page = (upper_left.1 / 8) as usize;
 
-        let mut page_offset = starting_page * disp_width;
+        // Calculate start and end X coordinates for each page
+        let page_lower = upper_left.0 as usize;
+        let page_upper = lower_right.0 as usize;
 
-        for _ in 0..=height {
-            let start_index = page_offset + upper_left.0 as usize;
-            let end_index = page_offset + lower_right.0 as usize;
-            let sub_buf = &buf[start_index..end_index];
-
-            page_offset += disp_width;
-
-            self.spi.write(&sub_buf).map_err(Error::Comm)?;
-        }
-
-        Ok(())
+        buf.chunks(disp_width)
+            .skip(starting_page)
+            .take(num_pages)
+            .map(|s| &s[page_lower..page_upper])
+            .try_for_each(|c| self.spi.write(&c).map_err(Error::Comm))
     }
 }

src/mode/graphics.rs

@@ -200,54 +200,32 @@ where
         let (display_width, _) = self.properties.get_size().dimensions();
         let display_rotation = self.properties.get_rotation();
 
-        let idx = match display_rotation {
+        let (idx, bit) = match display_rotation {
             DisplayRotation::Rotate0 | DisplayRotation::Rotate180 => {
-                if x >= display_width as u32 {
-                    return;
-                }
-                ((y as usize) / 8 * display_width as usize) + (x as usize)
-            }
-
-            DisplayRotation::Rotate90 | DisplayRotation::Rotate270 => {
-                if y >= display_width as u32 {
-                    return;
-                }
-                ((x as usize) / 8 * display_width as usize) + (y as usize)
-            }
-        };
-
-        if idx >= self.buffer.len() {
-            return;
-        }
-
-        // Keep track of max and min values
-        self.min_x = self.min_x.min(x as u8);
-        self.max_x = self.max_x.max(x as u8);
+                let idx = ((y as usize) / 8 * display_width as usize) + (x as usize);
+                let bit = y % 8;
 
-        self.min_y = self.min_y.min(y as u8);
-        self.max_y = self.max_y.max(y as u8);
-
-        let (byte, bit) = match display_rotation {
-            DisplayRotation::Rotate0 | DisplayRotation::Rotate180 => {
-                let byte =
-                    &mut self.buffer[((y as usize) / 8 * display_width as usize) + (x as usize)];
-                let bit = 1 << (y % 8);
-
-                (byte, bit)
+                (idx, bit)
             }
             DisplayRotation::Rotate90 | DisplayRotation::Rotate270 => {
-                let byte =
-                    &mut self.buffer[((x as usize) / 8 * display_width as usize) + (y as usize)];
-                let bit = 1 << (x % 8);
+                let idx = ((x as usize) / 8 * display_width as usize) + (y as usize);
+                let bit = x % 8;
 
-                (byte, bit)
+                (idx, bit)
             }
         };
 
-        if value == 0 {
-            *byte &= !bit;
-        } else {
-            *byte |= bit;
+        if let Some(byte) = self.buffer.get_mut(idx) {
+            // Keep track of max and min values
+            self.min_x = self.min_x.min(x as u8);
+            self.max_x = self.max_x.max(x as u8);
+
+            self.min_y = self.min_y.min(y as u8);
+            self.max_y = self.max_y.max(y as u8);
+
+            // Set pixel value in byte
+            // Ref this comment https://stackoverflow.com/questions/47981/how-do-you-set-clear-and-toggle-a-single-bit#comment46654671_47990
+            *byte = *byte & !(1 << bit) | (value << bit)
         }
     }