[libcamera-devel] libcamera: geometry: Provide in-place versions of the Size helpers
diff mbox series

Message ID 20200715102339.14025-1-laurent.pinchart@ideasonboard.com
State Accepted
Commit d5446e9f327ab4eaef38249b0907f11fbf52be45
Headers show
Series
  • [libcamera-devel] libcamera: geometry: Provide in-place versions of the Size helpers
Related show

Commit Message

Laurent Pinchart July 15, 2020, 10:23 a.m. UTC 
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@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(+)

Comments

Jacopo Mondi July 15, 2020, 12:12 p.m. UTC | #1 
Hi Laurent,

On Wed, Jul 15, 2020 at 01:23:39PM +0300, 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@ideasonboard.com>

Great!

Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>

Thanks
  j

> ---
> 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,
>
> Laurent Pinchart
>
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel@lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel
Kieran Bingham July 15, 2020, 2:33 p.m. UTC | #2 
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@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@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) ||
>

Patch
diff mbox series 

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) ||