| Message ID | 20210614111110.33324-1-kieran.bingham@ideasonboard.com |
|---|---|
| State | New |
| Delegated to: | Kieran Bingham |
| Headers | show |
| Series |
|
| Related | show |
Hi Kieran, Thank you for the patch. On Mon, Jun 14, 2021 at 12:11:10PM +0100, Kieran Bingham wrote: > Keep the image aspect ratio when displaying in the viewfinder. > > When the window is adjusted to a size that differs in aspect ratio to > the image, keep the image centered in the main viewpoint. > > Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > --- > > v2: (fixed up from Laurent's review comments) > > - No longer accept event on an invalid size. > - centering simplifed > > > src/qcam/viewfinder_qt.cpp | 16 ++++++++++++++-- > src/qcam/viewfinder_qt.h | 2 ++ > 2 files changed, 16 insertions(+), 2 deletions(-) > > diff --git a/src/qcam/viewfinder_qt.cpp b/src/qcam/viewfinder_qt.cpp > index e436714c6bdb..08ac29e9fa99 100644 > --- a/src/qcam/viewfinder_qt.cpp > +++ b/src/qcam/viewfinder_qt.cpp > @@ -15,6 +15,7 @@ > #include <QMap> > #include <QMutexLocker> > #include <QPainter> > +#include <QResizeEvent> > #include <QtDebug> > > #include <libcamera/formats.h> > @@ -34,7 +35,7 @@ static const QMap<libcamera::PixelFormat, QImage::Format> nativeFormats > }; > > ViewFinderQt::ViewFinderQt(QWidget *parent) > - : QWidget(parent), buffer_(nullptr) > + : QWidget(parent), place_(rect()), buffer_(nullptr) > { > icon_ = QIcon(":camera-off.svg"); > } > @@ -146,7 +147,7 @@ void ViewFinderQt::paintEvent(QPaintEvent *) > > /* If we have an image, draw it. */ > if (!image_.isNull()) { > - painter.drawImage(rect(), image_, image_.rect()); > + painter.drawImage(place_, image_, image_.rect()); > return; > } > > @@ -179,3 +180,14 @@ QSize ViewFinderQt::sizeHint() const > { > return size_.isValid() ? size_ : QSize(640, 480); > } > + > +void ViewFinderQt::resizeEvent(QResizeEvent *event) > +{ > + if (!size_.isValid()) > + return; > + > + place_.setSize(size_.scaled(event->size(), Qt::KeepAspectRatio)); > + place_.moveCenter(rect().center()); > + > + event->accept(); > +} > diff --git a/src/qcam/viewfinder_qt.h b/src/qcam/viewfinder_qt.h > index d755428887c0..bf31b81b3437 100644 > --- a/src/qcam/viewfinder_qt.h > +++ b/src/qcam/viewfinder_qt.h > @@ -42,6 +42,7 @@ Q_SIGNALS: > > protected: > void paintEvent(QPaintEvent *) override; > + void resizeEvent(QResizeEvent *) override; > QSize sizeHint() const override; > > private: > @@ -49,6 +50,7 @@ private: > > libcamera::PixelFormat format_; > QSize size_; > + QRect place_; > > /* Camera stopped icon */ > QSize vfSize_;
Hi Kieran, On Tue, Jun 15, 2021 at 01:32:16AM +0300, Laurent Pinchart wrote: > Hi Kieran, > > Thank you for the patch. > > On Mon, Jun 14, 2021 at 12:11:10PM +0100, Kieran Bingham wrote: > > Keep the image aspect ratio when displaying in the viewfinder. > > > > When the window is adjusted to a size that differs in aspect ratio to > > the image, keep the image centered in the main viewpoint. > > > > Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > > > --- > > > > v2: (fixed up from Laurent's review comments) > > > > - No longer accept event on an invalid size. > > - centering simplifed > > > > > > src/qcam/viewfinder_qt.cpp | 16 ++++++++++++++-- > > src/qcam/viewfinder_qt.h | 2 ++ > > 2 files changed, 16 insertions(+), 2 deletions(-) > > > > diff --git a/src/qcam/viewfinder_qt.cpp b/src/qcam/viewfinder_qt.cpp > > index e436714c6bdb..08ac29e9fa99 100644 > > --- a/src/qcam/viewfinder_qt.cpp > > +++ b/src/qcam/viewfinder_qt.cpp > > @@ -15,6 +15,7 @@ > > #include <QMap> > > #include <QMutexLocker> > > #include <QPainter> > > +#include <QResizeEvent> > > #include <QtDebug> > > > > #include <libcamera/formats.h> > > @@ -34,7 +35,7 @@ static const QMap<libcamera::PixelFormat, QImage::Format> nativeFormats > > }; > > > > ViewFinderQt::ViewFinderQt(QWidget *parent) > > - : QWidget(parent), buffer_(nullptr) > > + : QWidget(parent), place_(rect()), buffer_(nullptr) > > { > > icon_ = QIcon(":camera-off.svg"); > > } > > @@ -146,7 +147,7 @@ void ViewFinderQt::paintEvent(QPaintEvent *) > > > > /* If we have an image, draw it. */ > > if (!image_.isNull()) { > > - painter.drawImage(rect(), image_, image_.rect()); > > + painter.drawImage(place_, image_, image_.rect()); > > return; > > } > > > > @@ -179,3 +180,14 @@ QSize ViewFinderQt::sizeHint() const > > { > > return size_.isValid() ? size_ : QSize(640, 480); > > } > > + > > +void ViewFinderQt::resizeEvent(QResizeEvent *event) > > +{ > > + if (!size_.isValid()) > > + return; > > + > > + place_.setSize(size_.scaled(event->size(), Qt::KeepAspectRatio)); > > + place_.moveCenter(rect().center()); > > + > > + event->accept(); As far as I can tell, event->accept() isn't needed. It's however a good practice to call the base object's resizeEvent() function. I'd thus replace this with QWidget::resizeEvent(event); > > +} > > diff --git a/src/qcam/viewfinder_qt.h b/src/qcam/viewfinder_qt.h > > index d755428887c0..bf31b81b3437 100644 > > --- a/src/qcam/viewfinder_qt.h > > +++ b/src/qcam/viewfinder_qt.h > > @@ -42,6 +42,7 @@ Q_SIGNALS: > > > > protected: > > void paintEvent(QPaintEvent *) override; > > + void resizeEvent(QResizeEvent *) override; > > QSize sizeHint() const override; > > > > private: > > @@ -49,6 +50,7 @@ private: > > > > libcamera::PixelFormat format_; > > QSize size_; > > + QRect place_; > > > > /* Camera stopped icon */ > > QSize vfSize_;
A few (last ?) comments. As you only address the Qt-based viewfinder implementation, s/viewfinder/viewfinder_qt/ in the subject line would be appropriate. On Tue, Jun 15, 2021 at 01:59:07AM +0300, Laurent Pinchart wrote: > On Tue, Jun 15, 2021 at 01:32:16AM +0300, Laurent Pinchart wrote: > > On Mon, Jun 14, 2021 at 12:11:10PM +0100, Kieran Bingham wrote: > > > Keep the image aspect ratio when displaying in the viewfinder. > > > > > > When the window is adjusted to a size that differs in aspect ratio to > > > the image, keep the image centered in the main viewpoint. Did you mean s/viewpoint/viewfinder/ ? > > > > > > Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > > > > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > > > > > --- > > > > > > v2: (fixed up from Laurent's review comments) > > > > > > - No longer accept event on an invalid size. > > > - centering simplifed > > > > > > > > > src/qcam/viewfinder_qt.cpp | 16 ++++++++++++++-- > > > src/qcam/viewfinder_qt.h | 2 ++ > > > 2 files changed, 16 insertions(+), 2 deletions(-) > > > > > > diff --git a/src/qcam/viewfinder_qt.cpp b/src/qcam/viewfinder_qt.cpp > > > index e436714c6bdb..08ac29e9fa99 100644 > > > --- a/src/qcam/viewfinder_qt.cpp > > > +++ b/src/qcam/viewfinder_qt.cpp > > > @@ -15,6 +15,7 @@ > > > #include <QMap> > > > #include <QMutexLocker> > > > #include <QPainter> > > > +#include <QResizeEvent> > > > #include <QtDebug> > > > > > > #include <libcamera/formats.h> > > > @@ -34,7 +35,7 @@ static const QMap<libcamera::PixelFormat, QImage::Format> nativeFormats > > > }; > > > > > > ViewFinderQt::ViewFinderQt(QWidget *parent) > > > - : QWidget(parent), buffer_(nullptr) > > > + : QWidget(parent), place_(rect()), buffer_(nullptr) > > > { > > > icon_ = QIcon(":camera-off.svg"); > > > } > > > @@ -146,7 +147,7 @@ void ViewFinderQt::paintEvent(QPaintEvent *) > > > > > > /* If we have an image, draw it. */ > > > if (!image_.isNull()) { > > > - painter.drawImage(rect(), image_, image_.rect()); > > > + painter.drawImage(place_, image_, image_.rect()); > > > return; > > > } > > > > > > @@ -179,3 +180,14 @@ QSize ViewFinderQt::sizeHint() const > > > { > > > return size_.isValid() ? size_ : QSize(640, 480); > > > } > > > + > > > +void ViewFinderQt::resizeEvent(QResizeEvent *event) > > > +{ > > > + if (!size_.isValid()) > > > + return; > > > + > > > + place_.setSize(size_.scaled(event->size(), Qt::KeepAspectRatio)); > > > + place_.moveCenter(rect().center()); > > > + > > > + event->accept(); > > As far as I can tell, event->accept() isn't needed. It's however a good > practice to call the base object's resizeEvent() function. I'd thus > replace this with > > QWidget::resizeEvent(event); > > > > +} > > > diff --git a/src/qcam/viewfinder_qt.h b/src/qcam/viewfinder_qt.h > > > index d755428887c0..bf31b81b3437 100644 > > > --- a/src/qcam/viewfinder_qt.h > > > +++ b/src/qcam/viewfinder_qt.h > > > @@ -42,6 +42,7 @@ Q_SIGNALS: > > > > > > protected: > > > void paintEvent(QPaintEvent *) override; > > > + void resizeEvent(QResizeEvent *) override; > > > QSize sizeHint() const override; > > > > > > private: > > > @@ -49,6 +50,7 @@ private: > > > > > > libcamera::PixelFormat format_; > > > QSize size_; > > > + QRect place_; > > > > > > /* Camera stopped icon */ > > > QSize vfSize_;
diff --git a/src/qcam/viewfinder_qt.cpp b/src/qcam/viewfinder_qt.cpp index e436714c6bdb..08ac29e9fa99 100644 --- a/src/qcam/viewfinder_qt.cpp +++ b/src/qcam/viewfinder_qt.cpp @@ -15,6 +15,7 @@ #include <QMap> #include <QMutexLocker> #include <QPainter> +#include <QResizeEvent> #include <QtDebug> #include <libcamera/formats.h> @@ -34,7 +35,7 @@ static const QMap<libcamera::PixelFormat, QImage::Format> nativeFormats }; ViewFinderQt::ViewFinderQt(QWidget *parent) - : QWidget(parent), buffer_(nullptr) + : QWidget(parent), place_(rect()), buffer_(nullptr) { icon_ = QIcon(":camera-off.svg"); } @@ -146,7 +147,7 @@ void ViewFinderQt::paintEvent(QPaintEvent *) /* If we have an image, draw it. */ if (!image_.isNull()) { - painter.drawImage(rect(), image_, image_.rect()); + painter.drawImage(place_, image_, image_.rect()); return; } @@ -179,3 +180,14 @@ QSize ViewFinderQt::sizeHint() const { return size_.isValid() ? size_ : QSize(640, 480); } + +void ViewFinderQt::resizeEvent(QResizeEvent *event) +{ + if (!size_.isValid()) + return; + + place_.setSize(size_.scaled(event->size(), Qt::KeepAspectRatio)); + place_.moveCenter(rect().center()); + + event->accept(); +} diff --git a/src/qcam/viewfinder_qt.h b/src/qcam/viewfinder_qt.h index d755428887c0..bf31b81b3437 100644 --- a/src/qcam/viewfinder_qt.h +++ b/src/qcam/viewfinder_qt.h @@ -42,6 +42,7 @@ Q_SIGNALS: protected: void paintEvent(QPaintEvent *) override; + void resizeEvent(QResizeEvent *) override; QSize sizeHint() const override; private: @@ -49,6 +50,7 @@ private: libcamera::PixelFormat format_; QSize size_; + QRect place_; /* Camera stopped icon */ QSize vfSize_;
Keep the image aspect ratio when displaying in the viewfinder. When the window is adjusted to a size that differs in aspect ratio to the image, keep the image centered in the main viewpoint. Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com> --- v2: (fixed up from Laurent's review comments) - No longer accept event on an invalid size. - centering simplifed src/qcam/viewfinder_qt.cpp | 16 ++++++++++++++-- src/qcam/viewfinder_qt.h | 2 ++ 2 files changed, 16 insertions(+), 2 deletions(-)