Skip to content

swapped red and blue channels because it was messed up in the first PR#10331

Open
sandreas wants to merge 11 commits intoslint-ui:masterfrom
sandreas:9862-fix-switched-colors
Open

swapped red and blue channels because it was messed up in the first PR#10331
sandreas wants to merge 11 commits intoslint-ui:masterfrom
sandreas:9862-fix-switched-colors

Conversation

@sandreas
Copy link
Contributor

This is a follow up for #9914 which somehow seems to have messed up the red and blue channel (see attached screenshot vs the original image).

I hope this fixes the problem - I'm going to do more research if it does not.

slint-colors

@ogoffart ogoffart requested a review from tronical December 30, 2025 07:53
@tronical
Copy link
Member

tronical commented Jan 3, 2026

Hi @sandreas . thanks for the patch. I'm wondering: doesn't from_rgb() need to be adjusted as well? And if yes, isn't the the implementation identical with Xrgb888? If it is, then I suppose one type could be removed. What do you think?

@sandreas
Copy link
Contributor Author

sandreas commented Jan 3, 2026

Hi @sandreas . thanks for the patch. I'm wondering: doesn't from_rgb() need to be adjusted as well? And if yes, isn't the the implementation identical with Xrgb888? If it is, then I suppose one type could be removed. What do you think?

Thank you for the quick feedback. Probably you're right. First I was just trying to fix that slint somehow reported the BA24 Renderer (which turned out to internally by Bgra8888) was not supported at all. Maybe we could just add an alias instead of adding a whole new implementation that does the same.

Unfortunately it took a while until I noticed that the colors were switched - first I thought it had to do something with dark mode (which I was experimenting with at the time).

So I think I need to test this on the device I'm working with to ensure not patching the wrong thing (see https://github.com/sandreas/rust-slint-riscv64-musl-demo?tab=readme-ov-file#hardware). This might take some days, I'll report back.

@tronical
Copy link
Member

tronical commented Jan 3, 2026

An alias makes sense, or a more generic name (bgra8888 and we just also use it for the kernel's xrgb)

Anyway, take your time to test it on the device:)

@sandreas
Copy link
Contributor Author

sandreas commented Jan 11, 2026

@tronical

I'm pretty confused now... While an alias:

// ...
drm::buffer::DrmFourcc::Xrgb8888 | drm::buffer::DrmFourcc::Argb8888 | drm::buffer::DrmFourcc::Bgra8888 => {
// ...

did not change anything, my PR did change the colors for my PNG icons, but did not for a jpg cover image. This is very strange and I have no clue where to start with this.

Here is an image showing the png icons before my code change - which also represents the state of the switched colors of the Benjamin Blümchen cover I provided above:
BEFORE the PR

And here is an image after the patch (I included the cover on the front page to ensure the problem is not in one of the images):

AFTER THE PR

As you see, the Icon for audio books turned from blue-ish to yellow, music turned from red to blue and search from blue to red, which is now correct. However, the cover image is not displayed correctly. Below is light-mode Desktop screenshot of what I would expect:

EXPECTED / DESKTOP VARIANT (light mode, dark mode does not work on Linux Desktop)

So the Icons (PNG) are correct while the cover is not...
Could you tell me anything about this behaviour? Is the image renderer failing or is it me? ;)

@sandreas
Copy link
Contributor Author

sandreas commented Jan 12, 2026

@tronical
Ok I think I got it. Clearly was my fault and I think the problem was that I did not notice it was necessary to commit the local slint git repository followed by a cargo update in the using repository to check if the changes would fix the problem.

However, now everything seems to work with an alias and I removed a lot of duplicated code. I don't know why I did not notice that an alias is enough in the first place - well, I learned a lot of Rust in the last weeks :-)

Screenshot with test images and rectangles that show it is now working as expected:

@tronical
Copy link
Member

Something still seems off in this patch.

Before this patch, this was the encoding for Bgra:

            red: (v >> 0) as u8,
            green: (v >> 8) as u8,
            blue: (v >> 16) as u8,
            alpha: (v >> 24) as u8,

This could be interpreted as "rgba" or "abgr", but somehow not as "bgra" I think.

With this patch, Argb8888 is treated the same as Bgra8888, which also seems not the same.

I think at this point we need your help with testing to find the right encoding, as you have the necessary hardware.

My guts feeling is that what's in the git repo now is almost correct, just that the blue and red channels are swapped, so perhaps the existing Bgra target pixel implementation just needs fixing.

tronical added a commit that referenced this pull request Jan 22, 2026
…e software renderer

The implementation was essentially ABGR instead of BGRA. BGRA is defined like this:

    [31:0] B:G:R:A 8:8:8:8 little endian

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants