[libcamera-devel] [PATCH v9 3/7] qcam: MainWindow: Replace cameraCombo_ with CameraSelectorDialog

[Kieran Bingham]

Quoting Utkarsh Tiwari (2022-08-31 06:49:34)
> Replace the cameraCombo_ on the toolbar with a QPushButton which
> displays the CameraSelectorDialog. This would allow the user to view
> information about the camera when switching.
> 
> The QPushButton text is set to the camera Id currently in use.
> 
> Signed-off-by: Utkarsh Tiwari <utkarsh02t at gmail.com>
> Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
> Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> ---
> Differences from v9:
>     1. Checking for OptCamera now happens in 1/7 in openCamera dropping
>     the firstCameraSelect_
>     2. Improve curly brace structure
>     3. Drop QComboBox
>  src/qcam/main_window.cpp | 33 ++++++++++++++++-----------------
>  src/qcam/main_window.h   |  5 +++--
>  2 files changed, 19 insertions(+), 19 deletions(-)
> 
> diff --git a/src/qcam/main_window.cpp b/src/qcam/main_window.cpp
> index e8e22d49..505bdc56 100644
> --- a/src/qcam/main_window.cpp
> +++ b/src/qcam/main_window.cpp
> @@ -14,7 +14,6 @@
>  #include <libcamera/camera_manager.h>
>  #include <libcamera/version.h>
>  
> -#include <QComboBox>
>  #include <QCoreApplication>
>  #include <QFileDialog>
>  #include <QImage>
> @@ -195,14 +194,11 @@ int MainWindow::createToolbars()
>         connect(action, &QAction::triggered, this, &MainWindow::quit);
>  
>         /* Camera selector. */
> -       cameraCombo_ = new QComboBox();
> -       connect(cameraCombo_, QOverload<int>::of(&QComboBox::activated),
> +       cameraSelectButton_ = new QPushButton;
> +       connect(cameraSelectButton_, &QPushButton::clicked,
>                 this, &MainWindow::switchCamera);
>  
> -       for (const std::shared_ptr<Camera> &cam : cm_->cameras())
> -               cameraCombo_->addItem(QString::fromStdString(cam->id()));
> -
> -       toolbar_->addWidget(cameraCombo_);
> +       toolbar_->addWidget(cameraSelectButton_);
>  
>         toolbar_->addSeparator();
>  
> @@ -262,14 +258,18 @@ void MainWindow::updateTitle()
>   * Camera Selection
>   */
>  
> -void MainWindow::switchCamera(int index)
> +void MainWindow::switchCamera()
>  {
>         /* Get and acquire the new camera. */
> -       const auto &cameras = cm_->cameras();
> -       if (static_cast<unsigned int>(index) >= cameras.size())
> +       std::string newCameraId = chooseCamera();
> +
> +       if (newCameraId.empty())
>                 return;
>  
> -       const std::shared_ptr<Camera> &cam = cameras[index];
> +       if (camera_ && newCameraId == camera_->id())
> +               return;
> +
> +       const std::shared_ptr<Camera> &cam = cm_->get(newCameraId);
>  
>         if (cam->acquire()) {
>                 qInfo() << "Failed to acquire camera" << cam->id().c_str();
> @@ -288,6 +288,9 @@ void MainWindow::switchCamera(int index)
>         camera_ = cam;
>  
>         startStopAction_->setChecked(true);
> +
> +       /* Display the current cameraId in the toolbar .*/
> +       cameraSelectButton_->setText(QString::fromStdString(newCameraId));

Really minor, and doesn't deserve a new version/iteration (could be done
while applying) - but I'd set the text before calling the
startStopAction(true).

Logically - it makes sense to update the context before starting it.
That's all - functionally here, I don't think it will have a significant
difference.

This already has tags and is good to go though I think.


>  }
>  
>  std::string MainWindow::chooseCamera()
> @@ -324,8 +327,8 @@ int MainWindow::openCamera()
>                 return -EBUSY;
>         }
>  
> -       /* Set the combo-box entry with the currently selected Camera. */
> -       cameraCombo_->setCurrentText(QString::fromStdString(cameraName));
> +       /* Set the camera switch button with the currently selected Camera id. */
> +       cameraSelectButton_->setText(QString::fromStdString(cameraName));
>  
>         return 0;
>  }
> @@ -591,7 +594,6 @@ void MainWindow::processHotplug(HotplugEvent *e)
>         HotplugEvent::PlugEvent event = e->hotplugEvent();
>  
>         if (event == HotplugEvent::HotPlug) {
> -               cameraCombo_->addItem(cameraId);
>                 cameraSelectorDialog_->addCamera(cameraId);
>         } else if (event == HotplugEvent::HotUnplug) {
>                 /* Check if the currently-streaming camera is removed. */
> @@ -599,11 +601,8 @@ void MainWindow::processHotplug(HotplugEvent *e)
>                         toggleCapture(false);
>                         camera_->release();
>                         camera_.reset();
> -                       cameraCombo_->setCurrentIndex(0);
>                 }
>  
> -               int camIndex = cameraCombo_->findText(cameraId);
> -               cameraCombo_->removeItem(camIndex);
>                 cameraSelectorDialog_->removeCamera(cameraId);
>         }
>  }
> diff --git a/src/qcam/main_window.h b/src/qcam/main_window.h
> index def44605..79723256 100644
> --- a/src/qcam/main_window.h
> +++ b/src/qcam/main_window.h
> @@ -23,6 +23,7 @@
>  #include <QMainWindow>
>  #include <QMutex>
>  #include <QObject>
> +#include <QPushButton>
>  #include <QQueue>
>  #include <QTimer>
>  
> @@ -60,7 +61,7 @@ private Q_SLOTS:
>         void quit();
>         void updateTitle();
>  
> -       void switchCamera(int index);
> +       void switchCamera();
>         void toggleCapture(bool start);
>  
>         void saveImageAs();
> @@ -90,7 +91,7 @@ private:
>         /* UI elements */
>         QToolBar *toolbar_;
>         QAction *startStopAction_;
> -       QComboBox *cameraCombo_;
> +       QPushButton *cameraSelectButton_;
>         QAction *saveRaw_;
>         ViewFinder *viewfinder_;
>  
> -- 
> 2.34.1
>