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 color depth types to disp_dev API #14054

Draft
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

Citrullin
Copy link
Contributor

@Citrullin Citrullin commented May 11, 2020

Contribution description

This PR addresses the issue with colors in the disp_dev API. Mentioned briefly in #13787
It's a proposal how to address it.
The idea behind this is simple: The user application shouldn't care too much about the underlying color depth of a display.
It should provide the color data in the correct format. The driver should do the rest. This also makes it possible to switch displays with different color depths without changing the color data itself. Also the application code should be the same. The user only has to change the makefile.
For example:

  • User applications uses 24bit color data and a 24bit color display. The driver itself doesn't need to change the data that much, since it can display it directly.
  • For another version of the hardware, the user changes the display to a 16bit display. The driver abstracts this, accepts the 24bit color data and transforms it to the needed 16bit color.

This is an down scaling example. The same can be done for up scaling as well.
The biggest benefit is backwards compatibility. The user can simply maintain backwards compatibility, without caring too much about the old hardware.

Testing procedure

The tests are not changed yet.

Issues/PRs references

Addresses #13787 . Should be merged after #14051 .

*/
typedef struct {
bool black;
bool color;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not sure about this though. Maybe storing it in one uint8_t is better. Or maybe two booleans is better for the compiler to optimize.

@miri64 miri64 added Area: drivers Area: Device drivers Process: API change Integration Process: PR contains or issue proposes an API change. Should be handled with care. Type: new feature The issue requests / The PR implemements a new feature for RIOT labels Jul 2, 2020
@stale
Copy link

stale bot commented Jan 6, 2021

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. If you want me to ignore this issue, please mark it with the "State: don't stale" label. Thank you for your contributions.

@stale stale bot added the State: stale State: The issue / PR has no activity for >185 days label Jan 6, 2021
@Citrullin
Copy link
Contributor Author

"State: don't stale" Still an open topic

@stale stale bot removed the State: stale State: The issue / PR has no activity for >185 days label Jan 6, 2021
@aabadie aabadie added the State: waiting for other PR State: The PR requires another PR to be merged first label Feb 23, 2021
@MrKevinWeiss MrKevinWeiss added this to the Release 2021.07 milestone Jun 22, 2021
@MrKevinWeiss MrKevinWeiss removed this from the Release 2021.07 milestone Jul 15, 2021
@stale
Copy link

stale bot commented Mar 2, 2022

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. If you want me to ignore this issue, please mark it with the "State: don't stale" label. Thank you for your contributions.

@stale stale bot added the State: stale State: The issue / PR has no activity for >185 days label Mar 2, 2022
@Citrullin
Copy link
Contributor Author

Still an issue.

@stale stale bot removed the State: stale State: The issue / PR has no activity for >185 days label Mar 16, 2022
@stale
Copy link

stale bot commented Sep 21, 2022

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. If you want me to ignore this issue, please mark it with the "State: don't stale" label. Thank you for your contributions.

@stale stale bot added the State: stale State: The issue / PR has no activity for >185 days label Sep 21, 2022
@Teufelchen1 Teufelchen1 added the State: don't stale State: Tell state-bot to ignore this issue label Mar 18, 2024
@Teufelchen1
Copy link
Contributor

Hi!
Do you still plan to get this merged? The PR this depends on got closed by stale bot #14051.

@Citrullin
Copy link
Contributor Author

Hi! Do you still plan to get this merged? The PR this depends on got closed by stale bot #14051.

Not really, sorry. There was so much discussion about this (see #13787 etc.)
I guess it needs someone to push it. Get all people together, make a clean PR with all the concerns, get it merged.

@Teufelchen1 Teufelchen1 marked this pull request as draft April 15, 2024 09:58
@stale stale bot removed the State: stale State: The issue / PR has no activity for >185 days label Apr 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: drivers Area: Device drivers Process: API change Integration Process: PR contains or issue proposes an API change. Should be handled with care. State: don't stale State: Tell state-bot to ignore this issue State: waiting for other PR State: The PR requires another PR to be merged first Type: new feature The issue requests / The PR implemements a new feature for RIOT
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants