[libcamera-devel] [PATCH 1/2] qcam: main_window: Make cameraCombo_ private

[Laurent Pinchart]

On Thu, Apr 30, 2020 at 04:05:56PM +0100, Kieran Bingham wrote:
> On 30/04/2020 15:15, Umang Jain wrote:
> > This commit introduces no functional changes.
> > This is required so that the combo-box list can be managed
> > conveniently from various private functions in subsequent
> > commit.
> > 
> > Signed-off-by: Umang Jain <email at uajain.com>
> > ---
> >  src/qcam/main_window.cpp | 9 ++++-----
> >  src/qcam/main_window.h   | 2 ++
> >  2 files changed, 6 insertions(+), 5 deletions(-)
> > 
> > diff --git a/src/qcam/main_window.cpp b/src/qcam/main_window.cpp
> > index d021fa9..344b7ec 100644
> > --- a/src/qcam/main_window.cpp
> > +++ b/src/qcam/main_window.cpp
> > @@ -11,7 +11,6 @@
> >  #include <string>
> >  #include <sys/mman.h>
> >  
> > -#include <QComboBox>
> 
> Shouldn't we keep this here ... and...
> 
> >  #include <QCoreApplication>
> >  #include <QFileDialog>
> >  #include <QImage>
> > @@ -114,14 +113,14 @@ int MainWindow::createToolbars()
> >  	connect(action, &QAction::triggered, this, &MainWindow::quit);
> >  
> >  	/* Camera selector. */
> > -	QComboBox *cameraCombo = new QComboBox();
> > -	connect(cameraCombo, QOverload<int>::of(&QComboBox::activated),
> > +	cameraCombo_ = new QComboBox();
> > +	connect(cameraCombo_, QOverload<int>::of(&QComboBox::activated),
> >  		this, &MainWindow::switchCamera);
> >  
> >  	for (const std::shared_ptr<Camera> &cam : cm_->cameras())
> > -		cameraCombo->addItem(QString::fromStdString(cam->name()));
> > +		cameraCombo_->addItem(QString::fromStdString(cam->name()));
> >  
> > -	toolbar_->addWidget(cameraCombo);
> > +	toolbar_->addWidget(cameraCombo_);
> >  
> >  	toolbar_->addSeparator();
> >  
> > diff --git a/src/qcam/main_window.h b/src/qcam/main_window.h
> > index 5d6251c..28c325f 100644
> > --- a/src/qcam/main_window.h
> > +++ b/src/qcam/main_window.h
> > @@ -9,6 +9,7 @@
> >  
> >  #include <memory>
> >  
> > +#include <QComboBox>
> 
> instead just forward declare
> 
> 	class QComboBox;
> 
> But as this hasn't been done for other parts of this component, I don't
> think it makes much difference.
> 
> Either way...
> 
> Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com>

I was going to mention the same. We already forward-declare QAction that
way. QToolBar should do the same, and so should QComboBox here. Apart
from that,

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

> >  #include <QElapsedTimer>
> >  #include <QIcon>
> >  #include <QMainWindow>
> > @@ -72,6 +73,7 @@ private:
> >  	/* UI elements */
> >  	QToolBar *toolbar_;
> >  	QAction *startStopAction_;
> > +	QComboBox *cameraCombo_;
> >  	ViewFinder *viewfinder_;
> >  
> >  	QIcon iconPlay_;

-- 
Regards,

Laurent Pinchart