[libcamera-devel] [PATCH] libcamera: geometry: Provide in-place versions of the Size helpers

[Kieran Bingham]

Hi Laurent,

On 15/07/2020 11:23, Laurent Pinchart wrote:
> Add alignDownTo(), alignUpTo(), boundTo() and expandTo() helper
> functions to the Size class. These are in-place versions of the existing
> alignedDownTo(), alignedUpTo(), boundedTo() and expandedTo() functions.
> 
> The new helpers return a reference to the size, to allow chaining the
> functions. One can thus write
> 
> 	size.alignDownTo(16, 16).alignUpTo(32, 32)
> 	    .boundTo({ 40, 80 }).expandTo({ 16, 80 });
> 
> instead of
> 
> 	size.alignDownTo(16, 16);
> 	size.alignUpTo(32, 32);
> 	size.boundTo({ 40, 80 });
> 	size.expandTo({ 16, 80 });
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>

I do like the chaining in this case.

I actually have a local patch in an old branch that added chaning to the
DeviceMatch dm.add("A").add("B") as an experiment, but I wasn't sure it
was much benefit, as really it ended up being:

dm.add("a")
  .add("b");

instead of

dm.add("a");
dm.add("b);

But seeing this patch makes me rethink it, and maybe I'll post it for
fun (if it still applies).

Anyway, I digress, for this patch,

Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com>




> ---
> This patch applies on top of the previously submitted geometry series.
> ---
>  include/libcamera/geometry.h | 28 ++++++++++++++++++++++
>  src/libcamera/geometry.cpp   | 46 ++++++++++++++++++++++++++++++++++++
>  test/geometry.cpp            | 34 ++++++++++++++++++++++++++
>  3 files changed, 108 insertions(+)
> 
> diff --git a/include/libcamera/geometry.h b/include/libcamera/geometry.h
> index 30aaa7a30fe2..d858f85cf1f0 100644
> --- a/include/libcamera/geometry.h
> +++ b/include/libcamera/geometry.h
> @@ -32,6 +32,34 @@ public:
>  	bool isNull() const { return !width && !height; }
>  	const std::string toString() const;
>  
> +	Size &alignDownTo(unsigned int hAlignment, unsigned int vAlignment)
> +	{
> +		width = width / hAlignment * hAlignment;
> +		height = height / vAlignment * vAlignment;
> +		return *this;
> +	}
> +
> +	Size &alignUpTo(unsigned int hAlignment, unsigned int vAlignment)
> +	{
> +		width = (width + hAlignment - 1) / hAlignment * hAlignment;
> +		height = (height + vAlignment - 1) / vAlignment * vAlignment;
> +		return *this;
> +	}
> +
> +	Size &boundTo(const Size &bound)
> +	{
> +		width = std::min(width, bound.width);
> +		height = std::min(height, bound.height);
> +		return *this;
> +	}
> +
> +	Size &expandTo(const Size &expand)
> +	{
> +		width = std::max(width, expand.width);
> +		height = std::max(height, expand.height);
> +		return *this;
> +	}
> +
>  	constexpr Size alignedDownTo(unsigned int hAlignment,
>  				     unsigned int vAlignment) const
>  	{
> diff --git a/src/libcamera/geometry.cpp b/src/libcamera/geometry.cpp
> index 3a3784df39e6..23181bdec969 100644
> --- a/src/libcamera/geometry.cpp
> +++ b/src/libcamera/geometry.cpp
> @@ -61,6 +61,52 @@ const std::string Size::toString() const
>  	return std::to_string(width) + "x" + std::to_string(height);
>  }
>  
> +/**
> + * \fn Size::alignDownTo(unsigned int hAlignment, unsigned int vAlignment)
> + * \brief Align the size down horizontally and vertically in place
> + * \param[in] hAlignment Horizontal alignment
> + * \param[in] vAlignment Vertical alignment
> + *
> + * This functions rounds the width and height down to the nearest multiple of
> + * \a hAlignment and \a vAlignment respectively.
> + *
> + * \return A reference to this object
> + */
> +
> +/**
> + * \fn Size::alignUpTo(unsigned int hAlignment, unsigned int vAlignment)
> + * \brief Align the size up horizontally and vertically in place
> + * \param[in] hAlignment Horizontal alignment
> + * \param[in] vAlignment Vertical alignment
> + *
> + * This functions rounds the width and height up to the nearest multiple of
> + * \a hAlignment and \a vAlignment respectively.
> + *
> + * \return A reference to this object
> + */
> +
> +/**
> + * \fn Size::boundTo(const Size &bound)
> + * \brief Bound the size to \a bound in place
> + * \param[in] bound The maximum size
> + *
> + * This function sets the width and height to the minimum of this size and the
> + * \a bound size.
> + *
> + * \return A reference to this object
> + */
> +
> +/**
> + * \fn Size::expandTo(const Size &expand)
> + * \brief Expand the size to \a expand
> + * \param[in] expand The minimum size
> + *
> + * This function sets the width and height to the maximum of this size and the
> + * \a expand size.
> + *
> + * \return A reference to this object
> + */
> +
>  /**
>   * \fn Size::alignedDownTo(unsigned int hAlignment, unsigned int vAlignment)
>   * \brief Align the size down horizontally and vertically
> diff --git a/test/geometry.cpp b/test/geometry.cpp
> index fd0132c03b02..1a9fc1b8e3ed 100644
> --- a/test/geometry.cpp
> +++ b/test/geometry.cpp
> @@ -46,6 +46,40 @@ protected:
>  			return TestFail;
>  		}
>  
> +		/* Test alignDownTo(), alignUpTo(), boundTo() and expandTo() */
> +		Size s(50, 50);
> +
> +		s.alignDownTo(16, 16);
> +		if (s != Size(48, 48)) {
> +			cout << "Size::alignDownTo() test failed" << endl;
> +			return TestFail;
> +		}
> +
> +		s.alignUpTo(32, 32);
> +		if (s != Size(64, 64)) {
> +			cout << "Size::alignUpTo() test failed" << endl;
> +			return TestFail;
> +		}
> +
> +		s.boundTo({ 40, 40 });
> +		if (s != Size(40, 40)) {
> +			cout << "Size::boundTo() test failed" << endl;
> +			return TestFail;
> +		}
> +
> +		s.expandTo({ 50, 50 });
> +		if (s != Size(50, 50)) {
> +			cout << "Size::expandTo() test failed" << endl;
> +			return TestFail;
> +		}
> +
> +		s.alignDownTo(16, 16).alignUpTo(32, 32)
> +		 .boundTo({ 40, 80 }).expandTo({ 16, 80 });
> +		if (s != Size(40, 80)) {
> +			cout << "Size chained in-place modifiers test failed" << endl;
> +			return TestFail;
> +		}
> +
>  		/* Test alignedDownTo(), alignedUpTo(), boundedTo() and expandedTo() */
>  		if (Size(0, 0).alignedDownTo(16, 8) != Size(0, 0) ||
>  		    Size(1, 1).alignedDownTo(16, 8) != Size(0, 0) ||
> 

-- 
Regards
--
Kieran