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 struct #4

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Add color struct #4

wants to merge 3 commits into from

Conversation

michidk
Copy link
Contributor

@michidk michidk commented Mar 19, 2023

No description provided.

src/color.rs Outdated Show resolved Hide resolved
Self::new(r, g, b, 1.0)
}

swizzle!(r, r, r);
Copy link
Owner

Choose a reason for hiding this comment

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

These swizzles will produce vectors instead of colors, we should review whether we want this

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah I think we should rework the swizzle marco to support a specified type

src/color.rs Show resolved Hide resolved
}

impl_op_ex!(+= |a: &mut Color, b: &Color| { a.r += b.r; a.g += b.g; a.b += b.b; a.a += b.a; });
impl_op_ex!(-= |a: &mut Color, b: &Color| { a.r -= b.r; a.g -= b.g; a.b -= b.b; a.a -= b.a; });
Copy link
Owner

Choose a reason for hiding this comment

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

What happens when one of the components gets <0 ? The field docs specify the components to be [0.0-1.0]

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Maybe we should allow all ranges, to keep it simple.

Copy link
Owner

Choose a reason for hiding this comment

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

This would probably be the best solution

src/color.rs Show resolved Hide resolved
/// Creates a Color from a hex value.
/// The format of `hex` is `0xBBGGRR`, the MSB is discarded.
/// The `a` component is always set to 1.0.
pub fn from_hex_bgr(hex: u32) -> Self {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

When the hell would you ever want to use this? I think the selling point of gfx_maths is, that it is being simple. If you really want to have sth like this (I don't see a single use case though), I would implement it via macro.

Copy link
Owner

Choose a reason for hiding this comment

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

These functions cover most of the commonly used rgb pixel formats. E.g. some image formats store their pixels in BBGGRR instead of RRGGBB. I don't want to leave these common use cases unaccounted for.

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