| Message ID | 20230128162402.892389-1-luca@z3ntu.xyz |
|---|---|
| State | Accepted |
| Headers | show |
| Series |
|
| Related | show |
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@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. Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > /* Setup the camera id combo-box. */ > cameraIdComboBox_ = new QComboBox; > for (const auto &cam : cm_->cameras()) > -- > 2.39.1 >
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@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. But if you want I can also send you screenshots from the phone where I tested this. Regards Luca > > > Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > > > /* Setup the camera id combo-box. */ > > cameraIdComboBox_ = new QComboBox; > > for (const auto &cam : cm_->cameras())
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@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. > > > Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com> Hi, anything missing from getting this patch applied? Regards Luca > > > /* Setup the camera id combo-box. */ > > cameraIdComboBox_ = new QComboBox; > > for (const auto &cam : cm_->cameras())
Hi Luca, On Sat, Mar 04, 2023 at 06:45:50PM +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@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. > > > > > > Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > > Hi, > > anything missing from getting this patch applied? Yes, one more review tag, sorry for being late. I can't say I really understand the implications on bigger screens, but it seems legit to me Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com> Thanks j > > Regards > Luca > > > > > > /* Setup the camera id combo-box. */ > > > cameraIdComboBox_ = new QComboBox; > > > for (const auto &cam : cm_->cameras()) > > > >
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@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. [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@ideasonboard.com> > > > > > /* Setup the camera id combo-box. */ > > > cameraIdComboBox_ = new QComboBox; > > > for (const auto &cam : cm_->cameras())
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@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@ideasonboard.com> > > > > > > > /* Setup the camera id combo-box. */ > > > > cameraIdComboBox_ = new QComboBox; > > > > for (const auto &cam : cm_->cameras())
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); + /* Setup the camera id combo-box. */ cameraIdComboBox_ = new QComboBox; for (const auto &cam : cm_->cameras())
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@z3ntu.xyz> --- src/apps/qcam/cam_select_dialog.cpp | 3 +++ 1 file changed, 3 insertions(+)