| Message ID | 20240905162508.4169-2-laurent.pinchart@ideasonboard.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series |
|
| Related | show |
Quoting Laurent Pinchart (2024-09-05 17:25:07) > When the widget's aspect ratio doesn't match the camera aspect ratio, > the viewfinder is rendered letter-boxed. The side rectangles are not > painted by the viewfinder, and Qt thus renders the parent widget > background to fill that space. > > To make it black, we have two options: > > - The simplest option is to set the widget's autoFillBackground property > to true. This causes Qt to paint the whole widget with its background > colour before calling paintEvent(). As the camera image typically > covers most (if not all) of the viewfinder widget, this is less > efficient. > > - The more complicated option is to pain the letterbox rectangles s/pain/paint/ > manually. We can additionally set the widget's WA_OpaquePaintEvent > attribute to instruct Qt to skip painting the parent widget. This > reduces CPU usage by about 1% (and may reduce GPU usage as well). > > Note that the WA_OpaquePaintEvent attribute has to be disabled when we > render the stopped icon, as the icon has a transparent background. > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > --- > src/apps/qcam/viewfinder_qt.cpp | 23 +++++++++++++++++++++-- > 1 file changed, 21 insertions(+), 2 deletions(-) > > diff --git a/src/apps/qcam/viewfinder_qt.cpp b/src/apps/qcam/viewfinder_qt.cpp > index 492648cfa2ff..62b6f27fa23e 100644 > --- a/src/apps/qcam/viewfinder_qt.cpp > +++ b/src/apps/qcam/viewfinder_qt.cpp > @@ -44,6 +44,10 @@ ViewFinderQt::ViewFinderQt(QWidget *parent) > : QWidget(parent), place_(rect()), buffer_(nullptr) > { > icon_ = QIcon(":camera-off.svg"); > + > + QPalette pal = palette(); > + pal.setColor(QPalette::Window, Qt::black); > + setPalette(pal); > } > > ViewFinderQt::~ViewFinderQt() > @@ -118,6 +122,7 @@ void ViewFinderQt::render(libcamera::FrameBuffer *buffer, Image *image) > } > } > I would probably have a comment here ... > + setAttribute(Qt::WA_OpaquePaintEvent, true); > update(); > > if (buffer) > @@ -133,6 +138,7 @@ void ViewFinderQt::stop() > buffer_ = nullptr; > } > and here ... to say why we are starting and sotpping the paint attributes. > + setAttribute(Qt::WA_OpaquePaintEvent, false); > update(); > } > > @@ -147,8 +153,22 @@ void ViewFinderQt::paintEvent(QPaintEvent *) > { > QPainter painter(this); > > - /* If we have an image, draw it. */ > + painter.setBrush(palette().window()); > + > + /* If we have an image, draw it, with black letterbox rectangles. */ > if (!image_.isNull()) { > + if (place_.width() < width()) { > + QRect rect{ 0, 0, (width() - place_.width()) / 2, height() }; > + painter.drawRect(rect); > + rect.moveLeft(place_.right()); > + painter.drawRect(rect); > + } else { > + QRect rect{ 0, 0, width(), (height() - place_.height()) / 2 }; > + painter.drawRect(rect); > + rect.moveTop(place_.bottom()); > + painter.drawRect(rect); > + } Seems easy enough. Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com> And tested on my X13s However, I can't currently test qcam -r gles it seems which is crashing but that's on mainline too so not this series. > + > painter.drawImage(place_, image_, image_.rect()); > return; > } > @@ -174,7 +194,6 @@ void ViewFinderQt::paintEvent(QPaintEvent *) > else > point.setY((height() - pixmap_.height()) / 2); > > - painter.setBackgroundMode(Qt::OpaqueMode); > painter.drawPixmap(point, pixmap_); > } > > -- > Regards, > > Laurent Pinchart >
On Mon, Sep 09, 2024 at 01:33:54PM +0100, Kieran Bingham wrote: > Quoting Laurent Pinchart (2024-09-05 17:25:07) > > When the widget's aspect ratio doesn't match the camera aspect ratio, > > the viewfinder is rendered letter-boxed. The side rectangles are not > > painted by the viewfinder, and Qt thus renders the parent widget > > background to fill that space. > > > > To make it black, we have two options: > > > > - The simplest option is to set the widget's autoFillBackground property > > to true. This causes Qt to paint the whole widget with its background > > colour before calling paintEvent(). As the camera image typically > > covers most (if not all) of the viewfinder widget, this is less > > efficient. > > > > - The more complicated option is to pain the letterbox rectangles > > s/pain/paint/ > > > manually. We can additionally set the widget's WA_OpaquePaintEvent > > attribute to instruct Qt to skip painting the parent widget. This > > reduces CPU usage by about 1% (and may reduce GPU usage as well). > > > > Note that the WA_OpaquePaintEvent attribute has to be disabled when we > > render the stopped icon, as the icon has a transparent background. > > > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > > --- > > src/apps/qcam/viewfinder_qt.cpp | 23 +++++++++++++++++++++-- > > 1 file changed, 21 insertions(+), 2 deletions(-) > > > > diff --git a/src/apps/qcam/viewfinder_qt.cpp b/src/apps/qcam/viewfinder_qt.cpp > > index 492648cfa2ff..62b6f27fa23e 100644 > > --- a/src/apps/qcam/viewfinder_qt.cpp > > +++ b/src/apps/qcam/viewfinder_qt.cpp > > @@ -44,6 +44,10 @@ ViewFinderQt::ViewFinderQt(QWidget *parent) > > : QWidget(parent), place_(rect()), buffer_(nullptr) > > { > > icon_ = QIcon(":camera-off.svg"); > > + > > + QPalette pal = palette(); > > + pal.setColor(QPalette::Window, Qt::black); > > + setPalette(pal); > > } > > > > ViewFinderQt::~ViewFinderQt() > > @@ -118,6 +122,7 @@ void ViewFinderQt::render(libcamera::FrameBuffer *buffer, Image *image) > > } > > } > > > > I would probably have a comment here ... > > > + setAttribute(Qt::WA_OpaquePaintEvent, true); > > update(); > > > > if (buffer) > > @@ -133,6 +138,7 @@ void ViewFinderQt::stop() > > buffer_ = nullptr; > > } > > > > and here ... > > to say why we are starting and sotpping the paint attributes. > > > + setAttribute(Qt::WA_OpaquePaintEvent, false); > > update(); > > } > > > > @@ -147,8 +153,22 @@ void ViewFinderQt::paintEvent(QPaintEvent *) > > { > > QPainter painter(this); > > > > - /* If we have an image, draw it. */ > > + painter.setBrush(palette().window()); > > + > > + /* If we have an image, draw it, with black letterbox rectangles. */ > > if (!image_.isNull()) { > > + if (place_.width() < width()) { > > + QRect rect{ 0, 0, (width() - place_.width()) / 2, height() }; > > + painter.drawRect(rect); > > + rect.moveLeft(place_.right()); > > + painter.drawRect(rect); > > + } else { > > + QRect rect{ 0, 0, width(), (height() - place_.height()) / 2 }; > > + painter.drawRect(rect); > > + rect.moveTop(place_.bottom()); > > + painter.drawRect(rect); > > + } > > Seems easy enough. > > > Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > > And tested on my X13s > > However, I can't currently test qcam -r gles it seems which is crashing > but that's on mainline too so not this series. It doesn't crash for me, but it doesn't work either :-( The move to Qt6 apparently introduced an issue. > > + > > painter.drawImage(place_, image_, image_.rect()); > > return; > > } > > @@ -174,7 +194,6 @@ void ViewFinderQt::paintEvent(QPaintEvent *) > > else > > point.setY((height() - pixmap_.height()) / 2); > > > > - painter.setBackgroundMode(Qt::OpaqueMode); > > painter.drawPixmap(point, pixmap_); > > } > >
diff --git a/src/apps/qcam/viewfinder_qt.cpp b/src/apps/qcam/viewfinder_qt.cpp index 492648cfa2ff..62b6f27fa23e 100644 --- a/src/apps/qcam/viewfinder_qt.cpp +++ b/src/apps/qcam/viewfinder_qt.cpp @@ -44,6 +44,10 @@ ViewFinderQt::ViewFinderQt(QWidget *parent) : QWidget(parent), place_(rect()), buffer_(nullptr) { icon_ = QIcon(":camera-off.svg"); + + QPalette pal = palette(); + pal.setColor(QPalette::Window, Qt::black); + setPalette(pal); } ViewFinderQt::~ViewFinderQt() @@ -118,6 +122,7 @@ void ViewFinderQt::render(libcamera::FrameBuffer *buffer, Image *image) } } + setAttribute(Qt::WA_OpaquePaintEvent, true); update(); if (buffer) @@ -133,6 +138,7 @@ void ViewFinderQt::stop() buffer_ = nullptr; } + setAttribute(Qt::WA_OpaquePaintEvent, false); update(); } @@ -147,8 +153,22 @@ void ViewFinderQt::paintEvent(QPaintEvent *) { QPainter painter(this); - /* If we have an image, draw it. */ + painter.setBrush(palette().window()); + + /* If we have an image, draw it, with black letterbox rectangles. */ if (!image_.isNull()) { + if (place_.width() < width()) { + QRect rect{ 0, 0, (width() - place_.width()) / 2, height() }; + painter.drawRect(rect); + rect.moveLeft(place_.right()); + painter.drawRect(rect); + } else { + QRect rect{ 0, 0, width(), (height() - place_.height()) / 2 }; + painter.drawRect(rect); + rect.moveTop(place_.bottom()); + painter.drawRect(rect); + } + painter.drawImage(place_, image_, image_.rect()); return; } @@ -174,7 +194,6 @@ void ViewFinderQt::paintEvent(QPaintEvent *) else point.setY((height() - pixmap_.height()) / 2); - painter.setBackgroundMode(Qt::OpaqueMode); painter.drawPixmap(point, pixmap_); }
When the widget's aspect ratio doesn't match the camera aspect ratio, the viewfinder is rendered letter-boxed. The side rectangles are not painted by the viewfinder, and Qt thus renders the parent widget background to fill that space. To make it black, we have two options: - The simplest option is to set the widget's autoFillBackground property to true. This causes Qt to paint the whole widget with its background colour before calling paintEvent(). As the camera image typically covers most (if not all) of the viewfinder widget, this is less efficient. - The more complicated option is to pain the letterbox rectangles manually. We can additionally set the widget's WA_OpaquePaintEvent attribute to instruct Qt to skip painting the parent widget. This reduces CPU usage by about 1% (and may reduce GPU usage as well). Note that the WA_OpaquePaintEvent attribute has to be disabled when we render the stopped icon, as the icon has a transparent background. Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> --- src/apps/qcam/viewfinder_qt.cpp | 23 +++++++++++++++++++++-- 1 file changed, 21 insertions(+), 2 deletions(-)