modules: lvgl: take into account initial display orientation

[Snevzor]

Certain display drivers allow for setting an initial display orientation using a device tree property. LVGL should take this into account while initializing.

[faxe1008]

Did you check by any chance how the rotated flag changes behaviour of the input devices when they are reporting touch coordinates?

[Snevzor]

@faxe1008 I didn't think of this when submitting.

I don't have such input device to test with so I tried to build for SDL. Unfortunately, I have dependency issues to install the correct SDL version which I can't quickly resolve, so no "quick testing" for now.

#73300 seems to take care but I'll have to check further if indeed the .rotated flag messes this up now somewhere in LVGL.

[faxe1008]

I patched rotation support quite crudely into the sdl display:

diff --git a/drivers/display/display_sdl.c b/drivers/display/display_sdl.c
index e929a775f9b..601e9f20c22 100644
--- a/drivers/display/display_sdl.c
+++ b/drivers/display/display_sdl.c
@@ -35,6 +35,7 @@ struct sdl_display_data {
 	void *read_texture;
 	bool display_on;
 	enum display_pixel_format current_pixel_format;
+	enum display_orientation current_orientation;
 	uint8_t *buf;
 	uint8_t *read_buf;
 };
@@ -260,9 +261,15 @@ static int sdl_display_write(const struct device *dev, const uint16_t x,
 		sdl_display_write_bgr565(disp_data->buf, desc, buf);
 	}
 
-	sdl_display_write_bottom(desc->height, desc->width, x, y,
+	if(disp_data->current_orientation == DISPLAY_ORIENTATION_NORMAL) {
+		sdl_display_write_bottom(desc->height, desc->width, x, y,
 				 disp_data->renderer, disp_data->mutex, disp_data->texture,
-				 disp_data->buf, disp_data->display_on);
+				 disp_data->buf, disp_data->display_on, 0);
+	} else if(disp_data->current_orientation == DISPLAY_ORIENTATION_ROTATED_180) {
+		sdl_display_write_bottom(desc->height, desc->width, x, y,
+				 disp_data->renderer, disp_data->mutex, disp_data->texture,
+				 disp_data->buf, disp_data->display_on, 180);
+	}
 
 	return 0;
 }
@@ -516,6 +523,7 @@ static const struct display_driver_api sdl_display_api = {
 	static struct sdl_display_data sdl_data_##n = {			\
 		.buf = sdl_buf_##n,					\
 		.read_buf = sdl_read_buf_##n,				\
+		.current_orientation = DISPLAY_ORIENTATION_ROTATED_180,\
 	};								\
 									\
 	DEVICE_DT_INST_DEFINE(n, &sdl_display_init, NULL,		\
diff --git a/drivers/display/display_sdl_bottom.c b/drivers/display/display_sdl_bottom.c
index 0e995a341c9..b8e0139334a 100644
--- a/drivers/display/display_sdl_bottom.c
+++ b/drivers/display/display_sdl_bottom.c
@@ -67,7 +67,7 @@ int sdl_display_init_bottom(uint16_t height, uint16_t width, uint16_t zoom_pct,
 void sdl_display_write_bottom(const uint16_t height, const uint16_t width,
 			      const uint16_t x, const uint16_t y,
 			      void *renderer, void *mutex, void *texture,
-			      uint8_t *buf, bool display_on)
+			      uint8_t *buf, bool display_on, double angle)
 {
 	SDL_Rect rect;
 	int err;
@@ -87,7 +87,7 @@ void sdl_display_write_bottom(const uint16_t height, const uint16_t width,
 
 	if (display_on) {
 		SDL_RenderClear(renderer);
-		SDL_RenderCopy(renderer, texture, NULL, NULL);
+		SDL_RenderCopyEx(renderer, texture, NULL, NULL, angle, NULL, SDL_FLIP_NONE);
 		SDL_RenderPresent(renderer);
 	}
 
diff --git a/drivers/display/display_sdl_bottom.h b/drivers/display/display_sdl_bottom.h
index 54973777f1b..1d86b551838 100644
--- a/drivers/display/display_sdl_bottom.h
+++ b/drivers/display/display_sdl_bottom.h
@@ -26,7 +26,7 @@ int sdl_display_init_bottom(uint16_t height, uint16_t width, uint16_t zoom_pct,
 void sdl_display_write_bottom(const uint16_t height, const uint16_t width,
 			      const uint16_t x, const uint16_t y,
 			      void *renderer, void *mutex, void *texture,
-			      uint8_t *buf, bool display_on);
+			      uint8_t *buf, bool display_on, double angle);
 int sdl_display_read_bottom(const uint16_t height, const uint16_t width,
 			    const uint16_t x, const uint16_t y,
 			    void *renderer, void *buf, uint16_t pitch,

rotated.mp4

Yeah this is definetely an issue.

[Snevzor]

If lvgl_pointer_process_event has a similar approach as in https://github.com/lvgl/lvgl/blob/master/src/indev/lv_indev.c#L636

I did not expect this to be an issue but your test clearly shows there is an issue.

[faxe1008]

From the looks of it LVGL should be doing the correct thing, I need to test this on real hardware though not on the bodged native_sim. Since I am out of office for the rest of the week I won't able to though.

EDIT:// Yeah forgot to add the rotation into the capabilites struct within my hack:

@@ -455,6 +462,7 @@ static void sdl_display_get_capabilities(
        struct sdl_display_data *disp_data = dev->data;

        memset(capabilities, 0, sizeof(struct display_capabilities));
+       capabilities->current_orientation = disp_data->current_orientation;

With LVGL performing the rotation this code needs to be removed since it will apply the rotation a second time:

zephyr/modules/lvgl/input/lvgl_pointer_input.c

Line 87 in 19f645e

/* rotate touch point to match display rotation */

Can you maybe remove this within your PR and add an explanation for it?

[Snevzor]

I would like to do that, but I'm not sure if I'm able to because I have no means to test my changes.

It also looks like a large part of #73300 needs to be reverted (or completely?) because it introduced tmp_point as well.

[faxe1008]

Well you can verify the changes with a unit test:

• setup the native sim and remove the pointer to the display controller dev handle so you can also generate events from the test
• send the input events using the input api simulating the display press
• get the point from LVGL using lv_indev_get_point

With this setup you can test the rotations across the entire code path.

[Snevzor]

Sounds like a good idea @faxe1008. Unfortunately I still can't install SDL2 (32bit version) on my system without breaking lots of stuff. I also can't seem to find a docker image for this purpose (with display/sdl suport).
I'm new to ZTEST but perhaps I don't need SDL2 at all for this with the display_dummy driver?
I would also have to patch the display_dummy driver with orientation setting and without #73360 approved I still don't know if the way rotation is handled in the driver is correct.
I have limited time left before my holiday though.

[faxe1008]

@Snevzor there is also the native_sim64 target which should allow you to use the 64-bit SDL library.
Anyways I have started working on the tests: https://github.com/zephyrproject-rtos/zephyr/compare/main...faxe1008:zephyr:lvgl_input_tests?expand=1, will submit a PR once ready.

[Snevzor]

@faxe1008 thanks for the native_sim64 hint!

The test you are writing will definitely fill a gap, and it looks great already! I'm learning a lot from it.

[faxe1008]

@Snevzor I have opened #73556 so others will also be able to test the hardware rotation. To cut down on a lot of boilerplate it would make sense for the testing PR to add something similar to what you have done in this PR otherwise there will be an explosion of overlay files in the test.

[Snevzor]

Yes that makes sense @faxe1008. This PR can be closed then ;)

[github-actions]

This pull request has been marked as stale because it has been open (more than) 60 days with no activity. Remove the stale label or add a comment saying that you would like to have the label removed otherwise this pull request will automatically be closed in 14 days. Note, that you can always re-open a closed pull request at any time.