[libcamera-devel] [PATCH 5/6] qcam: Provide initial icon buttons "Play/Stop"

[Laurent Pinchart]

Hi Kieran,

Thank you for the patch.

On Thu, Feb 06, 2020 at 03:05:03PM +0000, Kieran Bingham wrote:
> Provide Quit, Play, Pause, Stop icons.
> 
> Utilise the provided QT resources to present icons for the toolbar.

s/QT/Qt/

> 
> Update the Quit button with a 'cross', and implement Play/Pause/Stop
> buttons.

I really like the Breeze icons from KDE, they have a quit button, but
they're unfortunately licensed under LGPL-3+, not LGPL-2.1+.

> 'Pause' is a no-op currently and likely could be removed, but I wanted
> an ability to distinguish between holding the stream and restarting it.

How about adding pause when we'll have that ability ? :-) A no-op button
is confusing.

> Signed-off-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
> ---
>  src/qcam/main_window.cpp | 12 +++++++++++-
>  src/qcam/main_window.h   |  7 +++----
>  src/qcam/meson.build     |  8 +++++++-
>  3 files changed, 21 insertions(+), 6 deletions(-)
> 
> diff --git a/src/qcam/main_window.cpp b/src/qcam/main_window.cpp
> index 1c7260f32d0a..0ae4c60b8699 100644
> --- a/src/qcam/main_window.cpp
> +++ b/src/qcam/main_window.cpp
> @@ -11,6 +11,7 @@
>  #include <sys/mman.h>
>  
>  #include <QCoreApplication>
> +#include <QIcon>
>  #include <QInputDialog>
>  #include <QTimer>
>  #include <QToolBar>
> @@ -63,7 +64,7 @@ int MainWindow::createToolbars(CameraManager *cm)
>  
>  	toolbar_ = addToolBar("");
>  
> -	action = toolbar_->addAction("Quit");
> +	action = toolbar_->addAction(QIcon(":x-circle.svg"), "Quit");
>  	connect(action, &QAction::triggered, this, &MainWindow::quit);
>  
>  	QAction *cameraAction = new QAction("&Cameras", this);
> @@ -79,6 +80,15 @@ int MainWindow::createToolbars(CameraManager *cm)
>  		connect(action, &QAction::triggered, this, [=]() { this->setCamera(cam); });
>  	}
>  
> +	action = toolbar_->addAction(QIcon(":play-circle.svg"), "start");
> +	connect(action, &QAction::triggered, this, &MainWindow::startCapture);
> +
> +	toolbar_->addAction(QIcon(":pause-circle.svg"), "pause");
> +	/* TODO: Connect an action to perform when 'pause' requested? or remove */
> +
> +	action = toolbar_->addAction(QIcon(":stop-circle.svg"), "stop");
> +	connect(action, &QAction::triggered, this, &MainWindow::stopCapture);
> +
>  	return 0;
>  }
>  
> diff --git a/src/qcam/main_window.h b/src/qcam/main_window.h
> index f7c96fdd5c30..b0bf16dd2a09 100644
> --- a/src/qcam/main_window.h
> +++ b/src/qcam/main_window.h
> @@ -46,14 +46,13 @@ private Q_SLOTS:
>  
>  	int setCamera(const std::shared_ptr<Camera> &cam);
>  
> +	int startCapture();
> +	void stopCapture();
> +
>  private:
>  	int createToolbars(CameraManager *cm);
>  	std::string chooseCamera(CameraManager *cm);
>  	int openCamera(CameraManager *cm);
> -
> -	int startCapture();
> -	void stopCapture();
> -
>  	void requestComplete(Request *request);
>  	int display(FrameBuffer *buffer);
>  
> diff --git a/src/qcam/meson.build b/src/qcam/meson.build
> index 1e71f20fa15e..b6544dbf3f2b 100644
> --- a/src/qcam/meson.build
> +++ b/src/qcam/meson.build
> @@ -11,6 +11,10 @@ qcam_moc_headers = files([
>      'main_window.h',
>  ])
>  
> +qcam_resources = files([
> +    'assets/feathericons/feathericons.qrc',
> +])

I wonder, should we only import the icons we use ? Would it also make
sense to generate the qrc file as part of the build process, or is it
supposed to be part of the sources ?

> +
>  qt5 = import('qt5')
>  qt5_dep = dependency('qt5',
>                       method : 'pkg-config',
> @@ -33,7 +37,9 @@ if qt5_dep.found()
>      moc_files = qt5.preprocess(moc_headers: qcam_moc_headers,
>                                 dependencies: qt5_dep)
>  
> -    qcam  = executable('qcam', qcam_sources, moc_files,
> +    resources = qt5.preprocess(qresources : qcam_resources)

How about combining the moc and rcc ?

    resources = qt5.preprocess(moc_headers: qcam_moc_headers,
                               qresources : qcam_resources,
                               dependencies: qt5_dep)

    qcam  = executable('qcam', qcam_sources, resources,
                       install : true,
                       dependencies : [libcamera_dep, qt5_dep],
                       cpp_args : qt5_cpp_args)

Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>

> +
> +    qcam  = executable('qcam', qcam_sources, moc_files, resources,
>                         install : true,
>                         dependencies : [libcamera_dep, qt5_dep],
>                         cpp_args : qt5_cpp_args)

-- 
Regards,

Laurent Pinchart