[libcamera-devel] [PATCH v2 3/3] qcam: viewfinder_qt: Support X RGB variants

[Laurent Pinchart]

Hi Kieran,

Thank you for the patch.

On Wed, Jul 06, 2022 at 10:06:42AM +0100, Kieran Bingham via libcamera-devel wrote:
> Support the X variants of the RGB pixel formats
> along side the equivalent Alpha component based versions.

s/along side/alongside/

(you can reflow the text too)

> The Alpha channel is ignored by both QImage::Format_RGB32, and
> QImage::Format_RGBX8888.

That's not guaranteed by the Qt documentation as far as I understand.
I'd write

The QImage::Format_RGB32 and QImage::Format_RGBX8888 formats only
specify that the alpha component must be 0xff. While the Qt
documentation doesn't guarantee that the alpha value will be ignored by
consumers, this seems to be the implemented behaviour, at least when
rendering with QPainter::drawImage().

Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>

> The existing use of QImage::Format_RGBA8888 is updated to use the
> QImage::Format_RGBX8888 variant to ensure that the image data is
> visible.
> 
> Signed-off-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
> ---
> 
> v2:
>  - Update to use QImage::Format_RGBX8888 over QImage::Format_RGBA8888
> 
>  src/qcam/viewfinder_qt.cpp | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/src/qcam/viewfinder_qt.cpp b/src/qcam/viewfinder_qt.cpp
> index 27955e3f9593..7a6a60c96393 100644
> --- a/src/qcam/viewfinder_qt.cpp
> +++ b/src/qcam/viewfinder_qt.cpp
> @@ -27,9 +27,11 @@
>  static const QMap<libcamera::PixelFormat, QImage::Format> nativeFormats
>  {
>  #if QT_VERSION >= QT_VERSION_CHECK(5, 2, 0)
> -	{ libcamera::formats::ABGR8888, QImage::Format_RGBA8888 },
> +	{ libcamera::formats::ABGR8888, QImage::Format_RGBX8888 },
> +	{ libcamera::formats::XBGR8888, QImage::Format_RGBX8888 },
>  #endif
>  	{ libcamera::formats::ARGB8888, QImage::Format_RGB32 },
> +	{ libcamera::formats::XRGB8888, QImage::Format_RGB32 },
>  #if QT_VERSION >= QT_VERSION_CHECK(5, 14, 0)
>  	{ libcamera::formats::RGB888, QImage::Format_BGR888 },
>  #endif

-- 
Regards,

Laurent Pinchart