| Message ID | 20200206150504.24204-6-kieran.bingham@ideasonboard.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series |
|
| Related | show |
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@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@ideasonboard.com> > + > + qcam = executable('qcam', qcam_sources, moc_files, resources, > install : true, > dependencies : [libcamera_dep, qt5_dep], > cpp_args : qt5_cpp_args)
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@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@ideasonboard.com> > >> + >> + qcam = executable('qcam', qcam_sources, moc_files, resources, >> install : true, >> dependencies : [libcamera_dep, qt5_dep], >> cpp_args : qt5_cpp_args) >
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', +]) + 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) + + qcam = executable('qcam', qcam_sources, moc_files, resources, install : true, dependencies : [libcamera_dep, qt5_dep], cpp_args : qt5_cpp_args)
Provide Quit, Play, Pause, Stop icons. Utilise the provided QT resources to present icons for the toolbar. Update the Quit button with a 'cross', and implement Play/Pause/Stop buttons. '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. Signed-off-by: Kieran Bingham <kieran.bingham@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(-)