From f7f84f4c20025cb6fc20d54caed62618f08c2e7b Mon Sep 17 00:00:00 2001
From: June Kim <kimjune01@gmail.com>
Date: Sat, 9 May 2026 10:26:08 -0700
Subject: [PATCH 1/2] fix: eliminate UB in read_pixels_of_type via
 bytemuck::cast_slice

The previous implementation used Vec::from_raw_parts to reinterpret a
Vec<u8> as Vec<T>, then mem::forget to suppress the original Vec's
destructor. This was UB for two reasons:
1. mem::forget receives an invalid value (already-transferred ownership)
2. Vec<T> deallocates with align_of::<T>(), but the memory was allocated
   with align_of::<u8>() = 1, violating alloc/dealloc layout invariants

Replace with bytemuck::cast_slice + to_vec(), which safely casts the
byte slice and copies into a properly-aligned Vec<T>. The Pod bound
already guarantees valid bit patterns for any byte sequence.

Closes #827
---
 fyrox-graphics/src/framebuffer.rs | 15 +++++----------
 1 file changed, 5 insertions(+), 10 deletions(-)

diff --git a/fyrox-graphics/src/framebuffer.rs b/fyrox-graphics/src/framebuffer.rs
index ee8dc3a93..686d23d36 100644
--- a/fyrox-graphics/src/framebuffer.rs
+++ b/fyrox-graphics/src/framebuffer.rs
@@ -328,16 +328,11 @@ impl dyn GpuFrameBufferTrait {
     where
         T: Pod,
     {
-        let mut bytes = self.read_pixels(read_target)?;
-        let typed = unsafe {
-            Vec::<T>::from_raw_parts(
-                bytes.as_mut_ptr() as *mut T,
-                bytes.len() / size_of::<T>(),
-                bytes.capacity() / size_of::<T>(),
-            )
-        };
-        std::mem::forget(bytes);
-        Some(typed)
+        let bytes = self.read_pixels(read_target)?;
+        // Use bytemuck for a safe, correctly-aligned conversion. This copies into a new
+        // Vec<T> with proper alignment, avoiding the UB from reinterpreting a Vec<u8>
+        // allocation as Vec<T> (which would deallocate with the wrong layout).
+        Some(bytemuck::cast_slice::<u8, T>(&bytes).to_vec())
     }
 }
 

From 1d269600123bc1576f2ea2f7ef952795ef392ce8 Mon Sep 17 00:00:00 2001
From: June Kim <kimjune01@gmail.com>
Date: Wed, 13 May 2026 17:32:27 -0700
Subject: [PATCH 2/2] fix: zero-copy TypedPixels<T> wrapper preserves original
 byte layout

Per @mrDIMAS review: avoid the .to_vec() copy by introducing a
TypedPixels<T> proxy struct that owns the original Vec<u8> (preserving
its allocation layout for correct deallocation) and exposes &[T] via
Deref + bytemuck::cast_slice. Mirrors the BytesStorage pattern used in
fyrox-impl/src/scene/mesh/buffer.rs.

The single caller (HDR luminance histogram) uses &pixels as a slice,
which works unchanged via deref coercion.
---
 fyrox-graphics/src/framebuffer.rs | 56 ++++++++++++++++++++++++++++---
 1 file changed, 51 insertions(+), 5 deletions(-)

diff --git a/fyrox-graphics/src/framebuffer.rs b/fyrox-graphics/src/framebuffer.rs
index 686d23d36..f9fafd3cc 100644
--- a/fyrox-graphics/src/framebuffer.rs
+++ b/fyrox-graphics/src/framebuffer.rs
@@ -322,17 +322,63 @@ pub trait GpuFrameBufferTrait: GpuFrameBufferAsAny {
     ) -> Result<DrawCallStatistics, FrameworkError>;
 }
 
+/// Owned, typed view of pixels read back from a [`GpuFrameBufferTrait`]. Preserves the
+/// original byte allocation (and therefore its memory layout) so it is dropped with the
+/// same `Layout` it was allocated with, while exposing a `&[T]` view of the contents.
+///
+/// This avoids the UB of reinterpreting a `Vec<u8>` allocation directly as `Vec<T>`
+/// (which would deallocate with `Layout::array::<T>(cap)` instead of the original
+/// `Layout::array::<u8>(cap)`) without copying the data.
+pub struct TypedPixels<T: Pod> {
+    bytes: Vec<u8>,
+    _marker: std::marker::PhantomData<T>,
+}
+
+impl<T: Pod> TypedPixels<T> {
+    /// Returns the pixels as a typed slice.
+    #[inline]
+    pub fn as_slice(&self) -> &[T] {
+        bytemuck::cast_slice(&self.bytes)
+    }
+
+    /// Number of `T` elements in the readback.
+    #[inline]
+    pub fn len(&self) -> usize {
+        self.bytes.len() / std::mem::size_of::<T>()
+    }
+
+    /// Whether the readback contains zero elements.
+    #[inline]
+    pub fn is_empty(&self) -> bool {
+        self.len() == 0
+    }
+}
+
+impl<T: Pod> std::ops::Deref for TypedPixels<T> {
+    type Target = [T];
+
+    #[inline]
+    fn deref(&self) -> &[T] {
+        self.as_slice()
+    }
+}
+
 impl dyn GpuFrameBufferTrait {
     /// Reads the pixels and reinterprets them using the given type.
-    pub fn read_pixels_of_type<T>(&self, read_target: ReadTarget) -> Option<Vec<T>>
+    ///
+    /// Returns an owning [`TypedPixels<T>`] that derefs to `&[T]`. The underlying byte
+    /// buffer is preserved as-is, so it is freed with the same memory layout it was
+    /// allocated with (avoiding the UB of dropping a `Vec<u8>` allocation as a `Vec<T>`),
+    /// and no extra copy is made when constructing the typed view.
+    pub fn read_pixels_of_type<T>(&self, read_target: ReadTarget) -> Option<TypedPixels<T>>
     where
         T: Pod,
     {
         let bytes = self.read_pixels(read_target)?;
-        // Use bytemuck for a safe, correctly-aligned conversion. This copies into a new
-        // Vec<T> with proper alignment, avoiding the UB from reinterpreting a Vec<u8>
-        // allocation as Vec<T> (which would deallocate with the wrong layout).
-        Some(bytemuck::cast_slice::<u8, T>(&bytes).to_vec())
+        Some(TypedPixels {
+            bytes,
+            _marker: std::marker::PhantomData,
+        })
     }
 }