Skip to content

Commit

Permalink
Use LinearRgba in bevy_gizmos internals (#12128)
Browse files Browse the repository at this point in the history
# Objective

This PR arose as part of the migration process for `bevy_color`: see
#12056.

While examining how `bevy_gizmos` stores color types internally, I found
that rather than storing a `Color` internally, it actually stores a
`[f32;4]` for a linear RGB, type aliased to a `ColorItem`.

While we don't *have* to clean this up to complete the migration, now
that we have explicit strong typing for linear color types we should use
them rather than replicating this idea throughout the codebase.

## Solution

- Added `LinearRgba::NAN`, for when you want to do cursed rendering
things.
- Replaced the internal color representation in `bevy_gizmo`: this was
`ColorItem`, but is now `LinearRgba`.
- `LinearRgba` is now `Pod`, enabling us to use the same fast `bytemuck`
bit twiddling tricks that we were using before. This requires:

1. Forcing `LinearRgba` to be `repr(C)`
2. Implementing `Zeroable`, and unsafe trait which defines what the
struct looks like when all values are zero.
3. Implementing `Pod`, a marker trait with stringent safety requirements
that is required for "plain old data".

---------

Co-authored-by: Alice Cecile <[email protected]>
  • Loading branch information
alice-i-cecile and Alice Cecile authored Feb 26, 2024
1 parent abc6f98 commit 5860e01
Show file tree
Hide file tree
Showing 5 changed files with 58 additions and 16 deletions.
1 change: 1 addition & 0 deletions crates/bevy_color/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ bevy_math = { path = "../bevy_math", version = "0.14.0-dev" }
bevy_reflect = { path = "../bevy_reflect", version = "0.14.0-dev", features = [
"bevy",
] }
bytemuck = "1"
serde = "1.0"
thiserror = "1.0"
wgpu = { version = "0.19.1", default-features = false }
Expand Down
41 changes: 41 additions & 0 deletions crates/bevy_color/src/linear_rgba.rs
Original file line number Diff line number Diff line change
@@ -1,11 +1,13 @@
use crate::{color_difference::EuclideanDistance, Alpha, Luminance, Mix, StandardColor};
use bevy_math::Vec4;
use bevy_reflect::{Reflect, ReflectDeserialize, ReflectSerialize};
use bytemuck::{Pod, Zeroable};
use serde::{Deserialize, Serialize};

/// Linear RGB color with alpha.
#[derive(Debug, Clone, Copy, PartialEq, Serialize, Deserialize, Reflect)]
#[reflect(PartialEq, Serialize, Deserialize)]
#[repr(C)]
pub struct LinearRgba {
/// The red channel. [0.0, 1.0]
pub red: f32,
Expand Down Expand Up @@ -44,6 +46,18 @@ impl LinearRgba {
alpha: 0.0,
};

/// An invalid color.
///
/// This type can be used to represent an invalid color value;
/// in some rendering applications the color will be ignored,
/// enabling performant hacks like hiding lines by setting their color to `INVALID`.
pub const NAN: Self = Self {
red: f32::NAN,
green: f32::NAN,
blue: f32::NAN,
alpha: f32::NAN,
};

/// Construct a new [`LinearRgba`] color from components.
pub const fn new(red: f32, green: f32, blue: f32, alpha: f32) -> Self {
Self {
Expand Down Expand Up @@ -279,6 +293,33 @@ impl encase::private::CreateFrom for LinearRgba {
}
}

/// A [`Zeroable`] type is one whose bytes can be filled with zeroes while remaining valid.
///
/// SAFETY: [`LinearRgba`] is inhabited
/// SAFETY: [`LinearRgba`]'s all-zero bit pattern is a valid value
unsafe impl Zeroable for LinearRgba {
fn zeroed() -> Self {
LinearRgba {
red: 0.0,
green: 0.0,
blue: 0.0,
alpha: 0.0,
}
}
}

/// The [`Pod`] trait is [`bytemuck`]'s marker for types that can be safely transmuted from a byte array.
///
/// It is intended to only be implemented for types which are "Plain Old Data".
///
/// SAFETY: [`LinearRgba`] is inhabited.
/// SAFETY: [`LinearRgba`] permits any bit value.
/// SAFETY: [`LinearRgba`] does not have padding bytes.
/// SAFETY: all of the fields of [`LinearRgba`] are [`Pod`], as f32 is [`Pod`].
/// SAFETY: [`LinearRgba`] is `repr(C)`
/// SAFETY: [`LinearRgba`] does not permit interior mutability.
unsafe impl Pod for LinearRgba {}

impl encase::ShaderSize for LinearRgba {}

#[cfg(test)]
Expand Down
1 change: 1 addition & 0 deletions crates/bevy_gizmos/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ webgpu = []
bevy_pbr = { path = "../bevy_pbr", version = "0.14.0-dev", optional = true }
bevy_sprite = { path = "../bevy_sprite", version = "0.14.0-dev", optional = true }
bevy_app = { path = "../bevy_app", version = "0.14.0-dev" }
bevy_color = { path = "../bevy_color", version = "0.14.0-dev" }
bevy_ecs = { path = "../bevy_ecs", version = "0.14.0-dev" }
bevy_math = { path = "../bevy_math", version = "0.14.0-dev" }
bevy_asset = { path = "../bevy_asset", version = "0.14.0-dev" }
Expand Down
28 changes: 13 additions & 15 deletions crates/bevy_gizmos/src/gizmos.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
use std::{iter, marker::PhantomData};

use crate::circles::DEFAULT_CIRCLE_SEGMENTS;
use bevy_color::LinearRgba;
use bevy_ecs::{
component::Tick,
system::{Deferred, ReadOnlySystemParam, Res, Resource, SystemBuffer, SystemMeta, SystemParam},
Expand All @@ -19,14 +20,13 @@ use crate::{
};

type PositionItem = [f32; 3];
type ColorItem = [f32; 4];

#[derive(Resource, Default)]
pub(crate) struct GizmoStorage<T: GizmoConfigGroup> {
pub list_positions: Vec<PositionItem>,
pub list_colors: Vec<ColorItem>,
pub strip_positions: Vec<PositionItem>,
pub strip_colors: Vec<ColorItem>,
pub(crate) list_positions: Vec<PositionItem>,
pub(crate) list_colors: Vec<LinearRgba>,
pub(crate) strip_positions: Vec<PositionItem>,
pub(crate) strip_colors: Vec<LinearRgba>,
marker: PhantomData<T>,
}

Expand Down Expand Up @@ -104,9 +104,9 @@ where
#[derive(Default)]
struct GizmoBuffer<T: GizmoConfigGroup> {
list_positions: Vec<PositionItem>,
list_colors: Vec<ColorItem>,
list_colors: Vec<LinearRgba>,
strip_positions: Vec<PositionItem>,
strip_colors: Vec<ColorItem>,
strip_colors: Vec<LinearRgba>,
marker: PhantomData<T>,
}

Expand Down Expand Up @@ -244,10 +244,8 @@ impl<'w, 's, T: GizmoConfigGroup> Gizmos<'w, 's, T> {
}
self.extend_strip_positions(positions);
let len = self.buffer.strip_positions.len();
self.buffer
.strip_colors
.resize(len - 1, color.as_linear_rgba_f32());
self.buffer.strip_colors.push([f32::NAN; 4]);
self.buffer.strip_colors.resize(len - 1, color.into());
self.buffer.strip_colors.push(LinearRgba::NAN);
}

/// Draw a line in 3D made of straight segments between the points, with a color gradient.
Expand Down Expand Up @@ -287,11 +285,11 @@ impl<'w, 's, T: GizmoConfigGroup> Gizmos<'w, 's, T> {

for (position, color) in points {
strip_positions.push(position.to_array());
strip_colors.push(color.as_linear_rgba_f32());
strip_colors.push(color.into());
}

strip_positions.push([f32::NAN; 3]);
strip_colors.push([f32::NAN; 4]);
strip_colors.push(LinearRgba::NAN);
}

/// Draw a wireframe sphere in 3D made out of 3 circles around the axes.
Expand Down Expand Up @@ -583,14 +581,14 @@ impl<'w, 's, T: GizmoConfigGroup> Gizmos<'w, 's, T> {
fn extend_list_colors(&mut self, colors: impl IntoIterator<Item = LegacyColor>) {
self.buffer
.list_colors
.extend(colors.into_iter().map(|color| color.as_linear_rgba_f32()));
.extend(colors.into_iter().map(LinearRgba::from));
}

#[inline]
fn add_list_color(&mut self, color: LegacyColor, count: usize) {
self.buffer
.list_colors
.extend(iter::repeat(color.as_linear_rgba_f32()).take(count));
.extend(iter::repeat(LinearRgba::from(color)).take(count));
}

#[inline]
Expand Down
3 changes: 2 additions & 1 deletion crates/bevy_gizmos/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ pub mod prelude {
use aabb::AabbGizmoPlugin;
use bevy_app::{App, Last, Plugin};
use bevy_asset::{load_internal_asset, Asset, AssetApp, Assets, Handle};
use bevy_color::LinearRgba;
use bevy_core::cast_slice;
use bevy_ecs::{
component::Component,
Expand Down Expand Up @@ -309,7 +310,7 @@ struct LineGizmoUniform {
#[derive(Asset, Debug, Default, Clone, TypePath)]
struct LineGizmo {
positions: Vec<[f32; 3]>,
colors: Vec<[f32; 4]>,
colors: Vec<LinearRgba>,
/// Whether this gizmo's topology is a line-strip or line-list
strip: bool,
}
Expand Down

0 comments on commit 5860e01

Please sign in to comment.