ui: display: display a background image on all screens #35
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
While the display itself is 16bpp the emulated framebuffer device is not. Make sure the demo_mode and png-ification code match this expectation. Signed-off-by: Leonard Göhrs <[email protected]>
This makes the screen a bit less boring while still requiring minimal implementation effort. The background image is included in the tacd binary as uncompressed bitmap. This has a couple of benefits and drawbacks: + Loading the background can never fail + The background is available instantly + We do not have to keep the decompressed background in RAM, as it is backed by the binary file. - The tacd binary size increases by 240 * 240 * 3 = 172800 bytes - The build.rs becomes more complex Signed-off-by: Leonard Göhrs <[email protected]>
|
I'd favor a solution where we don't embed the image inside the binary. This takes >100KiB of space, while we don't gain any system memory since the decoded picture needs to be available for framebuffer display anyway. I'd rather ship an asset and decode the picture during startup. |
KarlK90
requested changes
Sep 25, 2023
| let xres = fb.var_screen_info.xres; | ||
| let yres = fb.var_screen_info.yres; | ||
| let res = (xres as usize) * (yres as usize); | ||
| assert!(fb.var_screen_info.bits_per_pixel == 32); |
There was a problem hiding this comment.
Instead of a potential panic please return an appropriate error in this case.
| @@ -144,7 +153,7 @@ impl DrawTarget for DisplayExclusive { | |||
| where | |||
| I: IntoIterator<Item = Pixel<Self::Color>>, | |||
| { | |||
| let bpp = self.0.var_screen_info.bits_per_pixel / 8; | |||
| assert!(self.0.var_screen_info.bits_per_pixel == 32); | |||
There was a problem hiding this comment.
Instead of a potential panic please return an appropriate error in this case.
|
I've coverted the PR back to a draft because (in addition to the technical discussion above) not everyone liked the way the backgrounds look on screen, as they can make text harder to read. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The LXA TAC display looks quite boring due to the
tacdonly displaying black and white text.As a first step to fixing that we can display a background image behind all screens.
The background image is included in the
tacdbinary as uncompressed bitmap. This has a couple of benefits but also some drawbacks:build.rsbecomes more complexTODO: