[{"id":38521,"web_url":"https://patchwork.libcamera.org/comment/38521/","msgid":"<20260407235014.GL1268443@killaraus.ideasonboard.com>","date":"2026-04-07T23:50:14","subject":"Re: [PATCH 01/13] libcamera: shaders: unpacked: Fix indentations","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"On Tue, Apr 07, 2026 at 11:01:04PM +0100, Kieran Bingham wrote:\n> Reflow the unpacked bayer shader to use tabs for indentation and\n> match the style of the bayer_packed fragment shader.\n\nDoes anyone know of a formatter for GLSL ? clang-format does a\nrelatively decent job, it could make sense to start with that.\n\n> Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>\n> ---\n>  src/libcamera/shaders/bayer_unpacked.frag | 264 +++++++++++++++---------------\n>  1 file changed, 132 insertions(+), 132 deletions(-)\n> \n> diff --git a/src/libcamera/shaders/bayer_unpacked.frag b/src/libcamera/shaders/bayer_unpacked.frag\n> index 1b85196ae16130670eb3d1c077ab4884119ae63c..76ffc47a8a29f242c1fba88f32bd8db731edeee0 100644\n> --- a/src/libcamera/shaders/bayer_unpacked.frag\n> +++ b/src/libcamera/shaders/bayer_unpacked.frag\n> @@ -31,163 +31,163 @@ uniform float           contrastExp;\n>  \n>  float apply_contrast(float value)\n>  {\n> -    // Apply simple S-curve\n> -    if (value < 0.5)\n> -        return 0.5 * pow(value / 0.5, contrastExp);\n> -    else\n> -        return 1.0 - 0.5 * pow((1.0 - value) / 0.5, contrastExp);\n> +\t// Apply simple S-curve\n\nIt could make sense to standardize on one style for comments.\n\n> +\tif (value < 0.5)\n> +\t\treturn 0.5 * pow(value / 0.5, contrastExp);\n> +\telse\n> +\t\treturn 1.0 - 0.5 * pow((1.0 - value) / 0.5, contrastExp);\n>  }\n>  \n>  void main(void) {\n\nStyle mismatch in curly brace placement between apply_contrast() and\nmain(). I'd write\n\nvoid main(void)\n{\n\n> -    vec3 rgb;\n> +\tvec3 rgb;\n>  \n> -    #if defined(RAW10P)\n> -    #define pixel(p) p.r / 4.0 + p.g * 64.0\n> -    #define fetch(x, y) pixel(texture2D(tex_y, vec2(x, y)))\n> -    #elif defined(RAW12P)\n> -    #define pixel(p) p.r / 16.0 + p.g * 16.0\n> -    #define fetch(x, y) pixel(texture2D(tex_y, vec2(x, y)))\n> -    #else\n> -    #define fetch(x, y) texture2D(tex_y, vec2(x, y)).r\n> -    #endif\n> +\t#if defined(RAW10P)\n> +\t#define pixel(p) p.r / 4.0 + p.g * 64.0\n> +\t#define fetch(x, y) pixel(texture2D(tex_y, vec2(x, y)))\n> +\t#elif defined(RAW12P)\n> +\t#define pixel(p) p.r / 16.0 + p.g * 16.0\n> +\t#define fetch(x, y) pixel(texture2D(tex_y, vec2(x, y)))\n> +\t#else\n> +\t#define fetch(x, y) texture2D(tex_y, vec2(x, y)).r\n> +\t#endif\n\nI'd have removed the indentation for macros, as some macros are already\naligned on the left-most column.\n\nFeel free to pick which comments you deem worth addressing.\n\nReviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n\n>  \n> -    float C = fetch(center.x, center.y); // ( 0, 0)\n> -    const vec4 kC = vec4( 4.0,  6.0,  5.0,  5.0) / 8.0;\n> +\tfloat C = fetch(center.x, center.y); // ( 0, 0)\n> +\tconst vec4 kC = vec4( 4.0,  6.0,  5.0,  5.0) / 8.0;\n>  \n> -    // Determine which of four types of pixels we are on.\n> -    vec2 alternate = mod(floor(center.zw), 2.0);\n> +\t// Determine which of four types of pixels we are on.\n> +\tvec2 alternate = mod(floor(center.zw), 2.0);\n>  \n> -    vec4 Dvec = vec4(\n> -        fetch(xCoord[1], yCoord[1]),  // (-1,-1)\n> -        fetch(xCoord[1], yCoord[2]),  // (-1, 1)\n> -        fetch(xCoord[2], yCoord[1]),  // ( 1,-1)\n> -        fetch(xCoord[2], yCoord[2])); // ( 1, 1)\n> +\tvec4 Dvec = vec4(\n> +\t\tfetch(xCoord[1], yCoord[1]),  // (-1,-1)\n> +\t\tfetch(xCoord[1], yCoord[2]),  // (-1, 1)\n> +\t\tfetch(xCoord[2], yCoord[1]),  // ( 1,-1)\n> +\t\tfetch(xCoord[2], yCoord[2])); // ( 1, 1)\n>  \n> -    vec4 PATTERN = (kC.xyz * C).xyzz;\n> +\tvec4 PATTERN = (kC.xyz * C).xyzz;\n>  \n> -    // Can also be a dot product with (1,1,1,1) on hardware where that is\n> -    // specially optimized.\n> -    // Equivalent to: D = Dvec[0] + Dvec[1] + Dvec[2] + Dvec[3];\n> -    Dvec.xy += Dvec.zw;\n> -    Dvec.x  += Dvec.y;\n> +\t// Can also be a dot product with (1,1,1,1) on hardware where that is\n> +\t// specially optimized.\n> +\t// Equivalent to: D = Dvec[0] + Dvec[1] + Dvec[2] + Dvec[3];\n> +\tDvec.xy += Dvec.zw;\n> +\tDvec.x  += Dvec.y;\n>  \n> -    vec4 value = vec4(\n> -        fetch(center.x, yCoord[0]),   // ( 0,-2)\n> -        fetch(center.x, yCoord[1]),   // ( 0,-1)\n> -        fetch(xCoord[0], center.y),   // (-2, 0)\n> -        fetch(xCoord[1], center.y));  // (-1, 0)\n> +\tvec4 value = vec4(\n> +\t\tfetch(center.x, yCoord[0]),   // ( 0,-2)\n> +\t\tfetch(center.x, yCoord[1]),   // ( 0,-1)\n> +\t\tfetch(xCoord[0], center.y),   // (-2, 0)\n> +\t\tfetch(xCoord[1], center.y));  // (-1, 0)\n>  \n> -    vec4 temp = vec4(\n> -        fetch(center.x, yCoord[3]),   // ( 0, 2)\n> -        fetch(center.x, yCoord[2]),   // ( 0, 1)\n> -        fetch(xCoord[3], center.y),   // ( 2, 0)\n> -        fetch(xCoord[2], center.y));  // ( 1, 0)\n> +\tvec4 temp = vec4(\n> +\t\tfetch(center.x, yCoord[3]),   // ( 0, 2)\n> +\t\tfetch(center.x, yCoord[2]),   // ( 0, 1)\n> +\t\tfetch(xCoord[3], center.y),   // ( 2, 0)\n> +\t\tfetch(xCoord[2], center.y));  // ( 1, 0)\n>  \n> -    // Even the simplest compilers should be able to constant-fold these to\n> -    // avoid the division.\n> -    // Note that on scalar processors these constants force computation of some\n> -    // identical products twice.\n> -    const vec4 kA = vec4(-1.0, -1.5,  0.5, -1.0) / 8.0;\n> -    const vec4 kB = vec4( 2.0,  0.0,  0.0,  4.0) / 8.0;\n> -    const vec4 kD = vec4( 0.0,  2.0, -1.0, -1.0) / 8.0;\n> +\t// Even the simplest compilers should be able to constant-fold these to\n> +\t// avoid the division.\n> +\t// Note that on scalar processors these constants force computation of some\n> +\t// identical products twice.\n> +\tconst vec4 kA = vec4(-1.0, -1.5,  0.5, -1.0) / 8.0;\n> +\tconst vec4 kB = vec4( 2.0,  0.0,  0.0,  4.0) / 8.0;\n> +\tconst vec4 kD = vec4( 0.0,  2.0, -1.0, -1.0) / 8.0;\n>  \n> -    // Conserve constant registers and take advantage of free swizzle on load\n> -    #define kE (kA.xywz)\n> -    #define kF (kB.xywz)\n> +\t// Conserve constant registers and take advantage of free swizzle on load\n> +\t#define kE (kA.xywz)\n> +\t#define kF (kB.xywz)\n>  \n> -    value += temp;\n> +\tvalue += temp;\n>  \n> -    // There are five filter patterns (identity, cross, checker,\n> -    // theta, phi).  Precompute the terms from all of them and then\n> -    // use swizzles to assign to color channels.\n> -    //\n> -    // Channel   Matches\n> -    //   x       cross   (e.g., EE G)\n> -    //   y       checker (e.g., EE B)\n> -    //   z       theta   (e.g., EO R)\n> -    //   w       phi     (e.g., EO R)\n> -    #define A (value[0])\n> -    #define B (value[1])\n> -    #define D (Dvec.x)\n> -    #define E (value[2])\n> -    #define F (value[3])\n> +\t// There are five filter patterns (identity, cross, checker,\n> +\t// theta, phi).  Precompute the terms from all of them and then\n> +\t// use swizzles to assign to color channels.\n> +\t//\n> +\t// Channel   Matches\n> +\t//   x       cross   (e.g., EE G)\n> +\t//   y       checker (e.g., EE B)\n> +\t//   z       theta   (e.g., EO R)\n> +\t//   w       phi     (e.g., EO R)\n> +\t#define A (value[0])\n> +\t#define B (value[1])\n> +\t#define D (Dvec.x)\n> +\t#define E (value[2])\n> +\t#define F (value[3])\n>  \n> -    // Avoid zero elements. On a scalar processor this saves two MADDs\n> -    // and it has no effect on a vector processor.\n> -    PATTERN.yzw += (kD.yz * D).xyy;\n> +\t// Avoid zero elements. On a scalar processor this saves two MADDs\n> +\t// and it has no effect on a vector processor.\n> +\tPATTERN.yzw += (kD.yz * D).xyy;\n>  \n> -    PATTERN += (kA.xyz * A).xyzx + (kE.xyw * E).xyxz;\n> -    PATTERN.xw  += kB.xw * B;\n> -    PATTERN.xz  += kF.xz * F;\n> +\tPATTERN += (kA.xyz * A).xyzx + (kE.xyw * E).xyxz;\n> +\tPATTERN.xw  += kB.xw * B;\n> +\tPATTERN.xz  += kF.xz * F;\n>  \n> -    rgb =  (alternate.y == 0.0) ?\n> -        ((alternate.x == 0.0) ?\n> -            vec3(C, PATTERN.xy) :\n> -            vec3(PATTERN.z, C, PATTERN.w)) :\n> -        ((alternate.x == 0.0) ?\n> -            vec3(PATTERN.w, C, PATTERN.z) :\n> -            vec3(PATTERN.yx, C));\n> +\trgb =  (alternate.y == 0.0) ?\n> +\t\t((alternate.x == 0.0) ?\n> +\t\t\tvec3(C, PATTERN.xy) :\n> +\t\t\tvec3(PATTERN.z, C, PATTERN.w)) :\n> +\t\t((alternate.x == 0.0) ?\n> +\t\t\tvec3(PATTERN.w, C, PATTERN.z) :\n> +\t\t\tvec3(PATTERN.yx, C));\n>  \n> -    rgb = rgb - blacklevel;\n> +\trgb = rgb - blacklevel;\n>  \n> -    /*\n> -     *   CCM is a 3x3 in the format\n> -     *\n> -     *   +--------------+----------------+---------------+\n> -     *   | RedRedGain   | RedGreenGain   | RedBlueGain   |\n> -     *   +--------------+----------------+---------------+\n> -     *   | GreenRedGain | GreenGreenGain | GreenBlueGain |\n> -     *   +--------------+----------------+---------------+\n> -     *   | BlueRedGain  |  BlueGreenGain | BlueBlueGain  |\n> -     *   +--------------+----------------+---------------+\n> -     *\n> -     *   Rout = RedRedGain * Rin + RedGreenGain * Gin + RedBlueGain * Bin\n> -     *   Gout = GreenRedGain * Rin + GreenGreenGain * Gin + GreenBlueGain * Bin\n> -     *   Bout = BlueRedGain * Rin + BlueGreenGain * Gin + BlueBlueGain * Bin\n> -     *\n> -     *   We upload to the GPU without transposition glUniformMatrix3f(.., .., GL_FALSE, ccm);\n> -     *\n> -     *   CPU\n> -     *   float ccm [] = {\n> -     *             RedRedGain,   RedGreenGain,   RedBlueGain,\n> -     *             GreenRedGain, GreenGreenGain, GreenBlueGain,\n> -     *             BlueRedGain,  BlueGreenGain,  BlueBlueGain,\n> -     *   };\n> -     *\n> -     *   GPU\n> -     *   ccm = {\n> -     *             RedRedGain,   GreenRedGain,   BlueRedGain,\n> -     *             RedGreenGain, GreenGreenGain, BlueGreenGain,\n> -     *             RedBlueGain,  GreenBlueGain,  BlueBlueGain,\n> -     *   }\n> -     *\n> -     *   However the indexing for the mat data-type is column major hence\n> -     *   ccm[0][0] = RedRedGain, ccm[0][1] = RedGreenGain, ccm[0][2] = RedBlueGain\n> -     *\n> -     */\n> -    float rin, gin, bin;\n> -    rin = rgb.r;\n> -    gin = rgb.g;\n> -    bin = rgb.b;\n> +\t/*\n> +\t *   CCM is a 3x3 in the format\n> +\t *\n> +\t *   +--------------+----------------+---------------+\n> +\t *   | RedRedGain   | RedGreenGain   | RedBlueGain   |\n> +\t *   +--------------+----------------+---------------+\n> +\t *   | GreenRedGain | GreenGreenGain | GreenBlueGain |\n> +\t *   +--------------+----------------+---------------+\n> +\t *   | BlueRedGain  |  BlueGreenGain | BlueBlueGain  |\n> +\t *   +--------------+----------------+---------------+\n> +\t *\n> +\t *   Rout = RedRedGain * Rin + RedGreenGain * Gin + RedBlueGain * Bin\n> +\t *   Gout = GreenRedGain * Rin + GreenGreenGain * Gin + GreenBlueGain * Bin\n> +\t *   Bout = BlueRedGain * Rin + BlueGreenGain * Gin + BlueBlueGain * Bin\n> +\t *\n> +\t *   We upload to the GPU without transposition glUniformMatrix3f(.., .., GL_FALSE, ccm);\n> +\t *\n> +\t *   CPU\n> +\t *   float ccm [] = {\n> +\t *             RedRedGain,   RedGreenGain,   RedBlueGain,\n> +\t *             GreenRedGain, GreenGreenGain, GreenBlueGain,\n> +\t *             BlueRedGain,  BlueGreenGain,  BlueBlueGain,\n> +\t *   };\n> +\t *\n> +\t *   GPU\n> +\t *   ccm = {\n> +\t *             RedRedGain,   GreenRedGain,   BlueRedGain,\n> +\t *             RedGreenGain, GreenGreenGain, BlueGreenGain,\n> +\t *             RedBlueGain,  GreenBlueGain,  BlueBlueGain,\n> +\t *   }\n> +\t *\n> +\t *   However the indexing for the mat data-type is column major hence\n> +\t *   ccm[0][0] = RedRedGain, ccm[0][1] = RedGreenGain, ccm[0][2] = RedBlueGain\n> +\t *\n> +\t */\n> +\tfloat rin, gin, bin;\n> +\trin = rgb.r;\n> +\tgin = rgb.g;\n> +\tbin = rgb.b;\n>  \n> -    rgb.r = (rin * ccm[0][0]) + (gin * ccm[0][1]) + (bin * ccm[0][2]);\n> -    rgb.g = (rin * ccm[1][0]) + (gin * ccm[1][1]) + (bin * ccm[1][2]);\n> -    rgb.b = (rin * ccm[2][0]) + (gin * ccm[2][1]) + (bin * ccm[2][2]);\n> +\trgb.r = (rin * ccm[0][0]) + (gin * ccm[0][1]) + (bin * ccm[0][2]);\n> +\trgb.g = (rin * ccm[1][0]) + (gin * ccm[1][1]) + (bin * ccm[1][2]);\n> +\trgb.b = (rin * ccm[2][0]) + (gin * ccm[2][1]) + (bin * ccm[2][2]);\n>  \n> -    /*\n> -     * Contrast\n> -     */\n> -    rgb = clamp(rgb, 0.0, 1.0);\n> -    rgb.r = apply_contrast(rgb.r);\n> -    rgb.g = apply_contrast(rgb.g);\n> -    rgb.b = apply_contrast(rgb.b);\n> +\t/*\n> +\t * Contrast\n> +\t */\n> +\trgb = clamp(rgb, 0.0, 1.0);\n> +\trgb.r = apply_contrast(rgb.r);\n> +\trgb.g = apply_contrast(rgb.g);\n> +\trgb.b = apply_contrast(rgb.b);\n>  \n> -    /* Apply gamma after colour correction */\n> -    rgb = pow(rgb, vec3(gamma));\n> +\t/* Apply gamma after colour correction */\n> +\trgb = pow(rgb, vec3(gamma));\n>  \n>  #if defined (SWAP_BLUE)\n> -    gl_FragColor = vec4(rgb.bgr, 1.0);\n> +\tgl_FragColor = vec4(rgb.bgr, 1.0);\n>  #else\n> -    gl_FragColor = vec4(rgb, 1.0);\n> +\tgl_FragColor = vec4(rgb, 1.0);\n>  #endif\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 47BFBBEFBE\n\tfor <parsemail@patchwork.libcamera.org>;\n\tTue,  7 Apr 2026 23:50:19 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 41F9E62DB3;\n\tWed,  8 Apr 2026 01:50:18 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 6106562CEB\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tWed,  8 Apr 2026 01:50:16 +0200 (CEST)","from killaraus.ideasonboard.com\n\t(2001-14ba-703d-e500--2a1.rev.dnainternet.fi\n\t[IPv6:2001:14ba:703d:e500::2a1])\n\tby perceval.ideasonboard.com (Postfix) with UTF8SMTPSA id 6A54F1121; \n\tWed,  8 Apr 2026 01:48:48 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"ShY/xsLy\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1775605728;\n\tbh=GjmVAFWmqmBvK6wbRhzBZBs6zKpKIUj7zOGT8RcWKtg=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=ShY/xsLyINJX0BN0by6YYCZq/56VRsz0Urqywc51jigFapmeBJsDC9ZkU86sZzywl\n\t1ZhbVUIP9r4bn/M0H21g0gl03luRq3KgYGRkqTDGvQ0rtKANe1wxpR4DWC3tAkvUJv\n\tbYlOD1SurWA7Wpmfpz3AJ5jHRO69E1/l8V2hTYZo=","Date":"Wed, 8 Apr 2026 02:50:14 +0300","From":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","To":"Kieran Bingham <kieran.bingham@ideasonboard.com>","Cc":"libcamera-devel@lists.libcamera.org","Subject":"Re: [PATCH 01/13] libcamera: shaders: unpacked: Fix indentations","Message-ID":"<20260407235014.GL1268443@killaraus.ideasonboard.com>","References":"<20260407-kbingham-awb-split-v1-0-a39af3f4dc20@ideasonboard.com>\n\t<20260407-kbingham-awb-split-v1-1-a39af3f4dc20@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20260407-kbingham-awb-split-v1-1-a39af3f4dc20@ideasonboard.com>","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>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}}]