[{"id":12552,"web_url":"https://patchwork.libcamera.org/comment/12552/","msgid":"<20200916152319.GI1850958@oden.dyn.berto.se>","date":"2020-09-16T15:23:19","subject":"Re: [libcamera-devel] [PATCH 5/7] qcam: viewfinder_gl: Support\n\t#define in shaders","submitter":{"id":5,"url":"https://patchwork.libcamera.org/api/people/5/","name":"Niklas Söderlund","email":"niklas.soderlund@ragnatech.se"},"content":"Hi Laurent,\n\nThanks for your work.\n\nOn 2020-09-16 17:52:52 +0300, Laurent Pinchart wrote:\n> Prepare the infrastructure to support defining preprocessor macros in\n> shaders:\n> \n> - Rename the fragmentShaderSrc_ member to fragmentShaderFile_ to reflect\n> better that it contains a file name, not shader source code\n> - Add a new fragmentShaderDefines_ member to store preprocessor macros\n> - Prepend the macros to the shader source before compiling it\n> \n> Signed-off-by: Laurent Pinchart \n\nReviewed-by: Niklas Söderlund \n\n> ---\n> src/qcam/viewfinder_gl.cpp | 42 +++++++++++++++++++++++++++-----------\n> src/qcam/viewfinder_gl.h | 11 +++++-----\n> 2 files changed, 35 insertions(+), 18 deletions(-)\n> \n> diff --git a/src/qcam/viewfinder_gl.cpp b/src/qcam/viewfinder_gl.cpp\n> index 0fa06a290959..7cb5fb63656f 100644\n> --- a/src/qcam/viewfinder_gl.cpp\n> +++ b/src/qcam/viewfinder_gl.cpp\n> @@ -7,6 +7,8 @@\n> \n> #include \"viewfinder_gl.h\"\n> \n> +#include \n> +#include \n> #include \n> \n> #include \n> @@ -24,7 +26,7 @@ static const QList supportedFormats{\n> \n> ViewFinderGL::ViewFinderGL(QWidget *parent)\n> \t: QOpenGLWidget(parent), buffer_(nullptr), yuvData_(nullptr),\n> -\t fragmentShader_(nullptr), vertexShader_(nullptr),\n> +\t vertexShader_(nullptr), fragmentShader_(nullptr),\n> \t vertexBuffer_(QOpenGLBuffer::VertexBuffer),\n> \t textureU_(QOpenGLTexture::Target2D),\n> \t textureV_(QOpenGLTexture::Target2D),\n> @@ -97,46 +99,49 @@ void ViewFinderGL::render(libcamera::FrameBuffer *buffer, MappedBuffer *map)\n> bool ViewFinderGL::selectFormat(const libcamera::PixelFormat &format)\n> {\n> \tbool ret = true;\n> +\n> +\tfragmentShaderDefines_.clear();\n> +\n> \tswitch (format) {\n> \tcase libcamera::formats::NV12:\n> \t\thorzSubSample_ = 2;\n> \t\tvertSubSample_ = 2;\n> -\t\tfragmentShaderSrc_ = \":YUV_2_planes_UV.frag\";\n> +\t\tfragmentShaderFile_ = \":YUV_2_planes_UV.frag\";\n> \t\tbreak;\n> \tcase libcamera::formats::NV21:\n> \t\thorzSubSample_ = 2;\n> \t\tvertSubSample_ = 2;\n> -\t\tfragmentShaderSrc_ = \":YUV_2_planes_VU.frag\";\n> +\t\tfragmentShaderFile_ = \":YUV_2_planes_VU.frag\";\n> \t\tbreak;\n> \tcase libcamera::formats::NV16:\n> \t\thorzSubSample_ = 2;\n> \t\tvertSubSample_ = 1;\n> -\t\tfragmentShaderSrc_ = \":YUV_2_planes_UV.frag\";\n> +\t\tfragmentShaderFile_ = \":YUV_2_planes_UV.frag\";\n> \t\tbreak;\n> \tcase libcamera::formats::NV61:\n> \t\thorzSubSample_ = 2;\n> \t\tvertSubSample_ = 1;\n> -\t\tfragmentShaderSrc_ = \":YUV_2_planes_VU.frag\";\n> +\t\tfragmentShaderFile_ = \":YUV_2_planes_VU.frag\";\n> \t\tbreak;\n> \tcase libcamera::formats::NV24:\n> \t\thorzSubSample_ = 1;\n> \t\tvertSubSample_ = 1;\n> -\t\tfragmentShaderSrc_ = \":YUV_2_planes_UV.frag\";\n> +\t\tfragmentShaderFile_ = \":YUV_2_planes_UV.frag\";\n> \t\tbreak;\n> \tcase libcamera::formats::NV42:\n> \t\thorzSubSample_ = 1;\n> \t\tvertSubSample_ = 1;\n> -\t\tfragmentShaderSrc_ = \":YUV_2_planes_VU.frag\";\n> +\t\tfragmentShaderFile_ = \":YUV_2_planes_VU.frag\";\n> \t\tbreak;\n> \tcase libcamera::formats::YUV420:\n> \t\thorzSubSample_ = 2;\n> \t\tvertSubSample_ = 2;\n> -\t\tfragmentShaderSrc_ = \":YUV_3_planes.frag\";\n> +\t\tfragmentShaderFile_ = \":YUV_3_planes.frag\";\n> \t\tbreak;\n> \tcase libcamera::formats::YVU420:\n> \t\thorzSubSample_ = 2;\n> \t\tvertSubSample_ = 2;\n> -\t\tfragmentShaderSrc_ = \":YUV_3_planes.frag\";\n> +\t\tfragmentShaderFile_ = \":YUV_3_planes.frag\";\n> \t\tbreak;\n> \tdefault:\n> \t\tret = false;\n> @@ -168,11 +173,24 @@ bool ViewFinderGL::createFragmentShader()\n> \tint attributeVertex;\n> \tint attributeTexture;\n> \n> -\t/* Create Fragment Shader */\n> +\t/*\n> +\t * Create the fragment shader, compile it, and add it to the shader\n> +\t * program. The #define macros stored in fragmentShaderDefines_, if\n> +\t * any, are prepended to the source code.\n> +\t */\n> \tfragmentShader_ = new QOpenGLShader(QOpenGLShader::Fragment, this);\n> \n> -\t/* Compile the fragment shader */\n> -\tif (!fragmentShader_->compileSourceFile(fragmentShaderSrc_)) {\n> +\tQFile file(fragmentShaderFile_);\n> +\tif (!file.open(QIODevice::ReadOnly | QIODevice::Text)) {\n> +\t\tqWarning() << \"Shader\" << fragmentShaderFile_ << \"not found\";\n> +\t\treturn false;\n> +\t}\n> +\n> +\tQString defines = fragmentShaderDefines_.join('\\n') + \"\\n\";\n> +\tQByteArray src = file.readAll();\n> +\tsrc.prepend(defines.toUtf8());\n> +\n> +\tif (!fragmentShader_->compileSourceCode(src)) {\n> \t\tqWarning() << \"[ViewFinderGL]:\" << fragmentShader_->log();\n> \t\treturn false;\n> \t}\n> diff --git a/src/qcam/viewfinder_gl.h b/src/qcam/viewfinder_gl.h\n> index 7675d0a06351..53424dc10bc5 100644\n> --- a/src/qcam/viewfinder_gl.h\n> +++ b/src/qcam/viewfinder_gl.h\n> @@ -65,17 +65,16 @@ private:\n> \tQSize size_;\n> \tunsigned char *yuvData_;\n> \n> -\t/* OpenGL components for rendering */\n> -\tQOpenGLShader *fragmentShader_;\n> -\tQOpenGLShader *vertexShader_;\n> +\t/* Shaders */\n> \tQOpenGLShaderProgram shaderProgram_;\n> +\tQOpenGLShader *vertexShader_;\n> +\tQOpenGLShader *fragmentShader_;\n> +\tQString fragmentShaderFile_;\n> +\tQStringList fragmentShaderDefines_;\n> \n> \t/* Vertex buffer */\n> \tQOpenGLBuffer vertexBuffer_;\n> \n> -\t/* Fragment shader file name */\n> -\tQString fragmentShaderSrc_;\n> -\n> \t/* YUV texture planars and parameters */\n> \tGLuint textureUniformU_;\n> \tGLuint textureUniformV_;\n> -- \n> Regards,\n> \n> Laurent Pinchart\n> \n> _______________________________________________\n> libcamera-devel mailing list\n> libcamera-devel@lists.libcamera.org\n> https://lists.libcamera.org/listinfo/libcamera-devel","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id AC38CC3B5B\n\tfor ;\n\tWed, 16 Sep 2020 15:23:22 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 384EC61E2B;\n\tWed, 16 Sep 2020 17:23:22 +0200 (CEST)","from mail-lf1-x142.google.com (mail-lf1-x142.google.com\n\t[IPv6:2a00:1450:4864:20::142])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id A110A60533\n\tfor ;\n\tWed, 16 Sep 2020 17:23:20 +0200 (CEST)","by mail-lf1-x142.google.com with SMTP id z19so7443810lfr.4\n\tfor ;\n\tWed, 16 Sep 2020 08:23:20 -0700 (PDT)","from localhost (h-209-203.A463.priv.bahnhof.se. [155.4.209.203])\n\tby smtp.gmail.com with ESMTPSA id\n\tx5sm4633592lfd.119.2020.09.16.08.23.19\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tWed, 16 Sep 2020 08:23:19 -0700 (PDT)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=ragnatech-se.20150623.gappssmtp.com\n\theader.i=@ragnatech-se.20150623.gappssmtp.com\n\theader.b=\"SoHshafs\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=ragnatech-se.20150623.gappssmtp.com; s=20150623;\n\th=date:from:to:cc:subject:message-id:references:mime-version\n\t:content-disposition:content-transfer-encoding:in-reply-to;\n\tbh=BDpV4HLVyNB1k0AOdOh0IRsL+ci834X9lgxQnbED3aY=;\n\tb=SoHshafs9F3uEFw8f90RgMU7amRgJF5dmy20mP6Uuz5prKxQJEGZN09xIB7NvE5t/Z\n\t7an800S/dboFCgUL34jzpWcEkgIKPq3vwU1Skpg5vHCjQg102iWCdKPmnIikJEKYkLUe\n\tHusXottbmkz4DOKnhZBvYBGQJ8l1c8I9qqKReL2K/Ey1K42T6Es3vK5n3qILB8vA8fKh\n\tnrzAEhuCq5330D6wsPkKlMQfXXz157JbkmZp/Rf9+sLf8JeUpNZiUtcBZ34E90qewqB5\n\tSDZd6lbTyT3xXR1tWI/Ejz71TIJNh+6ANIsKVh7xnAOmNhH5wV+FAcKyzTE5wPLjXV3d\n\tZI1w==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:date:from:to:cc:subject:message-id:references\n\t:mime-version:content-disposition:content-transfer-encoding\n\t:in-reply-to;\n\tbh=BDpV4HLVyNB1k0AOdOh0IRsL+ci834X9lgxQnbED3aY=;\n\tb=VKNkGamnhHgNu4MXF/BjKwcaIfF6BTk3u+koS2saYt3WfkyGl5P2oKqP5R2dW7iMZV\n\tJvutp84sliaZpb9Qc/YpN3rRYoX6LNAnhO+0K05z3XAsWCRH4amjesyEvvbLBYIssIEh\n\txHsanmPMw4kokqVLAc//j/fHilWkFiu8dzJJvjBpeEiEEDV1zUc3jjU2r5XlUL1BpFNn\n\t3IsMOa5fvnxNfIz+3XhBINFW4a9VOXleO16hnUUOnbTIJvfcXs2wmlPBg38igiGibYn2\n\tki+Fo10N2OctSsSC3Jtokf9CHWut/GUuS5EADdim/CpFv7UPHE63/j1fbGdMn4d7cmQ7\n\tDnUQ==","X-Gm-Message-State":"AOAM530AWu4t7TOeY52b8JhtFMusNYKK0XDqmPnmwnLM8+AJuxwJGO1e\n\t178yeV6k1mcwh4ziHaeYGoNNCT0W49zurQ==","X-Google-Smtp-Source":"ABdhPJzgQVKT4SrnTt1i/YfJygv8dx+IJOp3yiA/lSk3UokaLpaah7Ffx3b90/Mbk9ajTSTcUiT/sA==","X-Received":"by 2002:ac2:5541:: with SMTP id l1mr8959753lfk.89.1600269800009; \n\tWed, 16 Sep 2020 08:23:20 -0700 (PDT)","Date":"Wed, 16 Sep 2020 17:23:19 +0200","From":"Niklas =?iso-8859-1?q?S=F6derlund?= ","To":"Laurent Pinchart ","Message-ID":"<20200916152319.GI1850958@oden.dyn.berto.se>","References":"<20200916145254.1644-1-laurent.pinchart@ideasonboard.com>\n\t<20200916145254.1644-6-laurent.pinchart@ideasonboard.com>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20200916145254.1644-6-laurent.pinchart@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH 5/7] qcam: viewfinder_gl: Support\n\t#define in shaders","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"libcamera-devel@lists.libcamera.org","Content-Type":"text/plain; charset=\"iso-8859-1\"","Content-Transfer-Encoding":"quoted-printable","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]