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

[Kieran Bingham]

Hi Laurent,

On 07/02/2020 00:00, Laurent Pinchart wrote:
> 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.

Pause is removed.


> 
>> 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 ?


There's no facility in meson to generate the qrc, which would have been
nice.

I'd rather specify the icons used in a meson variable and have it
generate. We could pull together our own script - but for now we can
just hard code the entries in the .qrc file itself.


>> +
>>  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)

Aha, for some reason I had assumed that had to be separated.

I'll update it and if it works, it's in.


> 
>     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
--
Kieran