screen size hardcoded and does not change with orientation

[brianmay]

Screen size is hard-coded and should vary depending on the orientation settings.

[brianmay]

Suggested fix (requires change from #7):

diff --git a/src/graphics.rs b/src/graphics.rs
index ad51121..8854d96 100644
--- a/src/graphics.rs
+++ b/src/graphics.rs
@@ -95,21 +95,10 @@ where
     }

     fn clear(&mut self, color: Self::Color) -> Result<(), Self::Error>
-    where
-        Self: Sized,
     {
-        let fb_size = self.model.framebuffer_size();
-        let pixel_count = usize::from(fb_size.0) * usize::from(fb_size.1);
-        let colors = core::iter::repeat(color).take(pixel_count); // blank entire HW RAM contents
-
-        match self.orientation {
-            Orientation::Portrait | Orientation::PortraitSwapped => {
-                self.set_pixels(0, 0, 319, 479, colors)
-            }
-            Orientation::Landscape | Orientation::LandscapeSwapped => {
-                self.set_pixels(0, 0, 479, 319, colors)
-            }
-        }
+        let size = self.size();
+        let area = Rectangle::new(Point::new(0, 0), size);
+        self.fill_solid(&area, color)
     }
 }

@@ -121,6 +110,10 @@ where
 {
     fn size(&self) -> Size {
         let ds = self.model.display_size();
-        Size::new(u32::from(ds.0), u32::from(ds.1))
+        let (width, height) = match self.orientation {
+            Orientation::Portrait => (ds.0, ds.1),
+            Orientation::Landscape => (ds.1, ds.0)
+        };
+        Size::new(u32::from(width), u32::from(height))
     }
 }

For the clear call, was wondering the the All Pixels OFF (22h) and All Pixels ON (23h) instructions to the display could work for black and white respectively. Would be a lot faster. But looks like these commands might fiddle with the sleep status which of the display makes me a bit nervous.

[almindor]

The framebuffer size was used for displays that have bigger draw area than display area (e.g. scrollable hidden area). Clear is supposed to clear the whole thing. Wouldn't your version clear just the visible part?

[brianmay]

Have a look at the code I am replacing. It is somewhat confusing actually, it uses framebuffer_size() but then it uses hard coded values of pixels 0..319 and pixels 0..479. Which matches the actual screen resolution of 320x480.

Also framebuffer_size() just returns the same thing anyway: https://github.com/almindor/mipidsi/blob/1b7d8e2d36596327bcc3867860c3558d47b6990e/src/models.rs#L57-L59

    /// Size of the display framebuffer as `(width, height)`
    fn framebuffer_size(&self) -> (u16, u16) {
        self.display_size()
    }

I replaced the fill_solid code because it assumed portrait resolution and broke with landscape. I replaced the clear code because it looked badly written.

The benefit of using the self.size() function is that it has already been corrected for the orientation. I didn't want to have to duplicate this logic for both the clear and fill_solid methods. Although I guess a another solution would be to have the model.display_size() and model.frame_size() functions take a parameter that gives the current screen orientation.

[brianmay]

Also I note this implements the OriginDimensions trait instead of the Dimensions trait, so the upper left of of the display area will always by 0,0:

https://docs.rs/embedded-graphics/0.7.1/embedded_graphics/geometry/trait.OriginDimensions.html