[libcamera-devel] qcam: Support scaling of the viewfinder

2655 diff mbox series

The viewfinder is drawn using a QLabel. This could support scaling
through QLabel::setScaledContents(), but in a very inefficient way. To
maintain reasonable efficiency, turn the viewfinder into a QWidget and
draw the image directly using a QPainter.

No performance change was noticed running on a fast x86 machine, and
performance was 60% higher when scaling up to full screen compared to
QLabel.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 src/qcam/main_window.cpp |  1 -
 src/qcam/viewfinder.cpp  | 22 +++++++++++++++-------
 src/qcam/viewfinder.h    |  8 ++++++--
 3 files changed, 21 insertions(+), 10 deletions(-)

Hi Laurent,

Thanks for your work.

On 2020-01-17 02:54:20 +0200, Laurent Pinchart wrote:
> The viewfinder is drawn using a QLabel. This could support scaling
> through QLabel::setScaledContents(), but in a very inefficient way. To
> maintain reasonable efficiency, turn the viewfinder into a QWidget and
> draw the image directly using a QPainter.
> 
> No performance change was noticed running on a fast x86 machine, and
> performance was 60% higher when scaling up to full screen compared to
> QLabel.

I could not detect any performance change on Scarlet.

> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

Reviewed-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>

> ---
>  src/qcam/main_window.cpp |  1 -
>  src/qcam/viewfinder.cpp  | 22 +++++++++++++++-------
>  src/qcam/viewfinder.h    |  8 ++++++--
>  3 files changed, 21 insertions(+), 10 deletions(-)
> 
> diff --git a/src/qcam/main_window.cpp b/src/qcam/main_window.cpp
> index 1d9c756f147a..df51fa888342 100644
> --- a/src/qcam/main_window.cpp
> +++ b/src/qcam/main_window.cpp
> @@ -33,7 +33,6 @@ MainWindow::MainWindow(CameraManager *cm, const OptionsParser::Options &options)
>  
>  	viewfinder_ = new ViewFinder(this);
>  	setCentralWidget(viewfinder_);
> -	viewfinder_->setFixedSize(500, 500);
>  	adjustSize();
>  
>  	ret = openCamera(cm);
> diff --git a/src/qcam/viewfinder.cpp b/src/qcam/viewfinder.cpp
> index 98a8ab68e5f6..6de284d1b782 100644
> --- a/src/qcam/viewfinder.cpp
> +++ b/src/qcam/viewfinder.cpp
> @@ -6,13 +6,13 @@
>   */
>  
>  #include <QImage>
> -#include <QPixmap>
> +#include <QPainter>
>  
>  #include "format_converter.h"
>  #include "viewfinder.h"
>  
>  ViewFinder::ViewFinder(QWidget *parent)
> -	: QLabel(parent), format_(0), width_(0), height_(0), image_(nullptr)
> +	: QWidget(parent), format_(0), width_(0), height_(0), image_(nullptr)
>  {
>  }
>  
> @@ -24,9 +24,7 @@ ViewFinder::~ViewFinder()
>  void ViewFinder::display(const unsigned char *raw, size_t size)
>  {
>  	converter_.convert(raw, size, image_);
> -
> -	QPixmap pixmap = QPixmap::fromImage(*image_);
> -	setPixmap(pixmap);
> +	update();
>  }
>  
>  int ViewFinder::setFormat(unsigned int format, unsigned int width,
> @@ -42,10 +40,20 @@ int ViewFinder::setFormat(unsigned int format, unsigned int width,
>  	width_ = width;
>  	height_ = height;
>  
> -	setFixedSize(width, height);
> -
>  	delete image_;
>  	image_ = new QImage(width, height, QImage::Format_RGB32);
>  
> +	updateGeometry();
>  	return 0;
>  }
> +
> +void ViewFinder::paintEvent(QPaintEvent *)
> +{
> +	QPainter painter(this);
> +	painter.drawImage(rect(), *image_, image_->rect());
> +}
> +
> +QSize ViewFinder::sizeHint() const
> +{
> +	return image_ ? image_->size() : QSize(640, 480);
> +}
> diff --git a/src/qcam/viewfinder.h b/src/qcam/viewfinder.h
> index 33bdb1460f84..ef5fd45b264a 100644
> --- a/src/qcam/viewfinder.h
> +++ b/src/qcam/viewfinder.h
> @@ -7,13 +7,13 @@
>  #ifndef __QCAM_VIEWFINDER_H__
>  #define __QCAM_VIEWFINDER_H__
>  
> -#include <QLabel>
> +#include <QWidget>
>  
>  #include "format_converter.h"
>  
>  class QImage;
>  
> -class ViewFinder : public QLabel
> +class ViewFinder : public QWidget
>  {
>  public:
>  	ViewFinder(QWidget *parent);
> @@ -23,6 +23,10 @@ public:
>  		      unsigned int height);
>  	void display(const unsigned char *rgb, size_t size);
>  
> +protected:
> +	void paintEvent(QPaintEvent *) override;
> +	QSize sizeHint() const override;
> +
>  private:
>  	unsigned int format_;
>  	unsigned int width_;
> -- 
> Regards,
> 
> Laurent Pinchart
> 
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel@lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel

Hi Laurent,

On 17/01/2020 00:54, Laurent Pinchart wrote:
> The viewfinder is drawn using a QLabel. This could support scaling
> through QLabel::setScaledContents(), but in a very inefficient way. To
> maintain reasonable efficiency, turn the viewfinder into a QWidget and
> draw the image directly using a QPainter.
> 
> No performance change was noticed running on a fast x86 machine, and
> performance was 60% higher when scaling up to full screen compared to
> QLabel.

This does indeed bring scaling performance improvements for me, and I am
very happy that we can now scale the QCam ViewFinder window both up and
down :-D

We should add a means to return to the original window size easily at
some point (perhaps a reset button on a toolbar) but that's out of scope
for this patch.

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

> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
>  src/qcam/main_window.cpp |  1 -
>  src/qcam/viewfinder.cpp  | 22 +++++++++++++++-------
>  src/qcam/viewfinder.h    |  8 ++++++--
>  3 files changed, 21 insertions(+), 10 deletions(-)
> 
> diff --git a/src/qcam/main_window.cpp b/src/qcam/main_window.cpp
> index 1d9c756f147a..df51fa888342 100644
> --- a/src/qcam/main_window.cpp
> +++ b/src/qcam/main_window.cpp
> @@ -33,7 +33,6 @@ MainWindow::MainWindow(CameraManager *cm, const OptionsParser::Options &options)
>  
>  	viewfinder_ = new ViewFinder(this);
>  	setCentralWidget(viewfinder_);
> -	viewfinder_->setFixedSize(500, 500);
>  	adjustSize();
>  
>  	ret = openCamera(cm);
> diff --git a/src/qcam/viewfinder.cpp b/src/qcam/viewfinder.cpp
> index 98a8ab68e5f6..6de284d1b782 100644
> --- a/src/qcam/viewfinder.cpp
> +++ b/src/qcam/viewfinder.cpp
> @@ -6,13 +6,13 @@
>   */
>  
>  #include <QImage>
> -#include <QPixmap>
> +#include <QPainter>
>  
>  #include "format_converter.h"
>  #include "viewfinder.h"
>  
>  ViewFinder::ViewFinder(QWidget *parent)
> -	: QLabel(parent), format_(0), width_(0), height_(0), image_(nullptr)
> +	: QWidget(parent), format_(0), width_(0), height_(0), image_(nullptr)
>  {
>  }
>  
> @@ -24,9 +24,7 @@ ViewFinder::~ViewFinder()
>  void ViewFinder::display(const unsigned char *raw, size_t size)
>  {
>  	converter_.convert(raw, size, image_);
> -
> -	QPixmap pixmap = QPixmap::fromImage(*image_);
> -	setPixmap(pixmap);
> +	update();
>  }
>  
>  int ViewFinder::setFormat(unsigned int format, unsigned int width,
> @@ -42,10 +40,20 @@ int ViewFinder::setFormat(unsigned int format, unsigned int width,
>  	width_ = width;
>  	height_ = height;
>  
> -	setFixedSize(width, height);
> -
>  	delete image_;
>  	image_ = new QImage(width, height, QImage::Format_RGB32);
>  
> +	updateGeometry();
>  	return 0;
>  }
> +
> +void ViewFinder::paintEvent(QPaintEvent *)
> +{
> +	QPainter painter(this);
> +	painter.drawImage(rect(), *image_, image_->rect());
> +}
> +
> +QSize ViewFinder::sizeHint() const
> +{
> +	return image_ ? image_->size() : QSize(640, 480);
> +}
> diff --git a/src/qcam/viewfinder.h b/src/qcam/viewfinder.h
> index 33bdb1460f84..ef5fd45b264a 100644
> --- a/src/qcam/viewfinder.h
> +++ b/src/qcam/viewfinder.h
> @@ -7,13 +7,13 @@
>  #ifndef __QCAM_VIEWFINDER_H__
>  #define __QCAM_VIEWFINDER_H__
>  
> -#include <QLabel>
> +#include <QWidget>
>  
>  #include "format_converter.h"
>  
>  class QImage;
>  
> -class ViewFinder : public QLabel
> +class ViewFinder : public QWidget
>  {
>  public:
>  	ViewFinder(QWidget *parent);
> @@ -23,6 +23,10 @@ public:
>  		      unsigned int height);
>  	void display(const unsigned char *rgb, size_t size);
>  
> +protected:
> +	void paintEvent(QPaintEvent *) override;
> +	QSize sizeHint() const override;
> +
>  private:
>  	unsigned int format_;
>  	unsigned int width_;
>