Skip to content

Conversation

@faxe1008
Copy link
Contributor

@faxe1008 faxe1008 commented May 31, 2024

Originally brought up within #73474 this PR tries to improve the support for hardware based display rotation during runtime.

Since in the past few weeks quite a few bugfixes in regards to pointer input device handling were merged it would be useful to have them properly unit tested.

Test output:

===================================================================
START - test_no_rotation
        Expected: x:20 y:150
        Press:    x:20 y:150
        Release:  x:20 y:150
 PASS - test_no_rotation in 0.001 seconds
===================================================================
START - test_rotation_180
        Expected: x:300 y:90
        Press:    x:300 y:90
        Release:  x:300 y:90
 PASS - test_rotation_180 in 0.001 seconds
===================================================================
START - test_rotation_270
        Expected: x:150 y:300
        Press:    x:150 y:300
        Release:  x:150 y:300
 PASS - test_rotation_270 in 0.001 seconds
===================================================================
START - test_rotation_90
        Expected: x:90 y:20
        Press:    x:90 y:20
        Release:  x:90 y:20
 PASS - test_rotation_90 in 0.001 seconds
===================================================================

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add a comment that this should not be called while rendering is in progress.

Adds the rotation property to the dummy display binding to allow for
setting an initial rotation within tests.

Signed-off-by: Fabian Blatz <[email protected]>
@faxe1008 faxe1008 force-pushed the lvgl_input_tests branch from bc4f513 to 2e09ba8 Compare May 31, 2024 12:25
Add a method which allows for syncing information within the
display_data struct after the display has been rotated in the
application using `display_set_orientation`. This also makes the
implementation of various input devices unit testable without requiring
a lot of overlay files.

After the current implementation is verified
against unit tests, the reload method will additionally set the
`rotated` flag on the display driver. The LVGL code will then
handle the translation of input devices points to points in the GUI
according to the display rotation.

Signed-off-by: Fabian Blatz <[email protected]>
@faxe1008 faxe1008 force-pushed the lvgl_input_tests branch from 2e09ba8 to e62ddaf Compare May 31, 2024 12:33
@faxe1008 faxe1008 marked this pull request as ready for review May 31, 2024 12:37
@zephyrbot zephyrbot added area: Devicetree Binding PR modifies or adds a Device Tree binding area: LVGL Light and Versatile Graphics Library Support area: Display labels May 31, 2024
@faxe1008
Copy link
Contributor Author

Changing this from draft state to review to get some more feedback and hopefully quite a few other pair of eyes on this since the coordinate conversion code has been tricky to handle.

Added @ioannis-karachalios as a reviewer because of #73407

@faxe1008 faxe1008 force-pushed the lvgl_input_tests branch from e62ddaf to 973242a Compare May 31, 2024 15:06
faxe1008 added 2 commits May 31, 2024 17:20
Adds a test for lvgl input devices in various configurations, like
rotated display, pointer inversions etc.

Signed-off-by: Fabian Blatz <[email protected]>
LVGL supports the `rotated` flag on its display drivers. It is used
during input device coordinate processing to translate the physical
touch points to the points in (rendered) GUI space. Previously this was
done by the Zephyr gluecode on system init. With this change it is now
possible to perform runtime rotation of the display, the
`lvgl_reload_display_capabilities` method should be called right after
`display_set_orientation`.

Signed-off-by: Fabian Blatz <[email protected]>
@Snevzor
Copy link
Contributor

Snevzor commented Jun 1, 2024

Perhaps the test can be expanded to take into account correct rotation initialisation based on the various rotation dts values as well? Like: looping through all dts values and then checking if the input values match. That way the updated LVGL init code is also tested.

@faxe1008 faxe1008 added the DNM This PR should not be merged (Do Not Merge) label Jun 2, 2024
@faxe1008
Copy link
Contributor Author

faxe1008 commented Jun 2, 2024

@Snevzor neat idea but I fear that this would mean that I'd have to add an overlay on for each rotation and do some messy things with skipping tests I pressume. Could be wrong though :^).

Btw I added a DNM label for now since I want to clarify the discussion around #73360 first.

@github-actions
Copy link

github-actions bot commented Aug 2, 2024

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.

@github-actions github-actions bot added the Stale label Aug 2, 2024
@github-actions github-actions bot closed this Aug 16, 2024
@danieldegrasse
Copy link
Contributor

@faxe1008 apologies for the necromancy here- It seems like this PR is the start of what we would need to standardize how rotation works within Zephyr (IE if the display driver should be swapping x/y parameters when rotated, or if the consumer of the display API should be handling this)

Do you still want to move this forwards? It seems like we'd need this, as well as changes to the gc9x01x and ili9xxx drivers to align them with the remainder of the codebase

@Snevzor
Copy link
Contributor

Snevzor commented Dec 5, 2024

@faxe1008 please let me know if you need some assistance, I definitely want to help where possible.

@faxe1008
Copy link
Contributor Author

faxe1008 commented Dec 9, 2024

Heyhey @danieldegrasse and @Snevzor yes eventually I want to continue working on that issue, either by contributing or reviewing, if someone feels inspired to take this up in the meantime. I want to first finish up the update of the module its self since this has been sitting on my plate quite some time now before moving on to other issues. Can you maybe spare some time to look at #81263?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area: Devicetree Binding PR modifies or adds a Device Tree binding area: Display area: LVGL Light and Versatile Graphics Library Support DNM This PR should not be merged (Do Not Merge) Stale

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants