Skip to content
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 portrait support for TS35 #25629

Merged
merged 4 commits into from
May 5, 2023
Merged

Add portrait support for TS35 #25629

merged 4 commits into from
May 5, 2023

Conversation

uncellon
Copy link
Contributor

@uncellon uncellon commented Apr 3, 2023

Description

This PR adds base portrait orientation support for MKS TS35 display (in TFT_COLOR_UI interface).

Using the TS35 display in portrait orientation (rotated 90/270 degrees in the config file) now looks like this:
Original

I reworked the current X and Y offsets of the horizontal layout and added support for vertical layout with and without touchscreen support.

Below I have attached an archive with examples of how the interface looks now.

Examples.zip

Requirements

  • Board: MKS Robin Nano V3.1
  • Display: TS35

Benefits

  • Portrait orientation is displayed correctly for 320x480 display.

Configurations

Configuration.zip

Related Issues

@ellensp
Copy link
Contributor

ellensp commented Apr 3, 2023

doesn't TFT_ROTATE_90 already do this?

@uncellon
Copy link
Contributor Author

uncellon commented Apr 3, 2023

@ellensp ellensp

doesn't TFT_ROTATE_90 already do this?

Conditionals_LCD.h - doesn't set the correct width and height when rotated 90/270 degrees.
ui_480x320.{h,cpp} - elements layout hardcoded onlu for a width of 320px.

Upd.: added an example of displaying the screen in the description of the PR.

@uncellon uncellon changed the title Added portrait support for TS35 Add portrait support for TS35 Apr 5, 2023
@thinkyhead
Copy link
Member

doesn't TFT_ROTATE_90 already do this?

On that topic, from a quick scan it looks like only TFT_ROTATION set to TFT_ROTATE_180 really does anything useful. Rotating a non-square device by 90° doesn't make much sense anyway. But I believe that some 90° rotation concept is necessary at the hardware level so that things will draw properly rotated for portrait mode. So, maybe we can look at cleaning that up and making all LCDs that have portrait/landscape options use a single shared config setting.

@uncellon
Copy link
Contributor Author

I don't quite understand why binding to specific pixel values was originally chosen instead of binding to specific areas of the screen. However, I know - it happened historically :) Now it is obvious that the files ui_WxH.{h,cpp} duplicate each other. Literally in January of this year, there was already a similar PR, where a vertical orientation for the 320x240 screen was added. I think that we can concentrate on supporting "layouts" - portrait and landscape, respectively. And do not pay attention to the specific X and Y values of UI elements in specific screen implementations. In fact, this PR is a dirty hack. A hack that allows you to at least somehow work with the portrait orientation of the TS35 screen (and others with a resolution of 480x320, I suppose).

@thinkyhead
Copy link
Member

I don't quite understand why binding to specific pixel values was originally chosen…

I see you amswered your own question. We get these implementations from the vendors who are first to use these devices in their machines, and they tend to be a little myopic, only writing the code they need in the immediate moment. It then falls on the community to generalize and extend their code to other compatible devices.

We have rough APIs like MarlinUI and ExtUI that are meant to be extended as new needs arise, but most coders try to stay within the lines rather than extend and improve those APIs.

I would encourage all contributors to jump in and rework any APIs that are insufficient for today's needs, being mindful that we still want those APIs to decompose into something tiny when built with minimal options so they can still run on smaller AVR devices.

@EvilGremlin
Copy link
Contributor

And speed and size, we don't have any resources to work with abstract things like "area of screen". Probably best thing we can do is make something akin to css style sheets with preprocessor defines for different resolutions and orientations, then use these values in actual dwrawing routines.

@thinkyhead
Copy link
Member

Each type of screen requires a different approach. Fluid layout is useful when you have a window that can be resized at runtime, but for Marlin we just need to statically generate code at build-time that responds to configuration settings and uses the available space efficiently. Where we have a generous amount of space (BIQU BX) we can display larger fonts and more information. It's just a matter of finding the time to build out each screen layout. And, it will help to keep combining the shared code so we can focus on just the parts that need individual customization.

@EvilGremlin
Copy link
Contributor

This is why i think css-like stuff. I think jmz at some point just went "screw it i just finish this easy way", this is why it's all copypaste code in color_ui.

@uncellon
Copy link
Contributor Author

And speed and size, we don't have any resources to work with abstract things like "area of screen"

All these "screen areas" are calculated at compile time. At least I tried to minimize runtime calculations (thanks for macro hell). It's just moving away from the pixel to a more abstract relative value.

@thinkyhead thinkyhead force-pushed the bugfix-2.1.x branch 3 times, most recently from de391db to 0f34163 Compare April 12, 2023 05:14
@thinkyhead thinkyhead merged commit 9cc16f8 into MarlinFirmware:bugfix-2.1.x May 5, 2023
@Foxies-CSTL
Copy link
Contributor

Hi @uncellon ,
Thanks for the improvement but there is still a problem with the adjustment of the icons in the motion screen.
Here is the one on my Mks-TFT32 (320x240) and on the TS35R (480x320).
I have the same problem with a Mks-TFT35 (480x320).

Mks-TS35R
image

Mks-TFT32
image

@uncellon
Copy link
Contributor Author

uncellon commented May 8, 2023

Hi!
Hmm, I really missed that moment, thanks for feedback and 320x240 screenshot. I will try to remake this screen soon as possible

EvilGremlin pushed a commit to EvilGremlin/Marlin that referenced this pull request May 8, 2023
@uncellon
Copy link
Contributor Author

uncellon commented May 8, 2023

Is there any design layout? Like Figma, for example? I'm looking at this "move axis" screen and it seems a bit chaotic. If there's a standard here, I'll just apply the layout.

@thisiskeithb
Copy link
Member

Is there any design layout? Like Figma, for example?

@jmz52 may have something, but you mostly just have to use existing code/layouts (you should also be able to test using the Simulator for faster development, but I’m not sure how well it’ll handle rotated UIs).

EvilGremlin pushed a commit to EvilGremlin/Marlin that referenced this pull request May 17, 2023
tspiva pushed a commit to tspiva/Marlin that referenced this pull request May 25, 2023
Andy-Big pushed a commit to Andy-Big/Marlin_FB_Reborn that referenced this pull request Jul 15, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants