almindor / mipidsi

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

Display orientation #38

Closed rfuest closed 5 months ago

rfuest commented 1 year ago

I've just tried to port the crate to an ILI9341 controller and discovered an issue with the Orientation type. One difference between the ILI9341 and the already supported ILI9342 is the default display orientation, because they have a different number row and column outputs. The ILI9342 supports 320x240 pixel LCDs and the ILI9341 supports 240x320 pixel LCDs.

The current Orientation::Landscape and Orientation::Portrait names don't really make sense if the default Portrait could be 320x240 or 240x320. Perhaps this should be change to the types we discussed in https://github.com/embedded-graphics/embedded-graphics/issues/647#issuecomment-1110065077?

almindor commented 1 year ago

I've just tried to port the crate to an ILI9341 controller and discovered an issue with the Orientation type. One difference between the ILI9341 and the already supported ILI9342 is the default display orientation, because they have a different number row and column outputs. The ILI9342 supports 320x240 pixel LCDs and the ILI9341 supports 240x320 pixel LCDs.

The current Orientation::Landscape and Orientation::Portrait names don't really make sense if the default Portrait could be 320x240 or 240x320. Perhaps this should be change to the types we discussed in embedded-graphics/embedded-graphics#647 (comment)?

I'd be glad to move the orientation type out to embedded-graphics I think it makes sense to be there.

rfuest commented 1 year ago

I'd be glad to move the orientation type out to embedded-graphics I think it makes sense to be there.

OK, good.

I was thinking about creating another crate for this, because e-g is supposed to be target agnostic and can also be used to draw to other draw targets, like an image file. Do you think that creating a display-driver-hal (or similar) crate is a good idea? It could be an "official embedded-graphics project", but not in the main e-g repo.

almindor commented 1 year ago

I'd be glad to move the orientation type out to embedded-graphics I think it makes sense to be there.

OK, good.

I was thinking about creating another crate for this, because e-g is supposed to be target agnostic and can also be used to draw to other draw targets, like an image file. Do you think that creating a display-driver-hal (or similar) crate is a good idea? It could be an "official embedded-graphics project", but not in the main e-g repo.

If we're to split this into a crate we might want to consider if this is the only type that goes in. I feel like there are probably more things.

What about stuff like color ordering? Right now Mipidsi has this type ColorOrder with Rgb and Bgr. Should this be there too?

rfuest commented 1 year ago

Yes. The Orientation type would only be a start and ColorOrder is also a good candidate.

In the end I would like to add something like the HalDisplay trait that was suggested in the e-g issue. But starting with a few types would at least get the ball rolling.

rfuest commented 1 year ago

I've just started a new project, which contains a Orientation trait and also a way to convert it the necessary parameters for MADCTR: https://github.com/embedded-graphics/display-driver-hal

almindor commented 1 year ago

I've just started a new project, which contains a Orientation trait and also a way to convert it the necessary parameters for MADCTR: https://github.com/embedded-graphics/display-driver-hal

Perfect! I'll check this out ASAP and create draft PR here for the driver.

bsodmike commented 1 year ago

Using a 240, 320 display has some unexpected issues with regards to the available screen size.

Here's a default display (at 320, 240), but even if I set the size to 240, 320 + the frame buffer, it will render the same black/grey split - but the "bottom" of the screen does indeed move to the very bottom.

ima_cb21a31

When configured as 240, 320 - notice the bottom row: ima_6882e7e

bsodmike commented 1 year ago

Apologies, this works now - I had to use the master branch

        builder
            .with_display_size(240, 320)
            .with_framebuffer_size(240, 320)
            .with_color_order(mipidsi::ColorOrder::Bgr)
            .with_invert_colors(true)
            .with_orientation(mipidsi::Orientation::Portrait(true))
            .init(&mut delay::Ets, Some(rst))
            .unwrap()

ima_d17eeca

rfuest commented 5 months ago

The new Orientation type has been added in #80.