[{"id":17500,"web_url":"https://patchwork.libcamera.org/comment/17500/","msgid":"<YME48ZD4sXLVMa15@pendragon.ideasonboard.com>","date":"2021-06-09T21:56:01","subject":"Re: [libcamera-devel] [PATCH v2 2/5] qcam: viewfinder_gl: Add\n\tshader to render packed RAW10 formats","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Andrey,\n\nThank you for the patch.\n\nOn Thu, May 27, 2021 at 01:55:08PM +0300, Andrey Konovalov wrote:\n> The shader supports all 4 packed RAW10 variants.\n> Simple bi-linear filtering is implemented.\n\nI had initially interpreted this as bi-linear filtering for the scaling\n(when the window size doesn't match the texture size), but it's for the\nBayer interpolation. Can we make it clear by writing\n\nSimple bi-linear Bayer interpolation of nearest pixels is implemented.\n\n?\n\n> The 2 LS bits of the 10-bit colour values are dropped as the RGBA\n> format we convert into has only 8 bits per colour.\n> \n> The texture coordinates passed to the fragment shader are ajusted\n> to point to the nearest pixel in the image. This prevents artifacts\n> when the image is scaled from the frame resolution to the window size.\n>\n> Signed-off-by: Andrey Konovalov <andrey.konovalov@linaro.org>\n> ---\n>  src/qcam/assets/shader/bayer_1x_packed.frag | 174 ++++++++++++++++++++\n>  src/qcam/assets/shader/shaders.qrc          |   1 +\n>  src/qcam/viewfinder_gl.cpp                  |  77 ++++++++-\n>  src/qcam/viewfinder_gl.h                    |   6 +\n>  4 files changed, 256 insertions(+), 2 deletions(-)\n>  create mode 100644 src/qcam/assets/shader/bayer_1x_packed.frag\n> \n> diff --git a/src/qcam/assets/shader/bayer_1x_packed.frag b/src/qcam/assets/shader/bayer_1x_packed.frag\n> new file mode 100644\n> index 00000000..0a87c6db\n> --- /dev/null\n> +++ b/src/qcam/assets/shader/bayer_1x_packed.frag\n> @@ -0,0 +1,174 @@\n> +/* SPDX-License-Identifier: BSD-2-Clause */\n> +/*\n> + * Based on the code from http://jgt.akpeters.com/papers/McGuire08/\n> + *\n> + * Efficient, High-Quality Bayer Demosaic Filtering on GPUs\n> + *\n> + * Morgan McGuire\n> + *\n> + * This paper appears in issue Volume 13, Number 4.\n> + * ---------------------------------------------------------\n> + * Copyright (c) 2008, Morgan McGuire. All rights reserved.\n> + *\n> + *\n> + * Modified by Linaro Ltd for 10/12-bit packed vs 8-bit raw Bayer format,\n> + * and for simpler demosaic algorithm.\n> + * Copyright (C) 2020, Linaro\n> + *\n> + * bayer_1x_packed.frag - Fragment shader code for raw Bayer 10-bit and 12-bit\n> + * packed formats\n> + */\n> +\n> +#ifdef GL_ES\n> +precision mediump float;\n> +#endif\n> +\n> +varying vec2 textureOut;\n> +\n> +/* the texture size: tex_size.xy is in bytes, tex_size.zw is in pixels */\n> +uniform vec4 tex_size;\n\ntex_size.xy isn't used anywhere as far as I can see. Could it be turned\ninto a vec2 and only store the size in pixels, or do you expect the size\nin bytes to be needed later ?\n\n> +uniform vec2 tex_step;\n> +uniform vec2 tex_bayer_first_red;\n> +\n> +uniform sampler2D tex_raw;\n> +\n> +void main(void)\n> +{\n> +\tvec3 rgb;\n> +\n> +\t/*\n> +\t * center.xy holds the coordinates of the pixel being sampled\n> +\t * on the [0, 1] range.\n\nLooking at the code, this doesn't seem to be correct. I think it holds\nthe value in bytes, while .zw is in pixels.\n\nWould it be more readable to create two vec2 variables named\ncenter_pixel and center_bytes ? Or would this have an impact on\nperformance ?\n\n> +\t * center.zw holds the coordinates of the pixel being sampled\n> +\t * on the [0, width/height-1] range.\n> +\t */\n> +\tvec4 center;\n\nCould you please add blank lines before every comment block ?\n\n> +\t/*\n> +\t * x-positions of the adjacent pixels on the [0, 1] range.\n> +\t */\n> +\tvec2 xcoords;\n> +\t/*\n> +\t * y-positions of the adjacent pixels on the [0, 1] range.\n> +\t */\n> +\tvec2 ycoords;\n> +\n> +\t/*\n> +\t * The coordinates passed to the shader in textureOut may point\n> +\t * to a place in between the pixels if the viewfinder window is scaled\n> +\t * from the original captured frame size. Align them to the nearest\n> +\t * pixel.\n> +\t */\n> +\tcenter.zw = floor(textureOut * tex_size.zw);\n\nUnless I'm mistaken, this doesn't align to the nearest pixel, but to the\nprevious one. We could use round() instead, but that would require GLSL\n1.30.\n\nCan textureOut really point to between pixels, given that\ntextureMinMagFilters_ is set to GL_NEAREST ?\n\n> +\tcenter.y = center.w;\n> +\t/*\n> +\t * Add a small number (a few mantissa's LSBs) to avoid float\n> +\t * representation issues. Maybe paranoic.\n> +\t */\n> +\tcenter.x = BPP_X * center.z + 0.02;\n> +\n> +\tconst float threshold_l = 0.127 /* fract(BPP_X) * 0.5 + 0.02 */;\n> +\tconst float threshold_h = 0.625 /* 1.0 - fract(BPP_X) * 1.5 */;\n> +\n> +\tfloat fract_x = fract(center.x);\n> +\t/*\n> +\t * The below floor() call ensures that center.x points\n> +\t * at one of the bytes representing the 8 higher bits of\n> +\t * the pixel value, not at the byte containing the LS bits\n> +\t * of the group of the pixels.\n> +\t */\n> +\tcenter.x = floor(center.x);\n> +\tcenter.xy *= tex_step;\n> +\n> +\txcoords = center.x + vec2(-tex_step.x, tex_step.x);\n> +\tycoords = center.y + vec2(-tex_step.y, tex_step.y);\n> +\t/*\n> +\t * If xcoords[0] points at the byte containing the LS bits\n> +         * of the previous group of the pixels, move xcoords[0] one\n\nIncorrect indentation here (and same below).\n\n> +\t * byte back.\n> +\t */\n> +\txcoords[0] += (fract_x < threshold_l) ? -tex_step.x : 0.0;\n> +\t/*\n> +\t * If xcoords[1] points at the byte containing the LS bits\n> +         * of the current group of the pixels, move xcoords[1] one\n> +\t * byte forward.\n> +\t */\n> +\txcoords[1] += (fract_x > threshold_h) ? tex_step.x : 0.0;\n> +\n> +\tvec2 alternate = mod(center.zw + tex_bayer_first_red, 2.0);\n> +\tbool even_col = alternate.x < 1.0;\n> +\tbool even_raw = alternate.y < 1.0;\n\ns/raw/row/\n\n> +\n> +\t/*\n> +\t * We need to sample the central pixel and the ones with offset\n> +\t * of -1 to +1 pixel in both X and Y directions. Let's name these\n> +\t * pixels as below, where C is the central pixel:\n\nI'd add a blank line here and a few below too.\n\n> +\t *   +----+----+----+----+\n> +\t *   | \\ x|    |    |    |\n> +\t *   |y \\ | -1 |  0 | +1 | \n> +\t *   +----+----+----+----+\n> +\t *   | +1 | D2 | A1 | D3 |\n> +\t *   +----+----+----+----+\n> +\t *   |  0 | B0 |  C | B1 |\n> +\t *   +----+----+----+----+\n> +\t *   | -1 | D0 | A0 | D1 |\n> +\t *   +----+----+----+----+\n> +\t * In the below equations (0,-1).r means \"r component of the texel\n> +\t * shifted by -tex_step.y from the center.xy one\" etc.\n> +\t * In the even raw / even column (EE) case the colour values are:\n\ns/raw/row/ (same below).\n\n> +\t *   R = C = (0,0).r,\n> +\t *   G = (A0 + A1 + B0 + B1) / 4.0 =\n> +\t *       ( (0,-1).r + (0,1).r + (-1,0).r + (1,0).r ) / 4.0,\n> +\t *   B = (D0 + D1 + D2 + D3) / 4.0 =\n> +\t *       ( (-1,-1).r + (1,-1).r + (-1,1).r + (1,1).r ) / 4.0\n> +\t * For even raw / odd column (EO):\n> +\t *   R = (B0 + B1) / 2.0 = ( (-1,0).r + (1,0).r ) / 2.0,\n> +\t *   G = C = (0,0).r,\n> +\t *   B = (A0 + A1) / 2.0 = ( (0,-1).r + (0,1).r ) / 2.0\n> +\t * For odd raw / even column (OE):\n> +\t *   R = (A0 + A1) / 2.0 = ( (0,-1).r + (0,1).r ) / 2.0,\n> +\t *   G = C = (0,0).r,\n> +\t *   B = (B0 + B1) / 2.0 = ( (-1,0).r + (1,0).r ) / 2.0\n> +\t * For odd raw / odd column (OO):\n> +\t *   R = (D0 + D1 + D2 + D3) / 4.0 =\n> +\t *       ( (-1,-1).r + (1,-1).r + (-1,1).r + (1,1).r ) / 4.0,\n> +\t *   G = (A0 + A1 + B0 + B1) / 4.0 =\n> +\t *       ( (0,-1).r + (0,1).r + (-1,0).r + (1,0).r ) / 4.0,\n> +\t *   B = C = (0,0).r\n> +\t */\n> +\n> +\t/*\n> +\t * Fetch the values and precalculate the terms:\n> +\t *   patterns.x = (A0 + A1) / 2.0\n> +\t *   patterns.y = (B0 + B1) / 2.0\n> +\t *   patterns.z = (A0 + A1 + B0 + B1) / 4.0\n> +\t *   patterns.w = (D0 + D1 + D2 + D3) / 4.0\n> +\t */\n> +\t#define fetch(x, y) texture2D(tex_raw, vec2(x, y)).r\n> +\n> +\tfloat C = texture2D(tex_raw, center.xy).r;\n> +\tvec4 patterns = vec4(\n> +\t\tfetch(center.x, ycoords[0]),\t/* A0: (0,-1) */\n> +\t\tfetch(xcoords[0], center.y),\t/* B0: (-1,0) */\n> +\t\tfetch(xcoords[0], ycoords[0]),\t/* D0: (-1,-1) */\n> +\t\tfetch(xcoords[1], ycoords[0]));\t/* D1: (1,-1) */\n> +\tvec4 temp = vec4(\n> +\t\tfetch(center.x, ycoords[1]),\t/* A1: (0,1) */\n> +\t\tfetch(xcoords[1], center.y),\t/* B1: (1,0) */\n> +\t\tfetch(xcoords[1], ycoords[1]),\t/* D3: (1,1) */\n> +\t\tfetch(xcoords[0], ycoords[1]));\t/* D2: (-1,1) */\n> +\tpatterns = (patterns + temp) * 0.5;\n> +\t\t/* .x = (A0 + A1) / 2.0, .y = (B0 + B1) / 2.0 */\n> +\t\t/* .z = (D0 + D3) / 2.0, .w = (D1 + D2) / 2.0 */\n> +\tpatterns.w = (patterns.z + patterns.w) * 0.5;\n> +\tpatterns.z = (patterns.x + patterns.y) * 0.5;\n> +\n> +\trgb = (even_col) ?\n> +\t\t((even_raw) ?\n> +\t\t\tvec3(C, patterns.zw) :\n> +\t\t\tvec3(patterns.x, C, patterns.y)) :\n> +\t\t((even_raw) ?\n> +\t\t\tvec3(patterns.y, C, patterns.x) :\n> +\t\t\tvec3(patterns.wz, C));\n\nNo need for parentheses around even_col and even_raw.\n\n> +\n> +\tgl_FragColor = vec4(rgb, 1.0);\n> +}\n> diff --git a/src/qcam/assets/shader/shaders.qrc b/src/qcam/assets/shader/shaders.qrc\n> index 8a8f9de1..d76d65c5 100644\n> --- a/src/qcam/assets/shader/shaders.qrc\n> +++ b/src/qcam/assets/shader/shaders.qrc\n> @@ -5,6 +5,7 @@\n>  \t<file>YUV_2_planes.frag</file>\n>  \t<file>YUV_3_planes.frag</file>\n>  \t<file>YUV_packed.frag</file>\n> +\t<file>bayer_1x_packed.frag</file>\n>  \t<file>identity.vert</file>\n>  </qresource>\n>  </RCC>\n> diff --git a/src/qcam/viewfinder_gl.cpp b/src/qcam/viewfinder_gl.cpp\n> index ff719418..b324d77f 100644\n> --- a/src/qcam/viewfinder_gl.cpp\n> +++ b/src/qcam/viewfinder_gl.cpp\n> @@ -36,6 +36,11 @@ static const QList<libcamera::PixelFormat> supportedFormats{\n>  \tlibcamera::formats::RGBA8888,\n>  \tlibcamera::formats::BGR888,\n>  \tlibcamera::formats::RGB888,\n> +\t/* Raw Bayer 10-bit packed */\n> +\tlibcamera::formats::SBGGR10_CSI2P,\n> +\tlibcamera::formats::SGBRG10_CSI2P,\n> +\tlibcamera::formats::SGRBG10_CSI2P,\n> +\tlibcamera::formats::SRGGB10_CSI2P,\n>  };\n>  \n>  ViewFinderGL::ViewFinderGL(QWidget *parent)\n> @@ -114,6 +119,9 @@ bool ViewFinderGL::selectFormat(const libcamera::PixelFormat &format)\n>  {\n>  \tbool ret = true;\n>  \n> +\t/* Set min/mag filters to GL_LINEAR by default. */\n> +\ttextureMinMagFilters_ = GL_LINEAR;\n> +\n>  \tfragmentShaderDefines_.clear();\n>  \n>  \tswitch (format) {\n> @@ -203,6 +211,34 @@ bool ViewFinderGL::selectFormat(const libcamera::PixelFormat &format)\n>  \t\tfragmentShaderDefines_.append(\"#define RGB_PATTERN bgr\");\n>  \t\tfragmentShaderFile_ = \":RGB.frag\";\n>  \t\tbreak;\n> +\tcase libcamera::formats::SBGGR10_CSI2P:\n> +\t\tfirstRed_.setX(1.0);\n> +\t\tfirstRed_.setY(1.0);\n> +\t\tfragmentShaderDefines_.append(\"#define BPP_X 1.25\");\n> +\t\tfragmentShaderFile_ = \":bayer_1x_packed.frag\";\n> +\t\ttextureMinMagFilters_ = GL_NEAREST;\n> +\t\tbreak;\n> +\tcase libcamera::formats::SGBRG10_CSI2P:\n> +\t\tfirstRed_.setX(0.0);\n> +\t\tfirstRed_.setY(1.0);\n> +\t\tfragmentShaderDefines_.append(\"#define BPP_X 1.25\");\n> +\t\tfragmentShaderFile_ = \":bayer_1x_packed.frag\";\n> +\t\ttextureMinMagFilters_ = GL_NEAREST;\n> +\t\tbreak;\n> +\tcase libcamera::formats::SGRBG10_CSI2P:\n> +\t\tfirstRed_.setX(1.0);\n> +\t\tfirstRed_.setY(0.0);\n> +\t\tfragmentShaderDefines_.append(\"#define BPP_X 1.25\");\n> +\t\tfragmentShaderFile_ = \":bayer_1x_packed.frag\";\n> +\t\ttextureMinMagFilters_ = GL_NEAREST;\n> +\t\tbreak;\n> +\tcase libcamera::formats::SRGGB10_CSI2P:\n> +\t\tfirstRed_.setX(0.0);\n> +\t\tfirstRed_.setY(0.0);\n> +\t\tfragmentShaderDefines_.append(\"#define BPP_X 1.25\");\n> +\t\tfragmentShaderFile_ = \":bayer_1x_packed.frag\";\n> +\t\ttextureMinMagFilters_ = GL_NEAREST;\n> +\t\tbreak;\n>  \tdefault:\n>  \t\tret = false;\n>  \t\tqWarning() << \"[ViewFinderGL]:\"\n> @@ -290,6 +326,8 @@ bool ViewFinderGL::createFragmentShader()\n>  \ttextureUniformU_ = shaderProgram_.uniformLocation(\"tex_u\");\n>  \ttextureUniformV_ = shaderProgram_.uniformLocation(\"tex_v\");\n>  \ttextureUniformStep_ = shaderProgram_.uniformLocation(\"tex_step\");\n> +\ttextureUniformSize_ = shaderProgram_.uniformLocation(\"tex_size\");\n> +\ttextureUniformBayerFirstRed_ = shaderProgram_.uniformLocation(\"tex_bayer_first_red\");\n>  \n>  \t/* Create the textures. */\n>  \tfor (std::unique_ptr<QOpenGLTexture> &texture : textures_) {\n> @@ -306,8 +344,10 @@ bool ViewFinderGL::createFragmentShader()\n>  void ViewFinderGL::configureTexture(QOpenGLTexture &texture)\n>  {\n>  \tglBindTexture(GL_TEXTURE_2D, texture.textureId());\n> -\tglTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MAG_FILTER, GL_LINEAR);\n> -\tglTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MIN_FILTER, GL_LINEAR);\n> +\tglTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MAG_FILTER,\n> +\t\t\ttextureMinMagFilters_);\n> +\tglTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MIN_FILTER,\n> +\t\t\ttextureMinMagFilters_);\n>  \tglTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_WRAP_S, GL_CLAMP_TO_EDGE);\n>  \tglTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_WRAP_T, GL_CLAMP_TO_EDGE);\n>  }\n> @@ -547,6 +587,39 @@ void ViewFinderGL::doRender()\n>  \t\tshaderProgram_.setUniformValue(textureUniformY_, 0);\n>  \t\tbreak;\n>  \n> +\tcase libcamera::formats::SBGGR10_CSI2P:\n> +\tcase libcamera::formats::SGBRG10_CSI2P:\n> +\tcase libcamera::formats::SGRBG10_CSI2P:\n> +\tcase libcamera::formats::SRGGB10_CSI2P:\n> +\t\t/*\n> +\t\t * Packed raw Bayer 10-bit formats are stored in GL_RED texture.\n> +\t\t * The texture width is 10/8 of the image width.\n> +\t\t * TODO: account for padding bytes at the end of the line.\n\ns/TODO:/\\todo/\n\nBut it's fixed in 3/5, which seems to have forgotten to drop this line.\n\n> +\t\t */\n> +\t\tglActiveTexture(GL_TEXTURE0);\n> +\t\tconfigureTexture(*textures_[0]);\n> +\t\tglTexImage2D(GL_TEXTURE_2D,\n> +\t\t\t     0,\n> +\t\t\t     GL_RED,\n> +\t\t\t     size_.width() * 5 / 4,\n> +\t\t\t     size_.height(),\n> +\t\t\t     0,\n> +\t\t\t     GL_RED,\n> +\t\t\t     GL_UNSIGNED_BYTE,\n> +\t\t\t     data_);\n> +\t\tshaderProgram_.setUniformValue(textureUniformY_, 0);\n> +\t\tshaderProgram_.setUniformValue(textureUniformBayerFirstRed_,\n> +\t\t\t\t\t       firstRed_);\n> +\t\tshaderProgram_.setUniformValue(textureUniformSize_,\n> +\t\t\t\t\t       size_.width() * 5 / 4,\n> +\t\t\t\t\t       size_.height(),\n> +\t\t\t\t\t       size_.width(),\n> +\t\t\t\t\t       size_.height());\n> +\t\tshaderProgram_.setUniformValue(textureUniformStep_,\n> +\t\t\t\t\t       1.0f / (size_.width() * 5 / 4 - 1),\n> +\t\t\t\t\t       1.0f / (size_.height() - 1));\n> +\t\tbreak;\n> +\n>  \tdefault:\n>  \t\tbreak;\n>  \t};\n> diff --git a/src/qcam/viewfinder_gl.h b/src/qcam/viewfinder_gl.h\n> index 1b1faa91..337718e3 100644\n> --- a/src/qcam/viewfinder_gl.h\n> +++ b/src/qcam/viewfinder_gl.h\n> @@ -81,13 +81,19 @@ private:\n>  \t/* Textures */\n>  \tstd::array<std::unique_ptr<QOpenGLTexture>, 3> textures_;\n>  \n> +\t/* Common texture parameters */\n> +\tGLuint textureMinMagFilters_;\n\nA blank line would be nice here.\n\n>  \t/* YUV texture parameters */\n>  \tGLuint textureUniformU_;\n>  \tGLuint textureUniformV_;\n>  \tGLuint textureUniformY_;\n> +\tGLuint textureUniformSize_;\n\nThis is for Bayer textures, so it should be moved below.\n\n>  \tGLuint textureUniformStep_;\n>  \tunsigned int horzSubSample_;\n>  \tunsigned int vertSubSample_;\n\nAnd here too.\n\n> +\t/* Raw Bayer texture parameters */\n> +\tGLuint textureUniformBayerFirstRed_;\n> +\tQPointF firstRed_;\n>  \n>  \tQMutex mutex_; /* Prevent concurrent access to image_ */\n>  };","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","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 04DC2BD78E\n\tfor <parsemail@patchwork.libcamera.org>;\n\tWed,  9 Jun 2021 21:56:22 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 6099268928;\n\tWed,  9 Jun 2021 23:56:21 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 4E604602A1\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tWed,  9 Jun 2021 23:56:19 +0200 (CEST)","from pendragon.ideasonboard.com (62-78-145-57.bb.dnainternet.fi\n\t[62.78.145.57])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id BFDC2D88;\n\tWed,  9 Jun 2021 23:56:18 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"NM7yaFQt\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1623275778;\n\tbh=/xAEoH34tH8RbHdR0AQygM48qWTCfmrJLtNBDvw7Ng0=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=NM7yaFQtLgSXD60O6uQahjKohdxBAeazd1e5sBf6CAzIg69Lsm58vlt6sgJalSNC/\n\tAvoCCkvkqwkN3Yv42s3snU8yZym7s4O2YVrUd6GEolGpBfRnGjZBUZ8McQQjkg4q4U\n\tKowuQbkfQbt04QKgaxw4iTvNadkuisD35gcBZYYg=","Date":"Thu, 10 Jun 2021 00:56:01 +0300","From":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","To":"Andrey Konovalov <andrey.konovalov@linaro.org>","Message-ID":"<YME48ZD4sXLVMa15@pendragon.ideasonboard.com>","References":"<20210527105511.447089-1-andrey.konovalov@linaro.org>\n\t<20210527105511.447089-3-andrey.konovalov@linaro.org>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20210527105511.447089-3-andrey.konovalov@linaro.org>","Subject":"Re: [libcamera-devel] [PATCH v2 2/5] qcam: viewfinder_gl: Add\n\tshader to render packed RAW10 formats","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Cc":"morgan@casual-effects.com, libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":17577,"web_url":"https://patchwork.libcamera.org/comment/17577/","msgid":"<c97567b0-9f6f-6b75-da59-722dd47d211a@linaro.org>","date":"2021-06-15T18:00:57","subject":"Re: [libcamera-devel] [PATCH v2 2/5] qcam: viewfinder_gl: Add\n\tshader to render packed RAW10 formats","submitter":{"id":25,"url":"https://patchwork.libcamera.org/api/people/25/","name":"Andrey Konovalov","email":"andrey.konovalov@linaro.org"},"content":"Hi Laurent,\n\nThank you for reviewing these patches!\n\nOn 10.06.2021 00:56, Laurent Pinchart wrote:\n> Hi Andrey,\n> \n> Thank you for the patch.\n> \n> On Thu, May 27, 2021 at 01:55:08PM +0300, Andrey Konovalov wrote:\n>> The shader supports all 4 packed RAW10 variants.\n>> Simple bi-linear filtering is implemented.\n> \n> I had initially interpreted this as bi-linear filtering for the scaling\n> (when the window size doesn't match the texture size), but it's for the\n> Bayer interpolation. Can we make it clear by writing\n> \n> Simple bi-linear Bayer interpolation of nearest pixels is implemented.\n> \n> ?\n\nThis will be in v4.\n\n>> The 2 LS bits of the 10-bit colour values are dropped as the RGBA\n>> format we convert into has only 8 bits per colour.\n>>\n>> The texture coordinates passed to the fragment shader are ajusted\n>> to point to the nearest pixel in the image. This prevents artifacts\n>> when the image is scaled from the frame resolution to the window size.\n>>\n>> Signed-off-by: Andrey Konovalov <andrey.konovalov@linaro.org>\n>> ---\n>>   src/qcam/assets/shader/bayer_1x_packed.frag | 174 ++++++++++++++++++++\n>>   src/qcam/assets/shader/shaders.qrc          |   1 +\n>>   src/qcam/viewfinder_gl.cpp                  |  77 ++++++++-\n>>   src/qcam/viewfinder_gl.h                    |   6 +\n>>   4 files changed, 256 insertions(+), 2 deletions(-)\n>>   create mode 100644 src/qcam/assets/shader/bayer_1x_packed.frag\n>>\n>> diff --git a/src/qcam/assets/shader/bayer_1x_packed.frag b/src/qcam/assets/shader/bayer_1x_packed.frag\n>> new file mode 100644\n>> index 00000000..0a87c6db\n>> --- /dev/null\n>> +++ b/src/qcam/assets/shader/bayer_1x_packed.frag\n>> @@ -0,0 +1,174 @@\n>> +/* SPDX-License-Identifier: BSD-2-Clause */\n>> +/*\n>> + * Based on the code from http://jgt.akpeters.com/papers/McGuire08/\n>> + *\n>> + * Efficient, High-Quality Bayer Demosaic Filtering on GPUs\n>> + *\n>> + * Morgan McGuire\n>> + *\n>> + * This paper appears in issue Volume 13, Number 4.\n>> + * ---------------------------------------------------------\n>> + * Copyright (c) 2008, Morgan McGuire. All rights reserved.\n>> + *\n>> + *\n>> + * Modified by Linaro Ltd for 10/12-bit packed vs 8-bit raw Bayer format,\n>> + * and for simpler demosaic algorithm.\n>> + * Copyright (C) 2020, Linaro\n>> + *\n>> + * bayer_1x_packed.frag - Fragment shader code for raw Bayer 10-bit and 12-bit\n>> + * packed formats\n>> + */\n>> +\n>> +#ifdef GL_ES\n>> +precision mediump float;\n>> +#endif\n>> +\n>> +varying vec2 textureOut;\n>> +\n>> +/* the texture size: tex_size.xy is in bytes, tex_size.zw is in pixels */\n>> +uniform vec4 tex_size;\n> \n> tex_size.xy isn't used anywhere as far as I can see. Could it be turned\n> into a vec2 and only store the size in pixels, or do you expect the size\n> in bytes to be needed later ?\n\nYou are right. Will make this change in v4.\n\n>> +uniform vec2 tex_step;\n>> +uniform vec2 tex_bayer_first_red;\n>> +\n>> +uniform sampler2D tex_raw;\n>> +\n>> +void main(void)\n>> +{\n>> +\tvec3 rgb;\n>> +\n>> +\t/*\n>> +\t * center.xy holds the coordinates of the pixel being sampled\n>> +\t * on the [0, 1] range.\n> \n> Looking at the code, this doesn't seem to be correct. I think it holds\n> the value in bytes, while .zw is in pixels.\n\nCorrect. I've fixed the comment for v4.\n\n> Would it be more readable to create two vec2 variables named\n> center_pixel and center_bytes ? Or would this have an impact on\n> performance ?\n\nI'll split vec4 center into two vec2 for v4 of the patchset. It shouldn't\nimpact the performance, just will use two GPU registers instead of one\n(not a big deal here).\n\n>> +\t * center.zw holds the coordinates of the pixel being sampled\n>> +\t * on the [0, width/height-1] range.\n>> +\t */\n>> +\tvec4 center;\n> \n> Could you please add blank lines before every comment block ?\n\nOK.\n\n>> +\t/*\n>> +\t * x-positions of the adjacent pixels on the [0, 1] range.\n>> +\t */\n>> +\tvec2 xcoords;\n>> +\t/*\n>> +\t * y-positions of the adjacent pixels on the [0, 1] range.\n>> +\t */\n>> +\tvec2 ycoords;\n>> +\n>> +\t/*\n>> +\t * The coordinates passed to the shader in textureOut may point\n>> +\t * to a place in between the pixels if the viewfinder window is scaled\n>> +\t * from the original captured frame size. Align them to the nearest\n>> +\t * pixel.\n>> +\t */\n>> +\tcenter.zw = floor(textureOut * tex_size.zw);\n> \n> Unless I'm mistaken, this doesn't align to the nearest pixel, but to the\n> previous one.\n\nTo my understanding, if textureOut points in between the pixels, floor() will\nset center.zw (center_pixel) to the nearest pixel to the left (speaking of the\nX axis).\n\n> We could use round() instead, but that would require GLSL\n> 1.30.\n\nNot sure if using round() has benefits over using floor().\n(Shifting the whole image by half a pixel could hardly be noticed by a naked eye\nanyway)\n\n> Can textureOut really point to between pixels, given that\n> textureMinMagFilters_ is set to GL_NEAREST ?\n\nThe comment in the shader code says that it is possible if the window is\nnot of the same size as the sensor resolution. And re-reading it now, this\ndoesn't seem to be the real reason.\nBut the problem with MIPI packed bayer formats (the 10- and 12-bit formats\nsupported by this and the 4/5 patches) is that we use 8-bit (GL_RED) texture\nto store 10- or 12-bit pixels (as there is no texture format with 10/12-bit\ntexels). And in this case GL_NEAREST wouldn't work correctly. Hence the need\nfor manual tricks.\n\nWhat I am not currently sure of, is if the 8-bit raw Bayer format would\njust work then. This is not a packed format, and it fits GL_RED texture just fine.\nThough I did tried using the original vertex and fragment shaders by Morgan,\nand did see the same defects as the ones fixed by aligning the pixel coordinates\nmanually.\n\nI'll check the 8-bit raw Bayer case again to make sure I didn't messed\nsomething up back in February when v1 of this patchset was created.\n\nAnd if the above explanation for the packed formats sounds OK for you,\nI could move the last patch which adds 8-bit raw Bayer support out of this\npatchset, and post the 8-bit one separately to speed the things up.\n\n>> +\tcenter.y = center.w;\n>> +\t/*\n>> +\t * Add a small number (a few mantissa's LSBs) to avoid float\n>> +\t * representation issues. Maybe paranoic.\n>> +\t */\n>> +\tcenter.x = BPP_X * center.z + 0.02;\n>> +\n>> +\tconst float threshold_l = 0.127 /* fract(BPP_X) * 0.5 + 0.02 */;\n>> +\tconst float threshold_h = 0.625 /* 1.0 - fract(BPP_X) * 1.5 */;\n>> +\n>> +\tfloat fract_x = fract(center.x);\n>> +\t/*\n>> +\t * The below floor() call ensures that center.x points\n>> +\t * at one of the bytes representing the 8 higher bits of\n>> +\t * the pixel value, not at the byte containing the LS bits\n>> +\t * of the group of the pixels.\n>> +\t */\n>> +\tcenter.x = floor(center.x);\n>> +\tcenter.xy *= tex_step;\n>> +\n>> +\txcoords = center.x + vec2(-tex_step.x, tex_step.x);\n>> +\tycoords = center.y + vec2(-tex_step.y, tex_step.y);\n>> +\t/*\n>> +\t * If xcoords[0] points at the byte containing the LS bits\n>> +         * of the previous group of the pixels, move xcoords[0] one\n> \n> Incorrect indentation here (and same below).\n\nAck, will fix in v4.\n\n>> +\t * byte back.\n>> +\t */\n>> +\txcoords[0] += (fract_x < threshold_l) ? -tex_step.x : 0.0;\n>> +\t/*\n>> +\t * If xcoords[1] points at the byte containing the LS bits\n>> +         * of the current group of the pixels, move xcoords[1] one\n>> +\t * byte forward.\n>> +\t */\n>> +\txcoords[1] += (fract_x > threshold_h) ? tex_step.x : 0.0;\n>> +\n>> +\tvec2 alternate = mod(center.zw + tex_bayer_first_red, 2.0);\n>> +\tbool even_col = alternate.x < 1.0;\n>> +\tbool even_raw = alternate.y < 1.0;\n> \n> s/raw/row/\n\nWill be fixed in v4.\n\n>> +\n>> +\t/*\n>> +\t * We need to sample the central pixel and the ones with offset\n>> +\t * of -1 to +1 pixel in both X and Y directions. Let's name these\n>> +\t * pixels as below, where C is the central pixel:\n> \n> I'd add a blank line here and a few below too.\n\nWill be fixed in v4.\n\n>> +\t *   +----+----+----+----+\n>> +\t *   | \\ x|    |    |    |\n>> +\t *   |y \\ | -1 |  0 | +1 |\n>> +\t *   +----+----+----+----+\n>> +\t *   | +1 | D2 | A1 | D3 |\n>> +\t *   +----+----+----+----+\n>> +\t *   |  0 | B0 |  C | B1 |\n>> +\t *   +----+----+----+----+\n>> +\t *   | -1 | D0 | A0 | D1 |\n>> +\t *   +----+----+----+----+\n>> +\t * In the below equations (0,-1).r means \"r component of the texel\n>> +\t * shifted by -tex_step.y from the center.xy one\" etc.\n>> +\t * In the even raw / even column (EE) case the colour values are:\n> \n> s/raw/row/ (same below).\n\nWill be fixed in v4.\n\n>> +\t *   R = C = (0,0).r,\n>> +\t *   G = (A0 + A1 + B0 + B1) / 4.0 =\n>> +\t *       ( (0,-1).r + (0,1).r + (-1,0).r + (1,0).r ) / 4.0,\n>> +\t *   B = (D0 + D1 + D2 + D3) / 4.0 =\n>> +\t *       ( (-1,-1).r + (1,-1).r + (-1,1).r + (1,1).r ) / 4.0\n>> +\t * For even raw / odd column (EO):\n>> +\t *   R = (B0 + B1) / 2.0 = ( (-1,0).r + (1,0).r ) / 2.0,\n>> +\t *   G = C = (0,0).r,\n>> +\t *   B = (A0 + A1) / 2.0 = ( (0,-1).r + (0,1).r ) / 2.0\n>> +\t * For odd raw / even column (OE):\n>> +\t *   R = (A0 + A1) / 2.0 = ( (0,-1).r + (0,1).r ) / 2.0,\n>> +\t *   G = C = (0,0).r,\n>> +\t *   B = (B0 + B1) / 2.0 = ( (-1,0).r + (1,0).r ) / 2.0\n>> +\t * For odd raw / odd column (OO):\n>> +\t *   R = (D0 + D1 + D2 + D3) / 4.0 =\n>> +\t *       ( (-1,-1).r + (1,-1).r + (-1,1).r + (1,1).r ) / 4.0,\n>> +\t *   G = (A0 + A1 + B0 + B1) / 4.0 =\n>> +\t *       ( (0,-1).r + (0,1).r + (-1,0).r + (1,0).r ) / 4.0,\n>> +\t *   B = C = (0,0).r\n>> +\t */\n>> +\n>> +\t/*\n>> +\t * Fetch the values and precalculate the terms:\n>> +\t *   patterns.x = (A0 + A1) / 2.0\n>> +\t *   patterns.y = (B0 + B1) / 2.0\n>> +\t *   patterns.z = (A0 + A1 + B0 + B1) / 4.0\n>> +\t *   patterns.w = (D0 + D1 + D2 + D3) / 4.0\n>> +\t */\n>> +\t#define fetch(x, y) texture2D(tex_raw, vec2(x, y)).r\n>> +\n>> +\tfloat C = texture2D(tex_raw, center.xy).r;\n>> +\tvec4 patterns = vec4(\n>> +\t\tfetch(center.x, ycoords[0]),\t/* A0: (0,-1) */\n>> +\t\tfetch(xcoords[0], center.y),\t/* B0: (-1,0) */\n>> +\t\tfetch(xcoords[0], ycoords[0]),\t/* D0: (-1,-1) */\n>> +\t\tfetch(xcoords[1], ycoords[0]));\t/* D1: (1,-1) */\n>> +\tvec4 temp = vec4(\n>> +\t\tfetch(center.x, ycoords[1]),\t/* A1: (0,1) */\n>> +\t\tfetch(xcoords[1], center.y),\t/* B1: (1,0) */\n>> +\t\tfetch(xcoords[1], ycoords[1]),\t/* D3: (1,1) */\n>> +\t\tfetch(xcoords[0], ycoords[1]));\t/* D2: (-1,1) */\n>> +\tpatterns = (patterns + temp) * 0.5;\n>> +\t\t/* .x = (A0 + A1) / 2.0, .y = (B0 + B1) / 2.0 */\n>> +\t\t/* .z = (D0 + D3) / 2.0, .w = (D1 + D2) / 2.0 */\n>> +\tpatterns.w = (patterns.z + patterns.w) * 0.5;\n>> +\tpatterns.z = (patterns.x + patterns.y) * 0.5;\n>> +\n>> +\trgb = (even_col) ?\n>> +\t\t((even_raw) ?\n>> +\t\t\tvec3(C, patterns.zw) :\n>> +\t\t\tvec3(patterns.x, C, patterns.y)) :\n>> +\t\t((even_raw) ?\n>> +\t\t\tvec3(patterns.y, C, patterns.x) :\n>> +\t\t\tvec3(patterns.wz, C));\n> \n> No need for parentheses around even_col and even_raw.\n\nWill be fixed in v4.\n\n>> +\n>> +\tgl_FragColor = vec4(rgb, 1.0);\n>> +}\n>> diff --git a/src/qcam/assets/shader/shaders.qrc b/src/qcam/assets/shader/shaders.qrc\n>> index 8a8f9de1..d76d65c5 100644\n>> --- a/src/qcam/assets/shader/shaders.qrc\n>> +++ b/src/qcam/assets/shader/shaders.qrc\n>> @@ -5,6 +5,7 @@\n>>   \t<file>YUV_2_planes.frag</file>\n>>   \t<file>YUV_3_planes.frag</file>\n>>   \t<file>YUV_packed.frag</file>\n>> +\t<file>bayer_1x_packed.frag</file>\n>>   \t<file>identity.vert</file>\n>>   </qresource>\n>>   </RCC>\n>> diff --git a/src/qcam/viewfinder_gl.cpp b/src/qcam/viewfinder_gl.cpp\n>> index ff719418..b324d77f 100644\n>> --- a/src/qcam/viewfinder_gl.cpp\n>> +++ b/src/qcam/viewfinder_gl.cpp\n>> @@ -36,6 +36,11 @@ static const QList<libcamera::PixelFormat> supportedFormats{\n>>   \tlibcamera::formats::RGBA8888,\n>>   \tlibcamera::formats::BGR888,\n>>   \tlibcamera::formats::RGB888,\n>> +\t/* Raw Bayer 10-bit packed */\n>> +\tlibcamera::formats::SBGGR10_CSI2P,\n>> +\tlibcamera::formats::SGBRG10_CSI2P,\n>> +\tlibcamera::formats::SGRBG10_CSI2P,\n>> +\tlibcamera::formats::SRGGB10_CSI2P,\n>>   };\n>>   \n>>   ViewFinderGL::ViewFinderGL(QWidget *parent)\n>> @@ -114,6 +119,9 @@ bool ViewFinderGL::selectFormat(const libcamera::PixelFormat &format)\n>>   {\n>>   \tbool ret = true;\n>>   \n>> +\t/* Set min/mag filters to GL_LINEAR by default. */\n>> +\ttextureMinMagFilters_ = GL_LINEAR;\n>> +\n>>   \tfragmentShaderDefines_.clear();\n>>   \n>>   \tswitch (format) {\n>> @@ -203,6 +211,34 @@ bool ViewFinderGL::selectFormat(const libcamera::PixelFormat &format)\n>>   \t\tfragmentShaderDefines_.append(\"#define RGB_PATTERN bgr\");\n>>   \t\tfragmentShaderFile_ = \":RGB.frag\";\n>>   \t\tbreak;\n>> +\tcase libcamera::formats::SBGGR10_CSI2P:\n>> +\t\tfirstRed_.setX(1.0);\n>> +\t\tfirstRed_.setY(1.0);\n>> +\t\tfragmentShaderDefines_.append(\"#define BPP_X 1.25\");\n>> +\t\tfragmentShaderFile_ = \":bayer_1x_packed.frag\";\n>> +\t\ttextureMinMagFilters_ = GL_NEAREST;\n>> +\t\tbreak;\n>> +\tcase libcamera::formats::SGBRG10_CSI2P:\n>> +\t\tfirstRed_.setX(0.0);\n>> +\t\tfirstRed_.setY(1.0);\n>> +\t\tfragmentShaderDefines_.append(\"#define BPP_X 1.25\");\n>> +\t\tfragmentShaderFile_ = \":bayer_1x_packed.frag\";\n>> +\t\ttextureMinMagFilters_ = GL_NEAREST;\n>> +\t\tbreak;\n>> +\tcase libcamera::formats::SGRBG10_CSI2P:\n>> +\t\tfirstRed_.setX(1.0);\n>> +\t\tfirstRed_.setY(0.0);\n>> +\t\tfragmentShaderDefines_.append(\"#define BPP_X 1.25\");\n>> +\t\tfragmentShaderFile_ = \":bayer_1x_packed.frag\";\n>> +\t\ttextureMinMagFilters_ = GL_NEAREST;\n>> +\t\tbreak;\n>> +\tcase libcamera::formats::SRGGB10_CSI2P:\n>> +\t\tfirstRed_.setX(0.0);\n>> +\t\tfirstRed_.setY(0.0);\n>> +\t\tfragmentShaderDefines_.append(\"#define BPP_X 1.25\");\n>> +\t\tfragmentShaderFile_ = \":bayer_1x_packed.frag\";\n>> +\t\ttextureMinMagFilters_ = GL_NEAREST;\n>> +\t\tbreak;\n>>   \tdefault:\n>>   \t\tret = false;\n>>   \t\tqWarning() << \"[ViewFinderGL]:\"\n>> @@ -290,6 +326,8 @@ bool ViewFinderGL::createFragmentShader()\n>>   \ttextureUniformU_ = shaderProgram_.uniformLocation(\"tex_u\");\n>>   \ttextureUniformV_ = shaderProgram_.uniformLocation(\"tex_v\");\n>>   \ttextureUniformStep_ = shaderProgram_.uniformLocation(\"tex_step\");\n>> +\ttextureUniformSize_ = shaderProgram_.uniformLocation(\"tex_size\");\n>> +\ttextureUniformBayerFirstRed_ = shaderProgram_.uniformLocation(\"tex_bayer_first_red\");\n>>   \n>>   \t/* Create the textures. */\n>>   \tfor (std::unique_ptr<QOpenGLTexture> &texture : textures_) {\n>> @@ -306,8 +344,10 @@ bool ViewFinderGL::createFragmentShader()\n>>   void ViewFinderGL::configureTexture(QOpenGLTexture &texture)\n>>   {\n>>   \tglBindTexture(GL_TEXTURE_2D, texture.textureId());\n>> -\tglTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MAG_FILTER, GL_LINEAR);\n>> -\tglTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MIN_FILTER, GL_LINEAR);\n>> +\tglTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MAG_FILTER,\n>> +\t\t\ttextureMinMagFilters_);\n>> +\tglTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MIN_FILTER,\n>> +\t\t\ttextureMinMagFilters_);\n>>   \tglTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_WRAP_S, GL_CLAMP_TO_EDGE);\n>>   \tglTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_WRAP_T, GL_CLAMP_TO_EDGE);\n>>   }\n>> @@ -547,6 +587,39 @@ void ViewFinderGL::doRender()\n>>   \t\tshaderProgram_.setUniformValue(textureUniformY_, 0);\n>>   \t\tbreak;\n>>   \n>> +\tcase libcamera::formats::SBGGR10_CSI2P:\n>> +\tcase libcamera::formats::SGBRG10_CSI2P:\n>> +\tcase libcamera::formats::SGRBG10_CSI2P:\n>> +\tcase libcamera::formats::SRGGB10_CSI2P:\n>> +\t\t/*\n>> +\t\t * Packed raw Bayer 10-bit formats are stored in GL_RED texture.\n>> +\t\t * The texture width is 10/8 of the image width.\n>> +\t\t * TODO: account for padding bytes at the end of the line.\n> \n> s/TODO:/\\todo/\n> \n> But it's fixed in 3/5, which seems to have forgotten to drop this line.\n\nWill be fixed in v4.\n\n>> +\t\t */\n>> +\t\tglActiveTexture(GL_TEXTURE0);\n>> +\t\tconfigureTexture(*textures_[0]);\n>> +\t\tglTexImage2D(GL_TEXTURE_2D,\n>> +\t\t\t     0,\n>> +\t\t\t     GL_RED,\n>> +\t\t\t     size_.width() * 5 / 4,\n>> +\t\t\t     size_.height(),\n>> +\t\t\t     0,\n>> +\t\t\t     GL_RED,\n>> +\t\t\t     GL_UNSIGNED_BYTE,\n>> +\t\t\t     data_);\n>> +\t\tshaderProgram_.setUniformValue(textureUniformY_, 0);\n>> +\t\tshaderProgram_.setUniformValue(textureUniformBayerFirstRed_,\n>> +\t\t\t\t\t       firstRed_);\n>> +\t\tshaderProgram_.setUniformValue(textureUniformSize_,\n>> +\t\t\t\t\t       size_.width() * 5 / 4,\n>> +\t\t\t\t\t       size_.height(),\n>> +\t\t\t\t\t       size_.width(),\n>> +\t\t\t\t\t       size_.height());\n>> +\t\tshaderProgram_.setUniformValue(textureUniformStep_,\n>> +\t\t\t\t\t       1.0f / (size_.width() * 5 / 4 - 1),\n>> +\t\t\t\t\t       1.0f / (size_.height() - 1));\n>> +\t\tbreak;\n>> +\n>>   \tdefault:\n>>   \t\tbreak;\n>>   \t};\n>> diff --git a/src/qcam/viewfinder_gl.h b/src/qcam/viewfinder_gl.h\n>> index 1b1faa91..337718e3 100644\n>> --- a/src/qcam/viewfinder_gl.h\n>> +++ b/src/qcam/viewfinder_gl.h\n>> @@ -81,13 +81,19 @@ private:\n>>   \t/* Textures */\n>>   \tstd::array<std::unique_ptr<QOpenGLTexture>, 3> textures_;\n>>   \n>> +\t/* Common texture parameters */\n>> +\tGLuint textureMinMagFilters_;\n> \n> A blank line would be nice here.\n\nWill be fixed in v4.\n\n>>   \t/* YUV texture parameters */\n>>   \tGLuint textureUniformU_;\n>>   \tGLuint textureUniformV_;\n>>   \tGLuint textureUniformY_;\n>> +\tGLuint textureUniformSize_;\n> \n> This is for Bayer textures, so it should be moved below.\n\nWill be fixed in v4.\n\n>>   \tGLuint textureUniformStep_;\n>>   \tunsigned int horzSubSample_;\n>>   \tunsigned int vertSubSample_;\n> \n> And here too.\n\nWill be fixed in v4.\n\nThanks,\nAndrey\n\n>> +\t/* Raw Bayer texture parameters */\n>> +\tGLuint textureUniformBayerFirstRed_;\n>> +\tQPointF firstRed_;\n>>   \n>>   \tQMutex mutex_; /* Prevent concurrent access to image_ */\n>>   };\n>","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","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 4DACAC3218\n\tfor <parsemail@patchwork.libcamera.org>;\n\tTue, 15 Jun 2021 18:01:05 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 3064C68943;\n\tTue, 15 Jun 2021 20:01:04 +0200 (CEST)","from mail-lj1-x22e.google.com (mail-lj1-x22e.google.com\n\t[IPv6:2a00:1450:4864:20::22e])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id E510A6029D\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue, 15 Jun 2021 20:01:01 +0200 (CEST)","by mail-lj1-x22e.google.com with SMTP id k8so9361211lja.4\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue, 15 Jun 2021 11:01:01 -0700 (PDT)","from [192.168.88.254] ([85.249.44.185])\n\tby smtp.gmail.com with ESMTPSA id\n\tf6sm1178434lfm.28.2021.06.15.11.00.58\n\t(version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128);\n\tTue, 15 Jun 2021 11:00:58 -0700 (PDT)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=linaro.org header.i=@linaro.org\n\theader.b=\"J44FLftu\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google;\n\th=subject:to:cc:references:from:message-id:date:user-agent\n\t:mime-version:in-reply-to:content-language:content-transfer-encoding; \n\tbh=k/wyQQuHNOzyveLM/73eqhD125vK3htwhDEYUHyxPC0=;\n\tb=J44FLftuzNOTxn/IkKlPMSah9BzAJdaBfuWXAluo4oHzuqjTxit2hDrOUQ25PL4PyB\n\t8Kho98vmDZmvOH0bpoxP7x3Ro5aqwMxPgcw4lnMYqwa9Anub5XrEnQPjkJ8R3S5Rfss4\n\tr+K8sEI5D6qyCF1iTDpkBQoS262h6BisXxzV7Y7cDwsOD6U2Kph0oTWddNcRW+HCj/JV\n\tFaMMMUAg5kg0TP3j9+y/tU6TNwa4C4zX1L9C+f+5ealCZoBAQd+5TIrMQzn9MCdqHlbl\n\tIxPARx6I46L4ldDzkCD+rdQS6sr4n/LQk6zQtY2qZrzPIY8fBKy5MnbWyPl/IBV67E66\n\txD5w==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:subject:to:cc:references:from:message-id:date\n\t:user-agent:mime-version:in-reply-to:content-language\n\t:content-transfer-encoding;\n\tbh=k/wyQQuHNOzyveLM/73eqhD125vK3htwhDEYUHyxPC0=;\n\tb=FLyeNAifKSg9tQUSS7/DkRqm31UO1IawTcGGh9UUq9Z4lOViFxw8vcRSxesID8Lnu/\n\tF4jND9Xa5UiX7ayOl9NsKN4W3xMLjcDQrKwvZEEZCzyqcQIDodDxqQCnaR/tk8AXcMxa\n\tLDb159KN/NswKJ9NCMLL0jS+7DaYO5BNkROKF7/07B52xDEpagfvVJ2neE6+BobLVezU\n\t4sXrkmn1XDHhAfVc7giyfkhzpD19BjWylorzSs3fhICCa3j+8X6p2NK7rEER8Ih5Qphq\n\tQMqWpvP9JNOiZtdoXo/bBa1d2Dvk1vGGEOBF2DlCVbgcWfbPlF5I0lomON/ovAMFby3M\n\t04EA==","X-Gm-Message-State":"AOAM530PCa6JnjbIks9aBQYLAjJlbwLnIxH/iw0tbsCL3vzN0WRg8ZZx\n\txs5I25zPYyygcYZSA0fhJV5AbA==","X-Google-Smtp-Source":"ABdhPJy8xXRHDNWgIzU0XFDn+tLlMBZo3/U4R/L+g8fpP8jecnZwOsUbLj6l1xzmLDqixFRvB8R95A==","X-Received":"by 2002:a2e:a0c2:: with SMTP id f2mr767614ljm.36.1623780059586; \n\tTue, 15 Jun 2021 11:00:59 -0700 (PDT)","To":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","References":"<20210527105511.447089-1-andrey.konovalov@linaro.org>\n\t<20210527105511.447089-3-andrey.konovalov@linaro.org>\n\t<YME48ZD4sXLVMa15@pendragon.ideasonboard.com>","From":"Andrey Konovalov <andrey.konovalov@linaro.org>","Message-ID":"<c97567b0-9f6f-6b75-da59-722dd47d211a@linaro.org>","Date":"Tue, 15 Jun 2021 21:00:57 +0300","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101\n\tThunderbird/78.8.1","MIME-Version":"1.0","In-Reply-To":"<YME48ZD4sXLVMa15@pendragon.ideasonboard.com>","Content-Type":"text/plain; charset=utf-8; format=flowed","Content-Language":"en-US","Content-Transfer-Encoding":"7bit","Subject":"Re: [libcamera-devel] [PATCH v2 2/5] qcam: viewfinder_gl: Add\n\tshader to render packed RAW10 formats","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Cc":"morgan@casual-effects.com, libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}}]