[PATCH v3 01/17] ipa: libipa: vector: Add mutable x(), y() and z() accessors

[Barnabás Pőcze]

Hi


2024. november 18., hétfő 23:16 keltezéssel, Laurent Pinchart <laurent.pinchart at ideasonboard.com> írta:

> The x(), y() and z() functions of the Vector class are convenience
> accessors for the first, second and third element of the vector
> respectively, meant to improve readability of class users when a vector
> represents coordinates in 1D, 2D or 3D space. Those accessors are
> limited to immutable access to the vector elements, as they return a
> copy. Extend the API with mutable accessors.
> 
> The immutable accessors are modified to return a reference to the vector
> elements instead of a copy for consistency. As they are inline
> functions, this should make no difference in terms of performance as the
> compiler can perform the same optimizations in their case.
> 
> While at it, reorder functions to declare operators before other member
> functions, to be consistent with the usual coding style.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> Reviewed-by: Milan Zamazal <mzamazal at redhat.com>
> ---
>  src/ipa/libipa/vector.cpp | 51 +++++++++++++++++++++++++--------------
>  src/ipa/libipa/vector.h   | 49 +++++++++++++++++++------------------
>  2 files changed, 58 insertions(+), 42 deletions(-)
> 
> diff --git a/src/ipa/libipa/vector.cpp b/src/ipa/libipa/vector.cpp
> index bd00b01961d5..8a39bfd27f90 100644
> --- a/src/ipa/libipa/vector.cpp
> +++ b/src/ipa/libipa/vector.cpp
> @@ -52,24 +52,6 @@ namespace ipa {
>   * \copydoc Vector::operator[](size_t i) const
>   */
> 
> -/**
> - * \fn Vector::x()
> - * \brief Convenience function to access the first element of the vector
> - * \return The first element of the vector
> - */
> -
> -/**
> - * \fn Vector::y()
> - * \brief Convenience function to access the second element of the vector
> - * \return The second element of the vector
> - */
> -
> -/**
> - * \fn Vector::z()
> - * \brief Convenience function to access the third element of the vector
> - * \return The third element of the vector
> - */
> -
>  /**
>   * \fn Vector::operator-() const
>   * \brief Negate a Vector by negating both all of its coordinates
> @@ -111,6 +93,39 @@ namespace ipa {
>   * \return The vector divided by \a factor
>   */
> 
> +/**
> + * \fn T &Vector::x()
> + * \brief Convenience function to access the first element of the vector
> + * \return The first element of the vector
> + */
> +
> +/**
> + * \fn T &Vector::y()
> + * \brief Convenience function to access the second element of the vector
> + * \return The second element of the vector
> + */
> +
> +/**
> + * \fn T &Vector::z()
> + * \brief Convenience function to access the third element of the vector
> + * \return The third element of the vector
> + */
> +
> +/**
> + * \fn constexpr const T &Vector::x() const
> + * \copydoc Vector::x()
> + */
> +
> +/**
> + * \fn constexpr const T &Vector::y() const
> + * \copydoc Vector::y()
> + */
> +
> +/**
> + * \fn constexpr const T &Vector::z() const
> + * \copydoc Vector::z()
> + */
> +
>  /**
>   * \fn Vector::length2()
>   * \brief Get the squared length of the vector
> diff --git a/src/ipa/libipa/vector.h b/src/ipa/libipa/vector.h
> index 8612a06a2ab2..1b11a34deee4 100644
> --- a/src/ipa/libipa/vector.h
> +++ b/src/ipa/libipa/vector.h
> @@ -53,30 +53,6 @@ public:
>  		return data_[i];
>  	}
> 
> -#ifndef __DOXYGEN__
> -	template<bool Dependent = false, typename = std::enable_if_t<Dependent || Rows >= 1>>
> -#endif /* __DOXYGEN__ */
> -	constexpr T x() const
> -	{
> -		return data_[0];
> -	}
> -
> -#ifndef __DOXYGEN__
> -	template<bool Dependent = false, typename = std::enable_if_t<Dependent || Rows >= 2>>
> -#endif /* __DOXYGEN__ */
> -	constexpr T y() const
> -	{
> -		return data_[1];
> -	}
> -
> -#ifndef __DOXYGEN__
> -	template<bool Dependent = false, typename = std::enable_if_t<Dependent || Rows >= 3>>
> -#endif /* __DOXYGEN__ */
> -	constexpr T z() const
> -	{
> -		return data_[2];
> -	}
> -
>  	constexpr Vector<T, Rows> operator-() const
>  	{
>  		Vector<T, Rows> ret;
> @@ -125,6 +101,31 @@ public:
>  		return ret;
>  	}
> 
> +#ifndef __DOXYGEN__
> +	template<bool Dependent = false, typename = std::enable_if_t<Dependent || Rows >= 1>>
> +#endif /* __DOXYGEN__ */
> +	constexpr const T &x() const { return data_[0]; }
> +#ifndef __DOXYGEN__
> +	template<bool Dependent = false, typename = std::enable_if_t<Dependent || Rows >= 2>>
> +#endif /* __DOXYGEN__ */
> +	constexpr const T &y() const { return data_[1]; }
> +#ifndef __DOXYGEN__
> +	template<bool Dependent = false, typename = std::enable_if_t<Dependent || Rows >= 3>>
> +#endif /* __DOXYGEN__ */
> +	constexpr const T &z() const { return data_[2]; }
> +#ifndef __DOXYGEN__
> +	template<bool Dependent = false, typename = std::enable_if_t<Dependent || Rows >= 1>>
> +#endif /* __DOXYGEN__ */
> +	T &x() { return data_[0]; }
> +#ifndef __DOXYGEN__
> +	template<bool Dependent = false, typename = std::enable_if_t<Dependent || Rows >= 2>>
> +#endif /* __DOXYGEN__ */
> +	T &y() { return data_[1]; }
> +#ifndef __DOXYGEN__
> +	template<bool Dependent = false, typename = std::enable_if_t<Dependent || Rows >= 3>>
> +#endif /* __DOXYGEN__ */
> +	T &z() { return data_[2]; }

I see no reason not to make the non-const qualified versions `constexpr` as well.
Same with r(), g(), b() in the next patch.


Regards,
Barnabás Pőcze

> +
>  	constexpr double length2() const
>  	{
>  		double ret = 0;
> --
> Regards,
> 
> Laurent Pinchart