From efab5cab9f853df0d6b6c0ef2378c05bc7263a72 Mon Sep 17 00:00:00 2001
From: Karol Herbst <kherbst@redhat.com>
Date: Wed, 27 Nov 2024 16:54:46 +0100
Subject: [PATCH] rusticl/util: reimplement Properties over Vec of scalars

Tuples don't have a well defined layout, which might make it inefficient
to copy from.

Reviewed-by: @LingMan
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/32268>
---
 .../frontends/rusticl/util/properties.rs      | 52 +++++++++++--------
 1 file changed, 30 insertions(+), 22 deletions(-)

diff --git a/src/gallium/frontends/rusticl/util/properties.rs b/src/gallium/frontends/rusticl/util/properties.rs
index 59cfbbdd23d..b0262fada07 100644
--- a/src/gallium/frontends/rusticl/util/properties.rs
+++ b/src/gallium/frontends/rusticl/util/properties.rs
@@ -1,12 +1,14 @@
+#[derive(Default)]
 pub struct Properties<T> {
-    props: Vec<(T, T)>,
+    props: Vec<T>,
 }
 
-impl<T: Copy + Default> Properties<T> {
+/// This encapsulates a C property array, where the list is 0 terminated.
+impl<T> Properties<T> {
     #[allow(clippy::not_unsafe_ptr_arg_deref)]
     pub fn from_ptr_raw(mut p: *const T) -> Vec<T>
     where
-        T: PartialEq,
+        T: Copy + Default + PartialEq,
     {
         let mut res: Vec<T> = Vec::new();
 
@@ -24,29 +26,36 @@ impl<T: Copy + Default> Properties<T> {
         res
     }
 
-    #[allow(clippy::not_unsafe_ptr_arg_deref)]
+    /// Creates a Properties object copying from the supplied pointer.
+    ///
+    /// It returns `None` if any property is found twice.
+    ///
+    /// If `p` is null the saved list of properties will be empty. Otherwise it will be 0
+    /// terminated.
     pub fn from_ptr(mut p: *const T) -> Option<Self>
     where
-        T: PartialEq,
+        T: Copy + Default + PartialEq,
     {
         let mut res = Self::default();
-
         if !p.is_null() {
-            let mut k: Vec<T> = Vec::new();
-            let mut v: Vec<T> = Vec::new();
-
             unsafe {
                 while *p != T::default() {
-                    if k.contains(&*p) {
+                    // Property lists are expected to be small, so no point in using HashSet or
+                    // sorting.
+                    if res.props.contains(&*p) {
                         return None;
                     }
-                    k.push(*p);
-                    v.push(*p.add(1));
+
+                    res.props.push(*p);
+                    res.props.push(*p.add(1));
+
+                    // Advance by two as we read through a list of pairs.
                     p = p.add(2);
                 }
             }
 
-            res.props = k.iter().cloned().zip(v).collect();
+            // terminate the array
+            res.props.push(T::default());
         }
 
         Some(res)
@@ -54,21 +63,20 @@ impl<T: Copy + Default> Properties<T> {
 
     /// Returns true when there is no property available.
     pub fn is_empty(&self) -> bool {
-        self.props.is_empty()
+        self.props.len() <= 1
     }
 
     pub fn iter(&self) -> impl Iterator<Item = (&T, &T)> {
-        self.props.iter().map(|(k, v)| (k, v))
+        // TODO: use array_chuncks once stabilized
+        self.props
+            .chunks_exact(2)
+            .map(|elems| (&elems[0], &elems[1]))
     }
 
     /// Returns the amount of key/value pairs available.
     pub fn len(&self) -> usize {
-        self.props.len()
-    }
-}
-
-impl<T> Default for Properties<T> {
-    fn default() -> Self {
-        Self { props: Vec::new() }
+        // only valid lengths are all uneven numbers and 0, so division by 2 gives us always the
+        // correct result.
+        self.props.len() / 2
     }
 }