Skip to content

refactor!: make PlaneGeometry fields private #75

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.

Sign up for GitHub

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

Merged
shssoichiro merged 2 commits into from
May 8, 2026
Merged

refactor!: make PlaneGeometry fields private #75

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
b8ff435
refactor!: make PlaneGeometry fields private
FreezyLemon May 7, 2026
d7140e4
test: use getters in geometry tests
FreezyLemon May 7, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions src/frame.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,13 +52,13 @@
//! .unwrap();
//!
//! // Access the planes
//! assert_eq!(frame.y_plane.width().get(), 1920);
//! assert_eq!(frame.y_plane.height().get(), 1080);
//! assert_eq!(frame.y_plane.width(), 1920);
//! assert_eq!(frame.y_plane.height(), 1080);
//!
//! // Chroma planes are half size for YUV420
//! let u_plane = frame.u_plane.as_ref().unwrap();
//! assert_eq!(u_plane.width().get(), 960);
//! assert_eq!(u_plane.height().get(), 540);
//! assert_eq!(u_plane.width(), 960);
//! assert_eq!(u_plane.height(), 540);
//! ```
//!
//! # Creating Frames with Padding
Expand Down
58 changes: 29 additions & 29 deletions src/frame/tests.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,8 +56,8 @@ fn basic_8bit_frame() {
.build::<u8>()
.unwrap();

assert_eq!(frame.y_plane.width().get(), 1920);
assert_eq!(frame.y_plane.height().get(), 1080);
assert_eq!(frame.y_plane.width(), 1920);
assert_eq!(frame.y_plane.height(), 1080);
assert_eq!(frame.bit_depth.get(), 8);
assert_eq!(frame.subsampling, ChromaSubsampling::Yuv420);
}
Expand All @@ -68,8 +68,8 @@ fn basic_10bit_frame() {
.build::<u16>()
.unwrap();

assert_eq!(frame.y_plane.width().get(), 3840);
assert_eq!(frame.y_plane.height().get(), 2160);
assert_eq!(frame.y_plane.width(), 3840);
assert_eq!(frame.y_plane.height(), 2160);
assert_eq!(frame.bit_depth.get(), 10);
}

Expand All @@ -83,10 +83,10 @@ fn yuv420_chroma_dimensions() {
let v_plane = frame.v_plane.as_ref().unwrap();

// YUV420 has chroma planes at half width and half height
assert_eq!(u_plane.width().get(), 960);
assert_eq!(u_plane.height().get(), 540);
assert_eq!(v_plane.width().get(), 960);
assert_eq!(v_plane.height().get(), 540);
assert_eq!(u_plane.width(), 960);
assert_eq!(u_plane.height(), 540);
assert_eq!(v_plane.width(), 960);
assert_eq!(v_plane.height(), 540);
}

#[test]
Expand All @@ -99,10 +99,10 @@ fn yuv422_chroma_dimensions() {
let v_plane = frame.v_plane.as_ref().unwrap();

// YUV422 has chroma planes at half width and full height
assert_eq!(u_plane.width().get(), 960);
assert_eq!(u_plane.height().get(), 1080);
assert_eq!(v_plane.width().get(), 960);
assert_eq!(v_plane.height().get(), 1080);
assert_eq!(u_plane.width(), 960);
assert_eq!(u_plane.height(), 1080);
assert_eq!(v_plane.width(), 960);
assert_eq!(v_plane.height(), 1080);
}

#[test]
Expand All @@ -115,10 +115,10 @@ fn yuv444_chroma_dimensions() {
let v_plane = frame.v_plane.as_ref().unwrap();

// YUV444 has chroma planes at full resolution
assert_eq!(u_plane.width().get(), 1920);
assert_eq!(u_plane.height().get(), 1080);
assert_eq!(v_plane.width().get(), 1920);
assert_eq!(v_plane.height().get(), 1080);
assert_eq!(u_plane.width(), 1920);
assert_eq!(u_plane.height(), 1080);
assert_eq!(v_plane.width(), 1920);
assert_eq!(v_plane.height(), 1080);
}

#[test]
Expand All @@ -127,8 +127,8 @@ fn monochrome_no_chroma_planes() {
.build::<u8>()
.unwrap();

assert_eq!(frame.y_plane.width().get(), 1920);
assert_eq!(frame.y_plane.height().get(), 1080);
assert_eq!(frame.y_plane.width(), 1920);
assert_eq!(frame.y_plane.height(), 1080);
assert!(frame.u_plane.is_none());
assert!(frame.v_plane.is_none());
assert_eq!(frame.subsampling, ChromaSubsampling::Monochrome);
Expand Down Expand Up @@ -217,8 +217,8 @@ fn frame_with_luma_padding() {
.unwrap();

// Visible dimensions should remain unchanged
assert_eq!(frame.y_plane.width().get(), 1920);
assert_eq!(frame.y_plane.height().get(), 1080);
assert_eq!(frame.y_plane.width(), 1920);
assert_eq!(frame.y_plane.height(), 1080);
}

#[test]
Expand All @@ -233,8 +233,8 @@ fn chroma_padding_derived_from_luma_yuv420() {

// For YUV420, chroma padding should be half of luma padding
let u_plane = frame.u_plane.as_ref().unwrap();
assert_eq!(u_plane.width().get(), 960); // chroma width
assert_eq!(u_plane.height().get(), 540); // chroma height
assert_eq!(u_plane.width(), 960); // chroma width
assert_eq!(u_plane.height(), 540); // chroma height
}

#[test]
Expand Down Expand Up @@ -321,12 +321,12 @@ fn small_resolution() {
.build::<u8>()
.unwrap();

assert_eq!(frame.y_plane.width().get(), 2);
assert_eq!(frame.y_plane.height().get(), 2);
assert_eq!(frame.y_plane.width(), 2);
assert_eq!(frame.y_plane.height(), 2);

let u_plane = frame.u_plane.as_ref().unwrap();
assert_eq!(u_plane.width().get(), 1);
assert_eq!(u_plane.height().get(), 1);
assert_eq!(u_plane.width(), 1);
assert_eq!(u_plane.height(), 1);
}

#[test]
Expand All @@ -338,7 +338,7 @@ fn builder_setters() {
.luma_padding_bottom(8)
.build::<u8>()
.unwrap();
assert!(frame.y_plane.width().get() == 1920);
assert!(frame.y_plane.width() == 1920);
}

#[test]
Expand All @@ -352,6 +352,6 @@ fn asymmetric_padding() {
.unwrap();

// Visible dimensions should remain unchanged
assert_eq!(frame.y_plane.width().get(), 1920);
assert_eq!(frame.y_plane.height().get(), 1080);
assert_eq!(frame.y_plane.width(), 1920);
assert_eq!(frame.y_plane.height(), 1080);
}
83 changes: 33 additions & 50 deletions src/plane.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,6 @@ mod tests;

#[cfg(feature = "padding_api")]
use std::mem::MaybeUninit;
use std::num::NonZeroUsize;

mod aligned;
use aligned::AlignedData;
Expand Down Expand Up @@ -99,28 +98,22 @@ pub struct Plane<T> {
}

impl<T> Plane<T> {
/// Returns the visible width of the plane in pixels
#[inline]
#[must_use]
pub fn width(&self) -> NonZeroUsize {
self.geometry.width
}

/// Returns the visible height of the plane in pixels
/// Returns the visible width of the plane in pixels.
///
/// Guaranteed to be non-zero.
#[inline]
#[must_use]
pub fn height(&self) -> NonZeroUsize {
self.geometry.height
pub fn width(&self) -> usize {
self.geometry.width()
}

/// Returns the index for the first visible pixel in `data`.
/// Returns the visible height of the plane in pixels.
///
/// This is a low-level API intended only for functions that require access to the padding.
/// Guaranteed to be non-zero.
#[inline]
#[must_use]
#[cfg_attr(not(feature = "padding_api"), doc(hidden))]
pub fn data_origin(&self) -> usize {
self.geometry.stride.get() * self.geometry.pad_top + self.geometry.pad_left
pub fn height(&self) -> usize {
self.geometry.height()
}
}

Expand Down Expand Up @@ -168,12 +161,7 @@ impl<T> Plane<T> {
#[inline]
#[must_use]
pub fn new_uninit(geometry: PlaneGeometry) -> Plane<MaybeUninit<T>> {
let geometry = geometry
.normalized()
.expect("plane geometry dimensions must not overflow");
let pixels = geometry
.allocation_len()
.expect("plane allocation size must not overflow usize");
let pixels = geometry.alloc_size();

Plane {
data: AlignedData::new_uninit(pixels),
Expand Down Expand Up @@ -232,12 +220,7 @@ impl<T> Plane<MaybeUninit<T>> {
impl<T: Pixel> Plane<T> {
/// Creates a new plane with the given geometry, initialized with zero-valued pixels.
pub(crate) fn new(geometry: PlaneGeometry) -> Self {
let geometry = geometry
.normalized()
.expect("plane geometry dimensions must not overflow");
let pixels = geometry
.allocation_len()
.expect("plane allocation size must not overflow usize");
let pixels = geometry.alloc_size();

Self {
data: AlignedData::new(pixels),
Expand Down Expand Up @@ -267,12 +250,12 @@ impl<T: Pixel> Plane<T> {
#[must_use]
pub fn rows(&self) -> impl DoubleEndedIterator<Item = &[T]> + ExactSizeIterator {
self.data
.chunks_exact(self.geometry.stride.get())
.skip(self.geometry.pad_top)
.take(self.geometry.height.get())
.chunks_exact(self.geometry.stride())
.skip(self.geometry.pad_top())
.take(self.geometry.height())
.map(|row| {
let start_idx = self.geometry.pad_left;
let end_idx = start_idx + self.geometry.width.get();
let start_idx = self.geometry.pad_left();
let end_idx = start_idx + self.geometry.width();
// SAFETY: The plane creation interface ensures the data is large enough
unsafe { row.get_unchecked(start_idx..end_idx) }
})
Expand All @@ -283,12 +266,12 @@ impl<T: Pixel> Plane<T> {
#[inline]
pub fn rows_mut(&mut self) -> impl DoubleEndedIterator<Item = &mut [T]> + ExactSizeIterator {
self.data
.chunks_exact_mut(self.geometry.stride.get())
.skip(self.geometry.pad_top)
.take(self.geometry.height.get())
.chunks_exact_mut(self.geometry.stride())
.skip(self.geometry.pad_top())
.take(self.geometry.height())
.map(|row| {
let start_idx = self.geometry.pad_left;
let end_idx = start_idx + self.geometry.width.get();
let start_idx = self.geometry.pad_left();
let end_idx = start_idx + self.geometry.width();
// SAFETY: The plane creation interface ensures the data is large enough
unsafe { row.get_unchecked_mut(start_idx..end_idx) }
})
Expand All @@ -302,7 +285,7 @@ impl<T: Pixel> Plane<T> {
#[inline]
#[must_use]
pub fn pixel(&self, x: usize, y: usize) -> Option<T> {
let index = self.data_origin() + self.geometry.stride.get() * y + x;
let index = self.geometry.data_origin() + self.geometry.stride() * y + x;
self.data.get(index).copied()
}

Expand All @@ -313,7 +296,7 @@ impl<T: Pixel> Plane<T> {
/// and should not be used to iterate over rows and pixels.
#[inline]
pub fn pixel_mut(&mut self, x: usize, y: usize) -> Option<&mut T> {
let index = self.data_origin() + self.geometry.stride.get() * y + x;
let index = self.geometry.data_origin() + self.geometry.stride() * y + x;
self.data.get_mut(index)
}

Expand All @@ -322,7 +305,7 @@ impl<T: Pixel> Plane<T> {
#[inline]
#[must_use]
pub fn pixels(&self) -> impl DoubleEndedIterator<Item = T> + ExactSizeIterator {
let total = self.width().get() * self.height().get();
let total = self.width() * self.height();
ExactSizeWrapper {
iter: self.rows().flatten().copied(),
len: total,
Expand All @@ -333,7 +316,7 @@ impl<T: Pixel> Plane<T> {
/// in row-major order.
#[inline]
pub fn pixels_mut(&mut self) -> impl DoubleEndedIterator<Item = &mut T> + ExactSizeIterator {
let total = self.width().get() * self.height().get();
let total = self.width() * self.height();
ExactSizeWrapper {
iter: self.rows_mut().flatten(),
len: total,
Expand All @@ -352,7 +335,7 @@ impl<T: Pixel> Plane<T> {
"unsupported pixel byte width: {byte_width}"
);

let total = self.width().get() * self.height().get() * byte_width;
let total = self.width() * self.height() * byte_width;
ExactSizeWrapper {
iter: self.pixels().flat_map(move |pix| {
let bytes: [u8; 2] = if byte_width == 1 {
Expand All @@ -379,8 +362,8 @@ impl<T: Pixel> Plane<T> {
/// this plane's `width * height`
#[inline]
pub fn copy_from_slice(&mut self, src: &[T]) -> Result<(), CopyError> {
let width = self.width().get();
let pixel_count = width * self.height().get();
let width = self.width();
let pixel_count = width * self.height();
if pixel_count != src.len() {
return Err(CopyError::DataLength {
expected: pixel_count,
Expand All @@ -407,7 +390,7 @@ impl<T: Pixel> Plane<T> {
/// this plane's `width * height * bytes_per_pixel`
#[inline]
pub fn copy_from_u8_slice(&mut self, src: &[u8]) -> Result<(), CopyError> {
self.copy_from_u8_slice_with_stride(src, self.width().get() * size_of::<T>())
self.copy_from_u8_slice_with_stride(src, self.width() * size_of::<T>())
}

/// Copies the data from `src` into this plane's visible pixels.
Expand All @@ -430,22 +413,22 @@ impl<T: Pixel> Plane<T> {
"unsupported pixel byte width: {byte_width}"
);

if stride < self.width().get() {
if stride < self.width() {
return Err(CopyError::InvalidStride {
stride,
width: self.width().get(),
width: self.width(),
});
}

let byte_count = stride * self.height().get();
let byte_count = stride * self.height();
if byte_count != src.len() {
return Err(CopyError::DataLength {
expected: byte_count,
found: src.len(),
});
}

let width = self.width().get();
let width = self.width();
if byte_width == 1 {
// Fast path for u8 pixels
for (row_idx, dest_row) in self.rows_mut().enumerate() {
Expand Down
Loading