[libcamera-devel] [PATCH] qcam: Decrease minimum width of selector dialog

[Laurent Pinchart]

On Mon, Mar 06, 2023 at 12:07:47PM +0200, Laurent Pinchart via libcamera-devel wrote:
> On Mon, Jan 30, 2023 at 06:58:29PM +0100, Luca Weiss via libcamera-devel wrote:
> > On Montag, 30. Jänner 2023 15:11:16 CET Kieran Bingham wrote:
> > > Quoting Luca Weiss via libcamera-devel (2023-01-28 16:24:03)
> > > 
> > > > On phone screens the default width is too wide, so the OK button cannot
> > > > be clicked.
> > > > 
> > > > Fix this by decreasing the minimum size of the dialog so it fits nicely.
> > > > 
> > > > Signed-off-by: Luca Weiss <luca at z3ntu.xyz>
> > > > ---
> > > > 
> > > >  src/apps/qcam/cam_select_dialog.cpp | 3 +++
> > > >  1 file changed, 3 insertions(+)
> > > > 
> > > > diff --git a/src/apps/qcam/cam_select_dialog.cpp
> > > > b/src/apps/qcam/cam_select_dialog.cpp index 3c8b12a9..2a600383 100644
> > > > --- a/src/apps/qcam/cam_select_dialog.cpp
> > > > +++ b/src/apps/qcam/cam_select_dialog.cpp
> > > > @@ -25,6 +25,9 @@
> > > > CameraSelectorDialog::CameraSelectorDialog(libcamera::CameraManager
> > > > *cameraManag> 
> > > >         /* Use a QFormLayout for the dialog. */
> > > >         QFormLayout *layout = new QFormLayout(this);
> > > > 
> > > > +       /* Decrease minimum width of dialog to fit on narrow screens */
> > > > +       setMinimumSize(250, 100);
> > > > +
> > > 
> > > This looks reasonable, and when applying here - it doesn't break my
> > > view, but my screen is large.
> > > 
> > > Is there any way to reproduce this issue otherwise? I suspect not easily
> > > without getting a small screen. But to me this is ok.
> > 
> > Not really I think because the window manager prohibits resizing the window 
> > below the minimum size. Perhaps setting some insane scaling (if the window 
> > manager allows this) so everything's just super big.
> > 
> > From what I could tell the minimum size gets set as the size the combobox 
> > takes up which can be quite a big with a long identification string as happens 
> > on ARM which includes the path to the dt node.
> 
> By default the minimum size will be set by the dialog box's QLayout,
> which indeed should size it based on the largest element. We could
> reduce the minimum size of the combo box, but I don't think that's
> better than changing the dialog's minimum size.
> 
> Do we need to reduce both the minimum width and height, or could we
> avoid touching the height ? Also, would it make sense to only lower the
> minimum size on small screens ? You can get the desktop's geometry from
> the QDesktopWidget class (accessible from QApplication::desktop()), see
> [1]. We could then compare it with the layout's minimum size, and only
> set a manual minimum size when the desktop's geometry is small.

QApplication::desktop() seems deprecated, and replaced with
primaryScreen(). The following code is what I have in mind.

diff --git a/src/apps/qcam/cam_select_dialog.cpp b/src/apps/qcam/cam_select_dialog.cpp
index 3c8b12a9c174..750e5f5df682 100644
--- a/src/apps/qcam/cam_select_dialog.cpp
+++ b/src/apps/qcam/cam_select_dialog.cpp
@@ -15,7 +15,9 @@
 #include <QComboBox>
 #include <QDialogButtonBox>
 #include <QFormLayout>
+#include <QGuiApplication>
 #include <QLabel>
+#include <QScreen>
 #include <QString>

 CameraSelectorDialog::CameraSelectorDialog(libcamera::CameraManager *cameraManager,
@@ -53,6 +55,14 @@ CameraSelectorDialog::CameraSelectorDialog(libcamera::CameraManager *cameraManag
 	layout->addRow("Location:", cameraLocation_);
 	layout->addRow("Model:", cameraModel_);
 	layout->addWidget(buttonBox);
+
+	/*
+	 * Decrease the minimum width of dialog to fit on narrow screens, with a
+	 * 20 pixels margin.
+	 */
+	QRect screenGeometry = qGuiApp->primaryScreen()->availableGeometry();
+	if (screenGeometry.width() < minimumWidth())
+		setMinimumWidth(screenGeometry.width() - 20);
 }

 CameraSelectorDialog::~CameraSelectorDialog() = default;

> [1] https://doc.qt.io/qt-5/qdesktopwidget.html#availableGeometry
> 
> > But if you want I can also send you screenshots from the phone where I tested 
> > this.
> 
> A screenshot that demonstrates the problem would be nice.
> 
> > > Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
> > > 
> > > >         /* Setup the camera id combo-box. */
> > > >         cameraIdComboBox_ = new QComboBox;
> > > >         for (const auto &cam : cm_->cameras())

-- 
Regards,

Laurent Pinchart