[libcamera-devel,v5,03/19] libcamera: geometry: Add 0-initialized SizeRange constructor
diff mbox series

Message ID 20190326083902.26121-4-jacopo@jmondi.org
State Superseded
Headers show
Series
  • libcamera: ipu3: Add ImgU support
Related show

Commit Message

Jacopo Mondi March 26, 2019, 8:38 a.m. UTC 
Add constructor to SizeRange which initialize all the size range fields
to 0.

While at there make the in-line constructor declarations respect the
coding style by moving braces to a new line.

Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
---
 src/libcamera/geometry.cpp       | 11 ++++++++++-
 src/libcamera/include/geometry.h |  9 ++++++++-
 2 files changed, 18 insertions(+), 2 deletions(-)

Comments

Laurent Pinchart March 27, 2019, 12:13 a.m. UTC | #1 
Hi Jacopo,

Thank you for the patch.

On Tue, Mar 26, 2019 at 09:38:46AM +0100, Jacopo Mondi wrote:
> Add constructor to SizeRange which initialize all the size range fields
> to 0.
> 
> While at there make the in-line constructor declarations respect the
> coding style by moving braces to a new line.
> 
> Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
> ---
>  src/libcamera/geometry.cpp       | 11 ++++++++++-
>  src/libcamera/include/geometry.h |  9 ++++++++-
>  2 files changed, 18 insertions(+), 2 deletions(-)
> 
> diff --git a/src/libcamera/geometry.cpp b/src/libcamera/geometry.cpp
> index b6b6592bdfec..dbc37ca8e3f4 100644
> --- a/src/libcamera/geometry.cpp
> +++ b/src/libcamera/geometry.cpp
> @@ -57,7 +57,16 @@ namespace libcamera {
>  
>  /**
>   * \fn SizeRange::SizeRange()
> - * \brief Construct a size range
> + * \brief Construct a size range initialized to 0
> + */
> +
> +/**
> + * \fn SizeRange::SizeRange(unsigned int minW, unsigned int minH, unsigned int maxW, unsigned int maxH)
> + * \brief Construct an initialized size range
> + * \param minW The minimum width
> + * \param minH The minimum height
> + * \param maxW The maximum width
> + * \param maxH The maximum height
>   */
>  
>  /**
> diff --git a/src/libcamera/include/geometry.h b/src/libcamera/include/geometry.h
> index eadc4ed4f9cb..749746495204 100644
> --- a/src/libcamera/include/geometry.h
> +++ b/src/libcamera/include/geometry.h
> @@ -18,10 +18,17 @@ struct Rectangle {
>  };
>  
>  struct SizeRange {
> +	SizeRange(void)
> +		: SizeRange(0, 0, 0, 0)
> +	{
> +	}
> +
>  	SizeRange(unsigned int minW, unsigned int minH,
>  		  unsigned int maxW, unsigned int maxH)
>  		: minWidth(minW), minHeight(minH), maxWidth(maxW),
> -		  maxHeight(maxH) {}
> +		  maxHeight(maxH)
> +	{
> +	}
>  
>  	unsigned int minWidth;
>  	unsigned int minHeight;

Open question here, what to you think of
https://en.cppreference.com/w/cpp/language/data_members#Member_initialization,
item 2 instead of creating a constructor for this kind of purpose ?
Jacopo Mondi March 27, 2019, 8 a.m. UTC | #2 
Hi Laurent,

On Wed, Mar 27, 2019 at 02:13:35AM +0200, Laurent Pinchart wrote:
> Hi Jacopo,
>
> Thank you for the patch.
>
> On Tue, Mar 26, 2019 at 09:38:46AM +0100, Jacopo Mondi wrote:
> > Add constructor to SizeRange which initialize all the size range fields
> > to 0.
> >
> > While at there make the in-line constructor declarations respect the
> > coding style by moving braces to a new line.
> >
> > Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
> > ---
> >  src/libcamera/geometry.cpp       | 11 ++++++++++-
> >  src/libcamera/include/geometry.h |  9 ++++++++-
> >  2 files changed, 18 insertions(+), 2 deletions(-)
> >
> > diff --git a/src/libcamera/geometry.cpp b/src/libcamera/geometry.cpp
> > index b6b6592bdfec..dbc37ca8e3f4 100644
> > --- a/src/libcamera/geometry.cpp
> > +++ b/src/libcamera/geometry.cpp
> > @@ -57,7 +57,16 @@ namespace libcamera {
> >
> >  /**
> >   * \fn SizeRange::SizeRange()
> > - * \brief Construct a size range
> > + * \brief Construct a size range initialized to 0
> > + */
> > +
> > +/**
> > + * \fn SizeRange::SizeRange(unsigned int minW, unsigned int minH, unsigned int maxW, unsigned int maxH)
> > + * \brief Construct an initialized size range
> > + * \param minW The minimum width
> > + * \param minH The minimum height
> > + * \param maxW The maximum width
> > + * \param maxH The maximum height
> >   */
> >
> >  /**
> > diff --git a/src/libcamera/include/geometry.h b/src/libcamera/include/geometry.h
> > index eadc4ed4f9cb..749746495204 100644
> > --- a/src/libcamera/include/geometry.h
> > +++ b/src/libcamera/include/geometry.h
> > @@ -18,10 +18,17 @@ struct Rectangle {
> >  };
> >
> >  struct SizeRange {
> > +	SizeRange(void)
> > +		: SizeRange(0, 0, 0, 0)
> > +	{
> > +	}
> > +
> >  	SizeRange(unsigned int minW, unsigned int minH,
> >  		  unsigned int maxW, unsigned int maxH)
> >  		: minWidth(minW), minHeight(minH), maxWidth(maxW),
> > -		  maxHeight(maxH) {}
> > +		  maxHeight(maxH)
> > +	{
> > +	}
> >
> >  	unsigned int minWidth;
> >  	unsigned int minHeight;
>
> Open question here, what to you think of
> https://en.cppreference.com/w/cpp/language/data_members#Member_initialization,
> item 2 instead of creating a constructor for this kind of purpose ?

That it might be nice, but I need to declare a default constructor
anyhow.

+       SizeRange() = default;

        SizeRange(unsigned int minW, unsigned int minH,
                  unsigned int maxW, unsigned int maxH)
@@ -34,10 +31,10 @@ struct SizeRange {
        {
        }

-       unsigned int minWidth;
-       unsigned int minHeight;
-       unsigned int maxWidth;
-       unsigned int maxHeight;
+       unsigned int minWidth = 0;
+       unsigned int minHeight = 0;
+       unsigned int maxWidth = 0;
+       unsigned int maxHeight = 0;
 };

I don't have any strong opinion to be honest.

Thanks
  j

>
> --
> Regards,
>
> Laurent Pinchart
Laurent Pinchart April 1, 2019, 9:53 p.m. UTC | #3 
Hi Jacopo,

On Wed, Mar 27, 2019 at 09:00:50AM +0100, Jacopo Mondi wrote:
> On Wed, Mar 27, 2019 at 02:13:35AM +0200, Laurent Pinchart wrote:
> > On Tue, Mar 26, 2019 at 09:38:46AM +0100, Jacopo Mondi wrote:
> >> Add constructor to SizeRange which initialize all the size range fields
> >> to 0.
> >>
> >> While at there make the in-line constructor declarations respect the
> >> coding style by moving braces to a new line.
> >>
> >> Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
> >> ---
> >>  src/libcamera/geometry.cpp       | 11 ++++++++++-
> >>  src/libcamera/include/geometry.h |  9 ++++++++-
> >>  2 files changed, 18 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/src/libcamera/geometry.cpp b/src/libcamera/geometry.cpp
> >> index b6b6592bdfec..dbc37ca8e3f4 100644
> >> --- a/src/libcamera/geometry.cpp
> >> +++ b/src/libcamera/geometry.cpp
> >> @@ -57,7 +57,16 @@ namespace libcamera {
> >>
> >>  /**
> >>   * \fn SizeRange::SizeRange()
> >> - * \brief Construct a size range
> >> + * \brief Construct a size range initialized to 0
> >> + */
> >> +
> >> +/**
> >> + * \fn SizeRange::SizeRange(unsigned int minW, unsigned int minH, unsigned int maxW, unsigned int maxH)
> >> + * \brief Construct an initialized size range
> >> + * \param minW The minimum width
> >> + * \param minH The minimum height
> >> + * \param maxW The maximum width
> >> + * \param maxH The maximum height
> >>   */
> >>
> >>  /**
> >> diff --git a/src/libcamera/include/geometry.h b/src/libcamera/include/geometry.h
> >> index eadc4ed4f9cb..749746495204 100644
> >> --- a/src/libcamera/include/geometry.h
> >> +++ b/src/libcamera/include/geometry.h
> >> @@ -18,10 +18,17 @@ struct Rectangle {
> >>  };
> >>
> >>  struct SizeRange {
> >> +	SizeRange(void)
> >> +		: SizeRange(0, 0, 0, 0)
> >> +	{
> >> +	}
> >> +
> >>  	SizeRange(unsigned int minW, unsigned int minH,
> >>  		  unsigned int maxW, unsigned int maxH)
> >>  		: minWidth(minW), minHeight(minH), maxWidth(maxW),
> >> -		  maxHeight(maxH) {}
> >> +		  maxHeight(maxH)
> >> +	{
> >> +	}
> >>
> >>  	unsigned int minWidth;
> >>  	unsigned int minHeight;
> >
> > Open question here, what to you think of
> > https://en.cppreference.com/w/cpp/language/data_members#Member_initialization,
> > item 2 instead of creating a constructor for this kind of purpose ?
> 
> That it might be nice, but I need to declare a default constructor
> anyhow.
> 
> +       SizeRange() = default;
> 
>         SizeRange(unsigned int minW, unsigned int minH,
>                   unsigned int maxW, unsigned int maxH)
> @@ -34,10 +31,10 @@ struct SizeRange {
>         {
>         }
> 
> -       unsigned int minWidth;
> -       unsigned int minHeight;
> -       unsigned int maxWidth;
> -       unsigned int maxHeight;
> +       unsigned int minWidth = 0;
> +       unsigned int minHeight = 0;
> +       unsigned int maxWidth = 0;
> +       unsigned int maxHeight = 0;
>  };
> 
> I don't have any strong opinion to be honest.

Neither do I, hence the open question :-) What do the other think ?

Patch
diff mbox series 

diff --git a/src/libcamera/geometry.cpp b/src/libcamera/geometry.cpp
index b6b6592bdfec..dbc37ca8e3f4 100644
--- a/src/libcamera/geometry.cpp
+++ b/src/libcamera/geometry.cpp
@@ -57,7 +57,16 @@  namespace libcamera {
 
 /**
  * \fn SizeRange::SizeRange()
- * \brief Construct a size range
+ * \brief Construct a size range initialized to 0
+ */
+
+/**
+ * \fn SizeRange::SizeRange(unsigned int minW, unsigned int minH, unsigned int maxW, unsigned int maxH)
+ * \brief Construct an initialized size range
+ * \param minW The minimum width
+ * \param minH The minimum height
+ * \param maxW The maximum width
+ * \param maxH The maximum height
  */
 
 /**
diff --git a/src/libcamera/include/geometry.h b/src/libcamera/include/geometry.h
index eadc4ed4f9cb..749746495204 100644
--- a/src/libcamera/include/geometry.h
+++ b/src/libcamera/include/geometry.h
@@ -18,10 +18,17 @@  struct Rectangle {
 };
 
 struct SizeRange {
+	SizeRange(void)
+		: SizeRange(0, 0, 0, 0)
+	{
+	}
+
 	SizeRange(unsigned int minW, unsigned int minH,
 		  unsigned int maxW, unsigned int maxH)
 		: minWidth(minW), minHeight(minH), maxWidth(maxW),
-		  maxHeight(maxH) {}
+		  maxHeight(maxH)
+	{
+	}
 
 	unsigned int minWidth;
 	unsigned int minHeight;