| Message ID | 20200628190920.3206340-2-niklas.soderlund@ragnatech.se |
|---|---|
| State | Accepted |
| Commit | 7fc65e9680f604099d5d4f294b37fe7eb93acb03 |
| Headers | show |
| Series |
|
| Related | show |
Hello, On Sun, Jun 28, 2020 at 09:09:19PM +0200, Niklas Söderlund wrote: > From: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > > It's common for code to check if a size is null. Add a helper function > to do so. > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > Reviewed-by: Umang Jain <email@uajain.com> > Reviewed-by: Jacopo Mondi <jacopo@jmondi.org> > Reviewed-by: Niklas Söderlund <niklas.soderlund@ragnatech.se> > --- > include/libcamera/geometry.h | 1 + > src/libcamera/geometry.cpp | 6 ++++++ > test/geometry.cpp | 10 ++++++++++ > 3 files changed, 17 insertions(+) > > diff --git a/include/libcamera/geometry.h b/include/libcamera/geometry.h > index edda42cf34ccbaf0..7d4b8bcfe3d8179e 100644 > --- a/include/libcamera/geometry.h > +++ b/include/libcamera/geometry.h > @@ -41,6 +41,7 @@ struct Size { > unsigned int width; > unsigned int height; > > + bool isNull() const { return !width && !height; } > const std::string toString() const; > }; > > diff --git a/src/libcamera/geometry.cpp b/src/libcamera/geometry.cpp > index fd78cf2c0ab79f8a..24c44fb43acf66ef 100644 > --- a/src/libcamera/geometry.cpp > +++ b/src/libcamera/geometry.cpp > @@ -107,6 +107,12 @@ bool operator==(const Rectangle &lhs, const Rectangle &rhs) > * \brief The Size height > */ > > +/** > + * \fn bool Size::isNull() const Bikeshedding on name apart, would it bring any value describing here when a size is 'null' ? I think it would.. > + * \brief Check if the size is null A Size is null when both its horizontal and vertical dimensions are 0. > + * \return True if both the width and height are 0, or false otherwise It's also said here, so it might not matter much Reviewed-by: Jacopo Mondi <jacopo@jmondi.org> Thanks j > + */ > + > /** > * \brief Assemble and return a string describing the size > * \return A string describing the size > diff --git a/test/geometry.cpp b/test/geometry.cpp > index 27e65565d7c60b47..904ad92c944816b4 100644 > --- a/test/geometry.cpp > +++ b/test/geometry.cpp > @@ -36,6 +36,16 @@ protected: > > int run() > { > + if (!Size().isNull() || !Size(0, 0).isNull()) { > + cout << "Null size incorrectly reported as not null" << endl; > + return TestFail; > + } > + > + if (Size(0, 100).isNull() || Size(100, 0).isNull() || Size(100, 100).isNull()) { > + cout << "Non-null size incorrectly reported as null" << endl; > + return TestFail; > + } > + > /* Test Size equality and inequality. */ > if (!compare(Size(100, 100), Size(100, 100), &operator==, "==", true)) > return TestFail; > -- > 2.27.0 >
Hi Jacopo, On Mon, Jun 29, 2020 at 09:32:08AM +0200, Jacopo Mondi wrote: > On Sun, Jun 28, 2020 at 09:09:19PM +0200, Niklas Söderlund wrote: > > From: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > > > > It's common for code to check if a size is null. Add a helper function > > to do so. > > > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > > Reviewed-by: Umang Jain <email@uajain.com> > > Reviewed-by: Jacopo Mondi <jacopo@jmondi.org> > > Reviewed-by: Niklas Söderlund <niklas.soderlund@ragnatech.se> > > --- > > include/libcamera/geometry.h | 1 + > > src/libcamera/geometry.cpp | 6 ++++++ > > test/geometry.cpp | 10 ++++++++++ > > 3 files changed, 17 insertions(+) > > > > diff --git a/include/libcamera/geometry.h b/include/libcamera/geometry.h > > index edda42cf34ccbaf0..7d4b8bcfe3d8179e 100644 > > --- a/include/libcamera/geometry.h > > +++ b/include/libcamera/geometry.h > > @@ -41,6 +41,7 @@ struct Size { > > unsigned int width; > > unsigned int height; > > > > + bool isNull() const { return !width && !height; } > > const std::string toString() const; > > }; > > > > diff --git a/src/libcamera/geometry.cpp b/src/libcamera/geometry.cpp > > index fd78cf2c0ab79f8a..24c44fb43acf66ef 100644 > > --- a/src/libcamera/geometry.cpp > > +++ b/src/libcamera/geometry.cpp > > @@ -107,6 +107,12 @@ bool operator==(const Rectangle &lhs, const Rectangle &rhs) > > * \brief The Size height > > */ > > > > +/** > > + * \fn bool Size::isNull() const > > Bikeshedding on name apart, would it bring any value describing here > when a size is 'null' ? I think it would.. > > > + * \brief Check if the size is null > > A Size is null when both its horizontal and vertical dimensions > are 0. Maybe s/horizontal and vertical dimensions/width and height/ ? > > > + * \return True if both the width and height are 0, or false otherwise > > It's also said here, so it might not matter much I'd usually say we should add it, but the documentation is so short and the return statement explains it that I don't mind much either way. > Reviewed-by: Jacopo Mondi <jacopo@jmondi.org> > > > + */ > > + > > /** > > * \brief Assemble and return a string describing the size > > * \return A string describing the size > > diff --git a/test/geometry.cpp b/test/geometry.cpp > > index 27e65565d7c60b47..904ad92c944816b4 100644 > > --- a/test/geometry.cpp > > +++ b/test/geometry.cpp > > @@ -36,6 +36,16 @@ protected: > > > > int run() > > { > > + if (!Size().isNull() || !Size(0, 0).isNull()) { > > + cout << "Null size incorrectly reported as not null" << endl; > > + return TestFail; > > + } > > + > > + if (Size(0, 100).isNull() || Size(100, 0).isNull() || Size(100, 100).isNull()) { > > + cout << "Non-null size incorrectly reported as null" << endl; > > + return TestFail; > > + } > > + > > /* Test Size equality and inequality. */ > > if (!compare(Size(100, 100), Size(100, 100), &operator==, "==", true)) > > return TestFail;
diff --git a/include/libcamera/geometry.h b/include/libcamera/geometry.h index edda42cf34ccbaf0..7d4b8bcfe3d8179e 100644 --- a/include/libcamera/geometry.h +++ b/include/libcamera/geometry.h @@ -41,6 +41,7 @@ struct Size { unsigned int width; unsigned int height; + bool isNull() const { return !width && !height; } const std::string toString() const; }; diff --git a/src/libcamera/geometry.cpp b/src/libcamera/geometry.cpp index fd78cf2c0ab79f8a..24c44fb43acf66ef 100644 --- a/src/libcamera/geometry.cpp +++ b/src/libcamera/geometry.cpp @@ -107,6 +107,12 @@ bool operator==(const Rectangle &lhs, const Rectangle &rhs) * \brief The Size height */ +/** + * \fn bool Size::isNull() const + * \brief Check if the size is null + * \return True if both the width and height are 0, or false otherwise + */ + /** * \brief Assemble and return a string describing the size * \return A string describing the size diff --git a/test/geometry.cpp b/test/geometry.cpp index 27e65565d7c60b47..904ad92c944816b4 100644 --- a/test/geometry.cpp +++ b/test/geometry.cpp @@ -36,6 +36,16 @@ protected: int run() { + if (!Size().isNull() || !Size(0, 0).isNull()) { + cout << "Null size incorrectly reported as not null" << endl; + return TestFail; + } + + if (Size(0, 100).isNull() || Size(100, 0).isNull() || Size(100, 100).isNull()) { + cout << "Non-null size incorrectly reported as null" << endl; + return TestFail; + } + /* Test Size equality and inequality. */ if (!compare(Size(100, 100), Size(100, 100), &operator==, "==", true)) return TestFail;