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

[Umang Jain]

Hi all,

On Thu, Apr 30, 2020 at 18:18, Laurent Pinchart 
<laurent.pinchart at ideasonboard.com> wrote:
> 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 
>> <mailto: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 
>> <mailto: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 
> <mailto:laurent.pinchart at ideasonboard.com>>

Well, the QToolBar is already forward-declared via #include 
<QMainWindow>
header, so it's not should not be necessary to foward-declare it here.

Thanks for the comments. I will submit v2 of the patches in a few 
minutes.
> 
>>  >  #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

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.libcamera.org/pipermail/libcamera-devel/attachments/20200430/32319efa/attachment.htm>