| Message ID | 20220809205042.344923-8-utkarsh02t@gmail.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series |
|
| Related | show |
Quoting Utkarsh Tiwari via libcamera-devel (2022-08-09 21:50:41) > Display the path of the selected capture script in a thinner font. > > Signed-off-by: Utkarsh Tiwari <utkarsh02t@gmail.com> > --- > src/qcam/cam_select_dialog.cpp | 38 +++++++++++++++++++++++++++------- > src/qcam/cam_select_dialog.h | 8 ++++++- > src/qcam/main_window.cpp | 3 ++- > 3 files changed, 40 insertions(+), 9 deletions(-) > > diff --git a/src/qcam/cam_select_dialog.cpp b/src/qcam/cam_select_dialog.cpp > index f3df9970..5a9de08c 100644 > --- a/src/qcam/cam_select_dialog.cpp > +++ b/src/qcam/cam_select_dialog.cpp > @@ -20,8 +20,9 @@ > #include <QString> > > CameraSelectorDialog::CameraSelectorDialog(libcamera::CameraManager *cameraManager, > - bool isScriptRunning, QWidget *parent) > - : QDialog(parent), cm_(cameraManager), isScriptRunning_(isScriptRunning) > + bool isScriptRunning, std::string scriptPath, QWidget *parent) > + : QDialog(parent), cm_(cameraManager), > + isScriptRunning_(isScriptRunning), scriptPath_(scriptPath) > { > /* Use a QFormLayout for the dialog. */ > QFormLayout *layout = new QFormLayout(this); > @@ -39,14 +40,31 @@ CameraSelectorDialog::CameraSelectorDialog(libcamera::CameraManager *cameraManag > connect(cameraIdComboBox_, &QComboBox::currentTextChanged, > this, &CameraSelectorDialog::handleCameraChange); > > + /* Setup widget for capture script button. */ > + QWidget *captureWidget = new QWidget; > + captureWidgetLayout_ = new QVBoxLayout(captureWidget); > + captureWidgetLayout_->setMargin(0); > + > captureScriptButton_ = new QPushButton; > connect(captureScriptButton_, &QPushButton::clicked, > this, &CameraSelectorDialog::handleCaptureScriptButton); > + captureWidgetLayout_->addWidget(captureScriptButton_); > + > + /* Use a thinner font to indicate script info. */ > + QFont smallFont; > + smallFont.setWeight(QFont::Thin); > + > + scriptPathLabel_ = new QLabel; Creating it again? That's another memory leak. It might be worth trying your patchs with valgrind or the memleak detectors that can be enabled with meson. > + scriptPathLabel_->setFont(smallFont); > + scriptPathLabel_->setWordWrap(true); > > /* Display the action that would be performed when button is clicked. */ > - if (isScriptRunning_) > + if (isScriptRunning_) { > captureScriptButton_->setText("Stop"); > - else > + > + scriptPathLabel_->setText(QString::fromStdString(scriptPath_)); > + captureWidgetLayout_->addWidget(scriptPathLabel_); > + } else > captureScriptButton_->setText("Open"); > > /* Setup the QDialogButton Box */ > @@ -63,7 +81,7 @@ CameraSelectorDialog::CameraSelectorDialog(libcamera::CameraManager *cameraManag > layout->addRow("Camera:", cameraIdComboBox_); > layout->addRow("Location:", cameraLocation_); > layout->addRow("Model:", cameraModel_); > - layout->addRow("Capture Script:", captureScriptButton_); > + layout->addRow("Capture Script:", captureWidget); > layout->addWidget(buttonBox); > } > > @@ -138,16 +156,22 @@ void CameraSelectorDialog::handleCaptureScriptButton() > Q_EMIT stopCaptureScript(); > isScriptRunning_ = false; > captureScriptButton_->setText("Open"); > + > + captureWidgetLayout_->removeWidget(scriptPathLabel_); > } else { > selectedScriptPath_ = QFileDialog::getOpenFileName(this, > "Run Capture Script", QDir::currentPath(), > "Capture Script (*.yaml)") > .toStdString(); > > - if (!selectedScriptPath_.empty()) > + if (!selectedScriptPath_.empty()) { > captureScriptButton_->setText("Loaded"); > - else > + scriptPathLabel_->setText(QString::fromStdString(selectedScriptPath_)); > + captureWidgetLayout_->addWidget(scriptPathLabel_); > + } else { > captureScriptButton_->setText("Open"); > + captureWidgetLayout_->removeWidget(scriptPathLabel_); > + } > } > } > > diff --git a/src/qcam/cam_select_dialog.h b/src/qcam/cam_select_dialog.h > index 56d90596..84904640 100644 > --- a/src/qcam/cam_select_dialog.h > +++ b/src/qcam/cam_select_dialog.h > @@ -18,17 +18,20 @@ > #include <QDialog> > #include <QDialogButtonBox> > #include <QFileDialog> > +#include <QFont> If this is only used in the .cpp file, include it there. I don't think it's needed in the header. > #include <QFormLayout> > #include <QLabel> > #include <QPushButton> > #include <QString> > +#include <QVBoxLayout> > +#include <QWidget> > > class CameraSelectorDialog : public QDialog > { > Q_OBJECT > public: > CameraSelectorDialog(libcamera::CameraManager *cameraManager, > - bool isScriptRunning, QWidget *parent); > + bool isScriptRunning, std::string scriptPath, QWidget *parent); > > ~CameraSelectorDialog() = default; > > @@ -66,5 +69,8 @@ private: > QComboBox *cameraIdComboBox_; > QLabel *cameraLocation_; > QLabel *cameraModel_; > + > + QVBoxLayout *captureWidgetLayout_; > QPushButton *captureScriptButton_; > + QLabel *scriptPathLabel_ = new QLabel; This doesn't match how everthing else is initialised, and I also think this could simply be a local instance, not a pointer: QLabel scriptPath_; > }; > diff --git a/src/qcam/main_window.cpp b/src/qcam/main_window.cpp > index d73fb42a..f2e3c576 100644 > --- a/src/qcam/main_window.cpp > +++ b/src/qcam/main_window.cpp > @@ -337,7 +337,8 @@ std::string MainWindow::chooseCamera() > bool scriptRunning = script_ != nullptr; > > /* Construct the selection dialog, unconditionally. */ > - cameraSelectorDialog_ = new CameraSelectorDialog(cm_, scriptRunning, this); > + cameraSelectorDialog_ = new CameraSelectorDialog(cm_, scriptRunning, > + scriptPath_, this); > > connect(cameraSelectorDialog_, &CameraSelectorDialog::stopCaptureScript, > this, &MainWindow::stopCaptureScript); > -- > 2.25.1 >
Hi Kieran, thanks for the review. On Wed, Aug 10, 2022 at 3:42 AM Kieran Bingham < kieran.bingham@ideasonboard.com> wrote: > Quoting Utkarsh Tiwari via libcamera-devel (2022-08-09 21:50:41) > > Display the path of the selected capture script in a thinner font. > > > > Signed-off-by: Utkarsh Tiwari <utkarsh02t@gmail.com> > > --- > > src/qcam/cam_select_dialog.cpp | 38 +++++++++++++++++++++++++++------- > > src/qcam/cam_select_dialog.h | 8 ++++++- > > src/qcam/main_window.cpp | 3 ++- > > 3 files changed, 40 insertions(+), 9 deletions(-) > > > > diff --git a/src/qcam/cam_select_dialog.cpp > b/src/qcam/cam_select_dialog.cpp > > index f3df9970..5a9de08c 100644 > > --- a/src/qcam/cam_select_dialog.cpp > > +++ b/src/qcam/cam_select_dialog.cpp > > @@ -20,8 +20,9 @@ > > #include <QString> > > > > CameraSelectorDialog::CameraSelectorDialog(libcamera::CameraManager > *cameraManager, > > - bool isScriptRunning, QWidget > *parent) > > - : QDialog(parent), cm_(cameraManager), > isScriptRunning_(isScriptRunning) > > + bool isScriptRunning, > std::string scriptPath, QWidget *parent) > > + : QDialog(parent), cm_(cameraManager), > > + isScriptRunning_(isScriptRunning), scriptPath_(scriptPath) > > { > > /* Use a QFormLayout for the dialog. */ > > QFormLayout *layout = new QFormLayout(this); > > @@ -39,14 +40,31 @@ > CameraSelectorDialog::CameraSelectorDialog(libcamera::CameraManager > *cameraManag > > connect(cameraIdComboBox_, &QComboBox::currentTextChanged, > > this, &CameraSelectorDialog::handleCameraChange); > > > > + /* Setup widget for capture script button. */ > > + QWidget *captureWidget = new QWidget; > > + captureWidgetLayout_ = new QVBoxLayout(captureWidget); > > + captureWidgetLayout_->setMargin(0); > > + > > captureScriptButton_ = new QPushButton; > > connect(captureScriptButton_, &QPushButton::clicked, > > this, &CameraSelectorDialog::handleCaptureScriptButton); > > + captureWidgetLayout_->addWidget(captureScriptButton_); > > + > > + /* Use a thinner font to indicate script info. */ > > + QFont smallFont; > > + smallFont.setWeight(QFont::Thin); > > + > > + scriptPathLabel_ = new QLabel; > > Creating it again? That's another memory leak. > > Oops this should be the only one, the one below is an remnant of rebases. > It might be worth trying your patchs with valgrind or the memleak > detectors that can be enabled with meson. > Yes, I would figure out how to run them. I have never used them before but I don't think they would be able to find these leaks because these objects *are* deleted when the window is closed as the parent of the these QObjects is still the QDialog and when it dies it deletes its children. > > > + scriptPathLabel_->setFont(smallFont); > > + scriptPathLabel_->setWordWrap(true); > > > > /* Display the action that would be performed when button is > clicked. */ > > - if (isScriptRunning_) > > + if (isScriptRunning_) { > > captureScriptButton_->setText("Stop"); > > - else > > + > > + > scriptPathLabel_->setText(QString::fromStdString(scriptPath_)); > > + captureWidgetLayout_->addWidget(scriptPathLabel_); > > + } else > > captureScriptButton_->setText("Open"); > > > > /* Setup the QDialogButton Box */ > > @@ -63,7 +81,7 @@ > CameraSelectorDialog::CameraSelectorDialog(libcamera::CameraManager > *cameraManag > > layout->addRow("Camera:", cameraIdComboBox_); > > layout->addRow("Location:", cameraLocation_); > > layout->addRow("Model:", cameraModel_); > > - layout->addRow("Capture Script:", captureScriptButton_); > > + layout->addRow("Capture Script:", captureWidget); > > layout->addWidget(buttonBox); > > } > > > > @@ -138,16 +156,22 @@ void > CameraSelectorDialog::handleCaptureScriptButton() > > Q_EMIT stopCaptureScript(); > > isScriptRunning_ = false; > > captureScriptButton_->setText("Open"); > > + > > + captureWidgetLayout_->removeWidget(scriptPathLabel_); > > } else { > > selectedScriptPath_ = QFileDialog::getOpenFileName(this, > > "Run > Capture Script", QDir::currentPath(), > > > "Capture Script (*.yaml)") > > .toStdString(); > > > > - if (!selectedScriptPath_.empty()) > > + if (!selectedScriptPath_.empty()) { > > captureScriptButton_->setText("Loaded"); > > - else > > + > scriptPathLabel_->setText(QString::fromStdString(selectedScriptPath_)); > > + > captureWidgetLayout_->addWidget(scriptPathLabel_); > > + } else { > > captureScriptButton_->setText("Open"); > > + > captureWidgetLayout_->removeWidget(scriptPathLabel_); > > + } > > } > > } > > > > diff --git a/src/qcam/cam_select_dialog.h b/src/qcam/cam_select_dialog.h > > index 56d90596..84904640 100644 > > --- a/src/qcam/cam_select_dialog.h > > +++ b/src/qcam/cam_select_dialog.h > > @@ -18,17 +18,20 @@ > > #include <QDialog> > > #include <QDialogButtonBox> > > #include <QFileDialog> > > +#include <QFont> > > If this is only used in the .cpp file, include it there. I don't think > it's needed in the header. > Agreed. > > > #include <QFormLayout> > > #include <QLabel> > > #include <QPushButton> > > #include <QString> > > +#include <QVBoxLayout> > > +#include <QWidget> > > > > class CameraSelectorDialog : public QDialog > > { > > Q_OBJECT > > public: > > CameraSelectorDialog(libcamera::CameraManager *cameraManager, > > - bool isScriptRunning, QWidget *parent); > > + bool isScriptRunning, std::string > scriptPath, QWidget *parent); > > > > ~CameraSelectorDialog() = default; > > > > @@ -66,5 +69,8 @@ private: > > QComboBox *cameraIdComboBox_; > > QLabel *cameraLocation_; > > QLabel *cameraModel_; > > + > > + QVBoxLayout *captureWidgetLayout_; > > QPushButton *captureScriptButton_; > > + QLabel *scriptPathLabel_ = new QLabel; > > > This doesn't match how everthing else is initialised, and I also think > this could simply be a local instance, not a pointer: > QLabel scriptPath_; > > Ah yes. this should just be QLabel *scriptPath_ = new QLabel; I still think we should use pointers and allocate them on the heap. If we go the stack based approach we would have to think in two phases 1. The GUI layout 2. The stack layout (Such that parent doesn't die before child) Currently we can limit the member variables to only the important ones For e.g captureWidget the important thing here is its layout and the actual widget's symbol just lives locally in the constructor. The QObject trees do take care of deleting everything if parents are set right. And at all places (even in this buggy recreation ) whenever we are creating them and adding them to its layout, the object deletion gets handled. > > > }; > > diff --git a/src/qcam/main_window.cpp b/src/qcam/main_window.cpp > > index d73fb42a..f2e3c576 100644 > > --- a/src/qcam/main_window.cpp > > +++ b/src/qcam/main_window.cpp > > @@ -337,7 +337,8 @@ std::string MainWindow::chooseCamera() > > bool scriptRunning = script_ != nullptr; > > > > /* Construct the selection dialog, unconditionally. */ > > - cameraSelectorDialog_ = new CameraSelectorDialog(cm_, > scriptRunning, this); > > + cameraSelectorDialog_ = new CameraSelectorDialog(cm_, > scriptRunning, > > + scriptPath_, > this); > > > > connect(cameraSelectorDialog_, > &CameraSelectorDialog::stopCaptureScript, > > this, &MainWindow::stopCaptureScript); > > -- > > 2.25.1 > > >
Quoting Utkarsh Tiwari (2022-08-10 05:26:58) > Hi Kieran, thanks for the review. > > On Wed, Aug 10, 2022 at 3:42 AM Kieran Bingham < > kieran.bingham@ideasonboard.com> wrote: > > > Quoting Utkarsh Tiwari via libcamera-devel (2022-08-09 21:50:41) > > > Display the path of the selected capture script in a thinner font. > > > > > > Signed-off-by: Utkarsh Tiwari <utkarsh02t@gmail.com> > > > --- > > > src/qcam/cam_select_dialog.cpp | 38 +++++++++++++++++++++++++++------- > > > src/qcam/cam_select_dialog.h | 8 ++++++- > > > src/qcam/main_window.cpp | 3 ++- > > > 3 files changed, 40 insertions(+), 9 deletions(-) > > > > > > diff --git a/src/qcam/cam_select_dialog.cpp > > b/src/qcam/cam_select_dialog.cpp > > > index f3df9970..5a9de08c 100644 > > > --- a/src/qcam/cam_select_dialog.cpp > > > +++ b/src/qcam/cam_select_dialog.cpp > > > @@ -20,8 +20,9 @@ > > > #include <QString> > > > > > > CameraSelectorDialog::CameraSelectorDialog(libcamera::CameraManager > > *cameraManager, > > > - bool isScriptRunning, QWidget > > *parent) > > > - : QDialog(parent), cm_(cameraManager), > > isScriptRunning_(isScriptRunning) > > > + bool isScriptRunning, > > std::string scriptPath, QWidget *parent) > > > + : QDialog(parent), cm_(cameraManager), > > > + isScriptRunning_(isScriptRunning), scriptPath_(scriptPath) > > > { > > > /* Use a QFormLayout for the dialog. */ > > > QFormLayout *layout = new QFormLayout(this); > > > @@ -39,14 +40,31 @@ > > CameraSelectorDialog::CameraSelectorDialog(libcamera::CameraManager > > *cameraManag > > > connect(cameraIdComboBox_, &QComboBox::currentTextChanged, > > > this, &CameraSelectorDialog::handleCameraChange); > > > > > > + /* Setup widget for capture script button. */ > > > + QWidget *captureWidget = new QWidget; > > > + captureWidgetLayout_ = new QVBoxLayout(captureWidget); > > > + captureWidgetLayout_->setMargin(0); > > > + > > > captureScriptButton_ = new QPushButton; > > > connect(captureScriptButton_, &QPushButton::clicked, > > > this, &CameraSelectorDialog::handleCaptureScriptButton); > > > + captureWidgetLayout_->addWidget(captureScriptButton_); > > > + > > > + /* Use a thinner font to indicate script info. */ > > > + QFont smallFont; > > > + smallFont.setWeight(QFont::Thin); > > > + > > > + scriptPathLabel_ = new QLabel; > > > > Creating it again? That's another memory leak. > > > > Oops this should be the only one, the one below is an remnant of > rebases. > > > It might be worth trying your patchs with valgrind or the memleak > > detectors that can be enabled with meson. > > > > Yes, I would figure out how to run them. I have never used them before > but I don't think they would be able to find these leaks because these > objects *are* deleted when the window is closed as the parent of the > these QObjects is still the QDialog and when it dies it deletes its > children. > > > > > > + scriptPathLabel_->setFont(smallFont); > > > + scriptPathLabel_->setWordWrap(true); > > > > > > /* Display the action that would be performed when button is > > clicked. */ > > > - if (isScriptRunning_) > > > + if (isScriptRunning_) { > > > captureScriptButton_->setText("Stop"); > > > - else > > > + > > > + > > scriptPathLabel_->setText(QString::fromStdString(scriptPath_)); > > > + captureWidgetLayout_->addWidget(scriptPathLabel_); > > > + } else > > > captureScriptButton_->setText("Open"); > > > > > > /* Setup the QDialogButton Box */ > > > @@ -63,7 +81,7 @@ > > CameraSelectorDialog::CameraSelectorDialog(libcamera::CameraManager > > *cameraManag > > > layout->addRow("Camera:", cameraIdComboBox_); > > > layout->addRow("Location:", cameraLocation_); > > > layout->addRow("Model:", cameraModel_); > > > - layout->addRow("Capture Script:", captureScriptButton_); > > > + layout->addRow("Capture Script:", captureWidget); > > > layout->addWidget(buttonBox); > > > } > > > > > > @@ -138,16 +156,22 @@ void > > CameraSelectorDialog::handleCaptureScriptButton() > > > Q_EMIT stopCaptureScript(); > > > isScriptRunning_ = false; > > > captureScriptButton_->setText("Open"); > > > + > > > + captureWidgetLayout_->removeWidget(scriptPathLabel_); > > > } else { > > > selectedScriptPath_ = QFileDialog::getOpenFileName(this, > > > "Run > > Capture Script", QDir::currentPath(), > > > > > "Capture Script (*.yaml)") > > > .toStdString(); > > > > > > - if (!selectedScriptPath_.empty()) > > > + if (!selectedScriptPath_.empty()) { > > > captureScriptButton_->setText("Loaded"); > > > - else > > > + > > scriptPathLabel_->setText(QString::fromStdString(selectedScriptPath_)); > > > + > > captureWidgetLayout_->addWidget(scriptPathLabel_); > > > + } else { > > > captureScriptButton_->setText("Open"); > > > + > > captureWidgetLayout_->removeWidget(scriptPathLabel_); > > > + } > > > } > > > } > > > > > > diff --git a/src/qcam/cam_select_dialog.h b/src/qcam/cam_select_dialog.h > > > index 56d90596..84904640 100644 > > > --- a/src/qcam/cam_select_dialog.h > > > +++ b/src/qcam/cam_select_dialog.h > > > @@ -18,17 +18,20 @@ > > > #include <QDialog> > > > #include <QDialogButtonBox> > > > #include <QFileDialog> > > > +#include <QFont> > > > > If this is only used in the .cpp file, include it there. I don't think > > it's needed in the header. > > > Agreed. > > > > > > #include <QFormLayout> > > > #include <QLabel> > > > #include <QPushButton> > > > #include <QString> > > > +#include <QVBoxLayout> > > > +#include <QWidget> > > > > > > class CameraSelectorDialog : public QDialog > > > { > > > Q_OBJECT > > > public: > > > CameraSelectorDialog(libcamera::CameraManager *cameraManager, > > > - bool isScriptRunning, QWidget *parent); > > > + bool isScriptRunning, std::string > > scriptPath, QWidget *parent); > > > > > > ~CameraSelectorDialog() = default; > > > > > > @@ -66,5 +69,8 @@ private: > > > QComboBox *cameraIdComboBox_; > > > QLabel *cameraLocation_; > > > QLabel *cameraModel_; > > > + > > > + QVBoxLayout *captureWidgetLayout_; > > > QPushButton *captureScriptButton_; > > > + QLabel *scriptPathLabel_ = new QLabel; > > > > > > This doesn't match how everthing else is initialised, and I also think > > this could simply be a local instance, not a pointer: > > QLabel scriptPath_; > > > > Ah yes. this should just be QLabel *scriptPath_ = new QLabel; > I still think we should use pointers and allocate them on the heap. > If we go the stack based approach we would have to think in two phases > 1. The GUI layout > 2. The stack layout (Such that parent doesn't die before child) This isn't about stack vs heap - if the QLabel is a direct member of CameraSelectorDialog, then when it is allocated, so too will the QLabel. It becomes 'part' of the CameraSelectorDialog, so it will be allocated at the same time (as part of the same allocation). > Currently we can limit the member variables to only the important ones > For e.g captureWidget the important thing here is its layout and the > actual widget's symbol just lives locally in the constructor. > > The QObject trees do take care of deleting everything if parents are > set right. And at all places (even in this buggy recreation ) whenever we > are creating them and adding them to its layout, the object deletion > gets handled. Ok - I don't know enough about how QT is handling pointer ownership here, so as long as it works and something is cleaning up correctly. > > > > > > }; > > > diff --git a/src/qcam/main_window.cpp b/src/qcam/main_window.cpp > > > index d73fb42a..f2e3c576 100644 > > > --- a/src/qcam/main_window.cpp > > > +++ b/src/qcam/main_window.cpp > > > @@ -337,7 +337,8 @@ std::string MainWindow::chooseCamera() > > > bool scriptRunning = script_ != nullptr; > > > > > > /* Construct the selection dialog, unconditionally. */ > > > - cameraSelectorDialog_ = new CameraSelectorDialog(cm_, > > scriptRunning, this); > > > + cameraSelectorDialog_ = new CameraSelectorDialog(cm_, > > scriptRunning, > > > + scriptPath_, > > this); > > > > > > connect(cameraSelectorDialog_, > > &CameraSelectorDialog::stopCaptureScript, > > > this, &MainWindow::stopCaptureScript); > > > -- > > > 2.25.1 > > > > >
diff --git a/src/qcam/cam_select_dialog.cpp b/src/qcam/cam_select_dialog.cpp index f3df9970..5a9de08c 100644 --- a/src/qcam/cam_select_dialog.cpp +++ b/src/qcam/cam_select_dialog.cpp @@ -20,8 +20,9 @@ #include <QString> CameraSelectorDialog::CameraSelectorDialog(libcamera::CameraManager *cameraManager, - bool isScriptRunning, QWidget *parent) - : QDialog(parent), cm_(cameraManager), isScriptRunning_(isScriptRunning) + bool isScriptRunning, std::string scriptPath, QWidget *parent) + : QDialog(parent), cm_(cameraManager), + isScriptRunning_(isScriptRunning), scriptPath_(scriptPath) { /* Use a QFormLayout for the dialog. */ QFormLayout *layout = new QFormLayout(this); @@ -39,14 +40,31 @@ CameraSelectorDialog::CameraSelectorDialog(libcamera::CameraManager *cameraManag connect(cameraIdComboBox_, &QComboBox::currentTextChanged, this, &CameraSelectorDialog::handleCameraChange); + /* Setup widget for capture script button. */ + QWidget *captureWidget = new QWidget; + captureWidgetLayout_ = new QVBoxLayout(captureWidget); + captureWidgetLayout_->setMargin(0); + captureScriptButton_ = new QPushButton; connect(captureScriptButton_, &QPushButton::clicked, this, &CameraSelectorDialog::handleCaptureScriptButton); + captureWidgetLayout_->addWidget(captureScriptButton_); + + /* Use a thinner font to indicate script info. */ + QFont smallFont; + smallFont.setWeight(QFont::Thin); + + scriptPathLabel_ = new QLabel; + scriptPathLabel_->setFont(smallFont); + scriptPathLabel_->setWordWrap(true); /* Display the action that would be performed when button is clicked. */ - if (isScriptRunning_) + if (isScriptRunning_) { captureScriptButton_->setText("Stop"); - else + + scriptPathLabel_->setText(QString::fromStdString(scriptPath_)); + captureWidgetLayout_->addWidget(scriptPathLabel_); + } else captureScriptButton_->setText("Open"); /* Setup the QDialogButton Box */ @@ -63,7 +81,7 @@ CameraSelectorDialog::CameraSelectorDialog(libcamera::CameraManager *cameraManag layout->addRow("Camera:", cameraIdComboBox_); layout->addRow("Location:", cameraLocation_); layout->addRow("Model:", cameraModel_); - layout->addRow("Capture Script:", captureScriptButton_); + layout->addRow("Capture Script:", captureWidget); layout->addWidget(buttonBox); } @@ -138,16 +156,22 @@ void CameraSelectorDialog::handleCaptureScriptButton() Q_EMIT stopCaptureScript(); isScriptRunning_ = false; captureScriptButton_->setText("Open"); + + captureWidgetLayout_->removeWidget(scriptPathLabel_); } else { selectedScriptPath_ = QFileDialog::getOpenFileName(this, "Run Capture Script", QDir::currentPath(), "Capture Script (*.yaml)") .toStdString(); - if (!selectedScriptPath_.empty()) + if (!selectedScriptPath_.empty()) { captureScriptButton_->setText("Loaded"); - else + scriptPathLabel_->setText(QString::fromStdString(selectedScriptPath_)); + captureWidgetLayout_->addWidget(scriptPathLabel_); + } else { captureScriptButton_->setText("Open"); + captureWidgetLayout_->removeWidget(scriptPathLabel_); + } } } diff --git a/src/qcam/cam_select_dialog.h b/src/qcam/cam_select_dialog.h index 56d90596..84904640 100644 --- a/src/qcam/cam_select_dialog.h +++ b/src/qcam/cam_select_dialog.h @@ -18,17 +18,20 @@ #include <QDialog> #include <QDialogButtonBox> #include <QFileDialog> +#include <QFont> #include <QFormLayout> #include <QLabel> #include <QPushButton> #include <QString> +#include <QVBoxLayout> +#include <QWidget> class CameraSelectorDialog : public QDialog { Q_OBJECT public: CameraSelectorDialog(libcamera::CameraManager *cameraManager, - bool isScriptRunning, QWidget *parent); + bool isScriptRunning, std::string scriptPath, QWidget *parent); ~CameraSelectorDialog() = default; @@ -66,5 +69,8 @@ private: QComboBox *cameraIdComboBox_; QLabel *cameraLocation_; QLabel *cameraModel_; + + QVBoxLayout *captureWidgetLayout_; QPushButton *captureScriptButton_; + QLabel *scriptPathLabel_ = new QLabel; }; diff --git a/src/qcam/main_window.cpp b/src/qcam/main_window.cpp index d73fb42a..f2e3c576 100644 --- a/src/qcam/main_window.cpp +++ b/src/qcam/main_window.cpp @@ -337,7 +337,8 @@ std::string MainWindow::chooseCamera() bool scriptRunning = script_ != nullptr; /* Construct the selection dialog, unconditionally. */ - cameraSelectorDialog_ = new CameraSelectorDialog(cm_, scriptRunning, this); + cameraSelectorDialog_ = new CameraSelectorDialog(cm_, scriptRunning, + scriptPath_, this); connect(cameraSelectorDialog_, &CameraSelectorDialog::stopCaptureScript, this, &MainWindow::stopCaptureScript);
Display the path of the selected capture script in a thinner font. Signed-off-by: Utkarsh Tiwari <utkarsh02t@gmail.com> --- src/qcam/cam_select_dialog.cpp | 38 +++++++++++++++++++++++++++------- src/qcam/cam_select_dialog.h | 8 ++++++- src/qcam/main_window.cpp | 3 ++- 3 files changed, 40 insertions(+), 9 deletions(-)