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 xxxx32 pixel formats #825

Closed
Rua opened this issue Dec 16, 2018 · 5 comments
Closed

Add xxxx32 pixel formats #825

Rua opened this issue Dec 16, 2018 · 5 comments

Comments

@Rua
Copy link
Contributor

Rua commented Dec 16, 2018

Since SDL 2.0.5, there are four new pixel formats for "array" formats: RGBA32, ARGB32, BGRA32 and ABGR32. These map to the existing 8888 "packed" formats, but in different ways per platform so that the in-memory order of the colours is always the same regardless of platform.

The Rust version of SDL is missing these. SDL_pixels.h defines these using conditional compilation, and I'm not sure how that should be done in Rust, or I would make a PR.

@Cobrand
Copy link
Member

Cobrand commented Dec 17, 2018

The Rust version of SDL is missing these. SDL_pixels.h defines these using conditional compilation, and I'm not sure how that should be done in Rust, or I would make a PR.

But that's only used internally by SDL, right? Wedon't have to make conditionals in the Rust code.

For the PR, I think youyr best bet would be to look here: https://github.com/Rust-SDL2/rust-sdl2/blob/master/src/sdl2/pixels.rs#L202 , add those values and then see where PixelFormatEnum is used in the code exactly. I don't think you'll have to change much, but I might forget something here, so let's be extra careful and see where all those variants are used in the code.

@Rua
Copy link
Contributor Author

Rua commented Dec 17, 2018

They're available to users of SDL and are documented: https://wiki.libsdl.org/SDL_PixelFormatEnum. We must assume that users of Rust SDL will want to use them, and I'm one of those users.

What is different is that they don't have unique enum values, but are simply aliases of existing formats and have the same enum value as the other format. Which formats they are aliases of differs depending on the endianness. RGBA32 is the same as RGBA8888 on a big-endian platform, but the same as ABGR8888 on a little-endian platform.

This is a bit of a problem for Rust, because enum values must be unique. Adding these new enum variants, and assigning them a number that's the same as another variant, will result in an error. I did a quick search and found that associated constants may work. Something like this:

#[cfg(target_endian = "big")]
impl PixelFormatEnum {
    pub const RGBA32: PixelFormatEnum = PixelFormatEnum::RGBA8888;
}

I can't say if this works exactly as an equivalent to a true enum variant, but it's probably better than nothing.

@Cobrand
Copy link
Member

Cobrand commented Dec 17, 2018

I understand now, the true problem comes from the fact that 2 enum variants can't have the same value. Makes sense. I'm fine with your proposal of the associated const, just make sure that nightly is not required to use associated consts (I'm not sure if it has been stabilized, but it was in nightly at some point for sure). In any case, if that doesn't work, the alternative would be to have a static method in the impl rgba32() -> i32 { ... } or something.

@Rua
Copy link
Contributor Author

Rua commented Dec 17, 2018

Associated constants are stable since Rust 1.20: https://rust-lang-nursery.github.io/edition-guide/rust-2018/trait-system/associated-constants.html.

@mnmaita
Copy link
Contributor

mnmaita commented Oct 2, 2019

@Cobrand Looks like #827 solved the issue.

@Cobrand Cobrand closed this as completed Oct 2, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants