almindor / mipidsi

MIPI Display Serial Interface unified driver
MIT License
108 stars 46 forks source link

Refactors the driver and models to support dynamic display sizing and clipping #28

Closed almindor closed 1 year ago

almindor commented 1 year ago

This PR tries to address #27 and #20 by refactoring the DisplayOptions and init paths so variants of the same Display and Model can be used with different physical display sizes.

This also adds support for "cropped" display variants in which the display framebuffer is bigger but the visible display is smaller as well as "offset" from the (0, 0) coordinates internally.

almindor commented 1 year ago

I think I'll make two changes to this PR based on current observations:

  1. Change DisplayOptions::window_offset into a Option<Fn(Orientation) -> (u16, u16)> so that logic can be applied from a closure instead of depending on direct values
  2. Change Model constructor logic to enforce "variant constructors" directly so there's no "default" config to fall back to.
  3. In relation to 2. probably split DisplayOptions so that Model/variant sizing definitions are in Model specific options instead.
almindor commented 1 year ago

@cloneable @Chippit @andreban @9names this should now be ready to test for all the variants. If you can please check if this code works for you.

It might also be missing some variant constructors, feel free to ping about that. There was also a bug fix in the fill_solid function which might explain why lines worked where fills didn't when framebuffer was misdefined (or bugged as it was in this PR before)

9names commented 1 year ago

This is what it looks like using st7789_pico1 on commit 442e663 pico_display_new I'll play with the offsets and see what I come up with...

9names commented 1 year ago

Here's the offsets that appear to be correct:

    match orientation {
        Orientation::Portrait(false) => (52, 40),
        Orientation::Portrait(true) => (53, 40),
        Orientation::Landscape(false) => (40, 52),
        Orientation::Landscape(true) => (40, 53),
        Orientation::PortraitInverted(false) => (53, 40),
        Orientation::PortraitInverted(true) => (52, 40),
        Orientation::LandscapeInverted(false) => (40, 53),
        Orientation::LandscapeInverted(true) => (40, 52),
    }
almindor commented 1 year ago

@9names @andreban @cloneable @Chippit I think this is really close now. I'm going to wait for a week or so before merging and releasing this in case someone finds an issue since it does change things on the API level

Chippit commented 1 year ago

Those Pico1 offsets look good for my Waveshare device as well.

andreban commented 1 year ago

I'm using the same Pico display as @9names. These offsets look good to me.

almindor commented 1 year ago

Thanks for the testing guys, I'll probably wrap this up in a day or two and release 0.4!