Skip to content

Conversation

@Finomnis
Copy link
Contributor

@Finomnis Finomnis commented Nov 10, 2024

The display_sdl already supports ARGB8888 colors, but does not display them correctly.

This PR performs the following things:

  • Add a gray checkerboard as background to the display_sdl driver
  • Add the sample samples/modules/lvgl/screen_transparency that demonstrates its functionality
  • Adjust samples/drivers/display in ARGB8888 mode to have a transparent background
  • Bug fixes

screen_transparency sample before adding transparency support to display_sdl:
image

with transparency support in display_sdl:
image

NOTE: The artifacts around the letters are a bug in LVGL. That bug is fixed in LVGL 9, but in 8.4, this is what it really looks like, this is not a mistake in the display_sdl driver.

This is the updated samples/drivers/display in ARGB8888 mode:
image

@Finomnis Finomnis force-pushed the alpha_screen_main branch 6 times, most recently from e7b40ee to c691c6c Compare November 10, 2024 09:07
@Finomnis Finomnis marked this pull request as ready for review November 10, 2024 09:09
@Finomnis Finomnis force-pushed the alpha_screen_main branch 3 times, most recently from e9e89d3 to fda6257 Compare November 10, 2024 09:18
@kartben
Copy link
Contributor

kartben commented Nov 10, 2024

@Finomnis very nice! Could you share a screenshot of how https://github.com/zephyrproject-rtos/zephyr/tree/main/samples%2Fdrivers%2Fdisplay?
I like the idea of the checkerboard but it feels like it would be nice if the background was customizable, i.e maybe a choice of WHITE/BLACK/GREEN/MAGENTA/CHECKER? And maybe white should still be the default?

@Finomnis
Copy link
Contributor Author

I like the idea of the checkerboard but it feels like it would be nice if the background was customizable, i.e maybe a choice of WHITE/BLACK/GREEN/MAGENTA/CHECKER? And maybe white should still be the default?

Should we make it configurable via Kconfig?

@Finomnis
Copy link
Contributor Author

Finomnis commented Nov 10, 2024

Could you share a screenshot of how https://github.com/zephyrproject-rtos/zephyr/tree/main/samples%2Fdrivers%2Fdisplay?

image
@kartben Would you mind explaining? The colors are 0x00ff0000 and so on, so in my opinion it's correct that they are 100% transparent. Is this a bug in the sample?

@Finomnis
Copy link
Contributor Author

Finomnis commented Nov 10, 2024

@kartben I'm very confused. Is the Alpha value 'transparent' in Zephyr defined as 0xff? Because that's the complete opposite of what it usually is.

LVGL also does "transparent" as 0x00, and so does the rest of the world.

@Finomnis
Copy link
Contributor Author

Finomnis commented Nov 10, 2024

@kartben I can't find the definition of ARGB8888. Can you help me with that?

The entire display_sdl driver uses it like 0xrrggbb => 0x00rrggbb. Although in my opinion it should be 0xffrrggbb everywhere.

This is very confusing and a little bit infuriating. What's going on?

@Finomnis Finomnis force-pushed the alpha_screen_main branch 3 times, most recently from 420b1df to 97d91dd Compare November 10, 2024 10:10
@Finomnis
Copy link
Contributor Author

@kartben Made grid size and color Kconfig values.

@kartben
Copy link
Contributor

kartben commented Nov 10, 2024

@kartben I'm very confused. Is the Alpha value 'transparent' in Zephyr defined as 0xff? Because that's the complete opposite of what it usually is.

LVGL also does "transparent" as 0x00, and so does the rest of the world.

Yes that looks like a bug, they should be 0xff..... indeed

@Finomnis
Copy link
Contributor Author

@kartben Fixed alpha handling in samples/drivers/display and added an alpha test patch. Now looks like this:

image

@Finomnis Finomnis force-pushed the alpha_screen_main branch 2 times, most recently from 79e0977 to cf80afd Compare November 10, 2024 10:54
@Finomnis
Copy link
Contributor Author

@kartben should I create an issue that gets fixed by this change or is it OK if this change exists on its own?

@kartben
Copy link
Contributor

kartben commented Nov 17, 2024

@kartben should I create an issue that gets fixed by this change or is it OK if this change exists on its own?

The latter is fine!

@Finomnis
Copy link
Contributor Author

The latter is fine!

@kartben In that case, is there any way to bump priority?

It's currently really hard to develop apps with screen transparency :/

@kartben
Copy link
Contributor

kartben commented Nov 17, 2024

The latter is fine!

@kartben In that case, is there any way to bump priority?

It's currently really hard to develop apps with screen transparency :/

PR has been open for only a week, right as everyone was pretty busy getting 4.0 ready. Please allow some time for reviewers to have a look at it, likely next week, I would say :)

@Finomnis
Copy link
Contributor Author

PR has been open for only a week, right as everyone was pretty busy getting 4.0 ready. Please allow some time for reviewers to have a look at it, likely next week, I would say :)

Oh, I did not realize it's only been open for a week :D guess I'm being too impatient.

Thanks for the response :)

Adds a sample that renders the simple `hello world` LVGL demo
with a transparent background, to demonstrate ARGB8888 framebuffer
capabilities.

Signed-off-by: Martin Stumpf <[email protected]>
`lv_task_handler()` returns a `uint32_t`, but `k_msleep` takes a
`int32_t`.

If no timer exists, `lv_task_handler()` returns `UINT32_MAX` to indicate
that we should wait forever. However, this gets auto-cast to `-1`,
indicating to `k_msleep` to not wait at all, creating a busy loop.

Hence, a clamping to `[0, INT32_MAX]` is required.

Signed-off-by: Martin Stumpf <[email protected]>
While the driver was already capable of processing `ARGB8888` data,
it did not actually show the alpha value in any way.

This change adds a checkerboard background that shows transparent
regions.

Signed-off-by: Martin Stumpf <[email protected]>
Non-alpha colors were converted to colors with `0x00` alpha, which makes
them fully transparent.

The correct way would be to add a `0xff` alpha, which this change does.

Signed-off-by: Martin Stumpf <[email protected]>
ARGB8888 values all had alpha '0x00' (=transparent).
Changed them to '0xFF' and added a transparency test patch in the middle
of the screen.

Signed-off-by: Martin Stumpf <[email protected]>
@Finomnis
Copy link
Contributor Author

@kartben Thanks for the review and your time :)

@nashif nashif merged commit 5d4f4ac into zephyrproject-rtos:main Nov 18, 2024
24 checks passed
@Finomnis Finomnis deleted the alpha_screen_main branch November 18, 2024 18:18
@Finomnis Finomnis mentioned this pull request Nov 18, 2024
8 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area: Display area: Samples Samples Release Notes To be mentioned in the release notes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants