[{"id":13585,"web_url":"https://patchwork.libcamera.org/comment/13585/","msgid":"<b852dd4b-0495-de92-1d27-4b2613fe285c@linaro.org>","date":"2020-11-03T21:24:47","subject":"Re: [libcamera-devel] [PATCH 2/7] qcam: viewfinder_gl: Keep\n\tfragment shader when format doesn't change","submitter":{"id":25,"url":"https://patchwork.libcamera.org/api/people/25/","name":"Andrey Konovalov","email":"andrey.konovalov@linaro.org"},"content":"Hi Laurent,\n\nThanks for your patch!\n\nOn 03.11.2020 18:50, Laurent Pinchart wrote:\n> When ViewFinderGL::setFormat() is called, the fragment shader is deleted\n> and recreated for the new format. This results in unnecessary shader\n> recompilation if only the size is changed and the pixel format remains\n> the same. Keep the existing shader in that case.\n> \n> The null test for fragmentShader_ can be removed, as if the shader\n> program is linked, the fragment shader is guaranteed to exist.\n> \n> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n\nReviewed-by: Andrey Konovalov <andrey.konovalov@linaro.org>\n\nThanks,\nAndrey\n\n> ---\n>   src/qcam/viewfinder_gl.cpp | 16 ++++++++++------\n>   1 file changed, 10 insertions(+), 6 deletions(-)\n> \n> diff --git a/src/qcam/viewfinder_gl.cpp b/src/qcam/viewfinder_gl.cpp\n> index 03a576ba89d2..c07292523504 100644\n> --- a/src/qcam/viewfinder_gl.cpp\n> +++ b/src/qcam/viewfinder_gl.cpp\n> @@ -53,19 +53,23 @@ const QList<libcamera::PixelFormat> &ViewFinderGL::nativeFormats() const\n>   int ViewFinderGL::setFormat(const libcamera::PixelFormat &format,\n>   \t\t\t    const QSize &size)\n>   {\n> -\t/* If the fragment is created remove it and create a new one. */\n> -\tif (fragmentShader_) {\n> +\tif (format != format_) {\n> +\t\t/*\n> +\t\t * If the fragment already exists, remove it and create a new\n> +\t\t * one for the new format.\n> +\t\t */\n>   \t\tif (shaderProgram_.isLinked()) {\n>   \t\t\tshaderProgram_.release();\n>   \t\t\tshaderProgram_.removeShader(fragmentShader_.get());\n>   \t\t\tfragmentShader_.reset();\n>   \t\t}\n> +\n> +\t\tif (!selectFormat(format))\n> +\t\t\treturn -1;\n> +\n> +\t\tformat_ = format;\n>   \t}\n>   \n> -\tif (!selectFormat(format))\n> -\t\treturn -1;\n> -\n> -\tformat_ = format;\n>   \tsize_ = size;\n>   \n>   \tupdateGeometry();\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 30E31BDB1E\n\tfor <parsemail@patchwork.libcamera.org>;\n\tTue,  3 Nov 2020 21:24:55 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id B1A9962BC9;\n\tTue,  3 Nov 2020 22:24:54 +0100 (CET)","from mail-lj1-x243.google.com (mail-lj1-x243.google.com\n\t[IPv6:2a00:1450:4864:20::243])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id DF1CC62B7E\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue,  3 Nov 2020 22:24:52 +0100 (CET)","by mail-lj1-x243.google.com with SMTP id x6so20663158ljd.3\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue, 03 Nov 2020 13:24:52 -0800 (PST)","from [192.168.118.216] ([85.249.41.73])\n\tby smtp.gmail.com with ESMTPSA id\n\tm2sm4141400lfo.25.2020.11.03.13.24.50\n\t(version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128);\n\tTue, 03 Nov 2020 13:24:51 -0800 (PST)"],"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=\"nRvr+9fk\"; 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=83kbZ+ub7P0AxxNGE7yv2Ye0SLkTfCZxi5mcdLLMYOI=;\n\tb=nRvr+9fknpSVHSlJxTd6qxWdUud04V7h1GSWQrW3JUhQAHTuVzwyIi8PUpED6AmFc+\n\thvfyRxjNMzjOcFI/bVDfALYLeloWmwpaEmhACGAq7+wWVBAX+sqjWhOJ7aIq0s66tFZ2\n\tQ95XhgrT4qMjV7dMoWCJ3S4wWFIR1ckJErZpFqjEo4cZlmARZzdWNTUelzE7x/8fO1r5\n\tTnwlU8tlXRvHZ+UFTswjsyn2oprp1grqff4qvHtsNkJc2P24BMG9FUjtcJn96pEdEKzX\n\tAVmhynfbqj4Q/1+09CYc1pJdB9FgGCOj2ftm3leFI5nuOq+6nY7c1rqD0gaQ7ylXabwE\n\tTNOg==","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=83kbZ+ub7P0AxxNGE7yv2Ye0SLkTfCZxi5mcdLLMYOI=;\n\tb=kAv8OzR8laA2ZNfsOdh9bCTK62d3OuTXbM4BR33UoUZFplA/qGS6TqAuvENpMvrRkP\n\tDWds90/ZxSD5cenOvWkduMd540jS3igzXJqRvTMRiLz7n/FoOroo6LgAQe6+d92LSLEP\n\tErr3bBWDNZ8qImBdVJb2dUwqDXwiEJEeBdmK9VnTo+nQP0nTVAAPfWWdS9nu27Ctxx2X\n\tfnfhoUNOhfdXHerSXQQXjRm1C6SSCEwCdpMJyKtZx8S67r/OvGpurWxQjXxpVnnFNU25\n\tH63H1z0GP2/0BOTbjd01mCUU6JevO2qOVFqiuHsWU2ZL4Nk6hxGzjJFoAN89H2gPBWNP\n\t5qbA==","X-Gm-Message-State":"AOAM533d9x0cR2aE5LUzJP/OPluHrvvIyQ7/0TbuTcSVSRAHGa4S6csC\n\tjsgljpeU4h9ubZsm3Rywm7y4DQ==","X-Google-Smtp-Source":"ABdhPJwDas18cBA/caxqOWz96ATKHSeKeP4hENVQ0Wzw7qmBFSyytqxC8r93Wp7lxLNIHHMUiJH3TQ==","X-Received":"by 2002:a2e:320a:: with SMTP id\n\ty10mr6599789ljy.393.1604438692117; \n\tTue, 03 Nov 2020 13:24:52 -0800 (PST)","To":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>,\n\tlibcamera-devel@lists.libcamera.org","References":"<20201103155025.5948-1-laurent.pinchart@ideasonboard.com>\n\t<20201103155025.5948-3-laurent.pinchart@ideasonboard.com>","From":"Andrey Konovalov <andrey.konovalov@linaro.org>","Message-ID":"<b852dd4b-0495-de92-1d27-4b2613fe285c@linaro.org>","Date":"Wed, 4 Nov 2020 00:24:47 +0300","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101\n\tThunderbird/68.10.0","MIME-Version":"1.0","In-Reply-To":"<20201103155025.5948-3-laurent.pinchart@ideasonboard.com>","Content-Language":"en-US","Subject":"Re: [libcamera-devel] [PATCH 2/7] qcam: viewfinder_gl: Keep\n\tfragment shader when format doesn't change","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>","Content-Transfer-Encoding":"7bit","Content-Type":"text/plain; charset=\"us-ascii\"; Format=\"flowed\"","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":13586,"web_url":"https://patchwork.libcamera.org/comment/13586/","msgid":"<7253c17f-ccae-7e3d-f527-50a9764eb5c7@ideasonboard.com>","date":"2020-11-03T22:47:23","subject":"Re: [libcamera-devel] [PATCH 2/7] qcam: viewfinder_gl: Keep\n\tfragment shader when format doesn't change","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Hi Laurent,\n\nOn 03/11/2020 15:50, Laurent Pinchart wrote:\n> When ViewFinderGL::setFormat() is called, the fragment shader is deleted\n> and recreated for the new format. This results in unnecessary shader\n> recompilation if only the size is changed and the pixel format remains\n> the same. Keep the existing shader in that case.\n> \n> The null test for fragmentShader_ can be removed, as if the shader\n> program is linked, the fragment shader is guaranteed to exist.\n> \n> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n\nReviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>\n\n> ---\n>  src/qcam/viewfinder_gl.cpp | 16 ++++++++++------\n>  1 file changed, 10 insertions(+), 6 deletions(-)\n> \n> diff --git a/src/qcam/viewfinder_gl.cpp b/src/qcam/viewfinder_gl.cpp\n> index 03a576ba89d2..c07292523504 100644\n> --- a/src/qcam/viewfinder_gl.cpp\n> +++ b/src/qcam/viewfinder_gl.cpp\n> @@ -53,19 +53,23 @@ const QList<libcamera::PixelFormat> &ViewFinderGL::nativeFormats() const\n>  int ViewFinderGL::setFormat(const libcamera::PixelFormat &format,\n>  \t\t\t    const QSize &size)\n>  {\n> -\t/* If the fragment is created remove it and create a new one. */\n> -\tif (fragmentShader_) {\n> +\tif (format != format_) {\n> +\t\t/*\n> +\t\t * If the fragment already exists, remove it and create a new\n> +\t\t * one for the new format.\n> +\t\t */\n>  \t\tif (shaderProgram_.isLinked()) {\n>  \t\t\tshaderProgram_.release();\n>  \t\t\tshaderProgram_.removeShader(fragmentShader_.get());\n>  \t\t\tfragmentShader_.reset();\n>  \t\t}\n> +\n> +\t\tif (!selectFormat(format))\n> +\t\t\treturn -1;\n> +\n> +\t\tformat_ = format;\n>  \t}\n>  \n> -\tif (!selectFormat(format))\n> -\t\treturn -1;\n> -\n> -\tformat_ = format;\n>  \tsize_ = size;\n>  \n>  \tupdateGeometry();\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 989B0BDB89\n\tfor <parsemail@patchwork.libcamera.org>;\n\tTue,  3 Nov 2020 22:47:28 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 209CE62BC9;\n\tTue,  3 Nov 2020 23:47:28 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 227C962067\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue,  3 Nov 2020 23:47:27 +0100 (CET)","from [192.168.0.20]\n\t(cpc89244-aztw30-2-0-cust3082.18-1.cable.virginm.net [86.31.172.11])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 83313124D;\n\tTue,  3 Nov 2020 23:47:26 +0100 (CET)"],"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=\"wcC4Z8Mx\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1604443646;\n\tbh=w24XOowUhPNmnhvFvBuIzyjjLo69Am7UL77Z1GtT0E4=;\n\th=Reply-To:Subject:To:References:From:Date:In-Reply-To:From;\n\tb=wcC4Z8MxC4rBP5848YiKuAKOCL+6PGJudUblzv1gwZVO4zl4NWF2r3jrJ86YCH7UC\n\tSX9rQGWvxIyXbn/O1fSxjQW8wbH5q77yO3YeJsEv1XFtD831Ziv0IOjy1J8HU/RUjP\n\tmXNR34xcQzaT2Y/wYncCkhP8rmXrNGAUjH3N2ssE=","To":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>,\n\tlibcamera-devel@lists.libcamera.org","References":"<20201103155025.5948-1-laurent.pinchart@ideasonboard.com>\n\t<20201103155025.5948-3-laurent.pinchart@ideasonboard.com>","From":"Kieran Bingham <kieran.bingham@ideasonboard.com>","Autocrypt":"addr=kieran.bingham@ideasonboard.com; keydata=\n\tmQINBFYE/WYBEACs1PwjMD9rgCu1hlIiUA1AXR4rv2v+BCLUq//vrX5S5bjzxKAryRf0uHat\n\tV/zwz6hiDrZuHUACDB7X8OaQcwhLaVlq6byfoBr25+hbZG7G3+5EUl9cQ7dQEdvNj6V6y/SC\n\trRanWfelwQThCHckbobWiQJfK9n7rYNcPMq9B8e9F020LFH7Kj6YmO95ewJGgLm+idg1Kb3C\n\tpotzWkXc1xmPzcQ1fvQMOfMwdS+4SNw4rY9f07Xb2K99rjMwZVDgESKIzhsDB5GY465sCsiQ\n\tcSAZRxqE49RTBq2+EQsbrQpIc8XiffAB8qexh5/QPzCmR4kJgCGeHIXBtgRj+nIkCJPZvZtf\n\tKr2EAbc6tgg6DkAEHJb+1okosV09+0+TXywYvtEop/WUOWQ+zo+Y/OBd+8Ptgt1pDRyOBzL8\n\tRXa8ZqRf0Mwg75D+dKntZeJHzPRJyrlfQokngAAs4PaFt6UfS+ypMAF37T6CeDArQC41V3ko\n\tlPn1yMsVD0p+6i3DPvA/GPIksDC4owjnzVX9kM8Zc5Cx+XoAN0w5Eqo4t6qEVbuettxx55gq\n\t8K8FieAjgjMSxngo/HST8TpFeqI5nVeq0/lqtBRQKumuIqDg+Bkr4L1V/PSB6XgQcOdhtd36\n\tOe9X9dXB8YSNt7VjOcO7BTmFn/Z8r92mSAfHXpb07YJWJosQOQARAQABtDBLaWVyYW4gQmlu\n\tZ2hhbSA8a2llcmFuLmJpbmdoYW1AaWRlYXNvbmJvYXJkLmNvbT6JAlcEEwEKAEECGwMFCwkI\n\tBwIGFQgJCgsCBBYCAwECHgECF4ACGQEWIQSQLdeYP70o/eNy1HqhHkZyEKRh/QUCXWTtygUJ\n\tCyJXZAAKCRChHkZyEKRh/f8dEACTDsbLN2nioNZMwyLuQRUAFcXNolDX48xcUXsWS2QjxaPm\n\tVsJx8Uy8aYkS85mdPBh0C83OovQR/OVbr8AxhGvYqBs3nQvbWuTl/+4od7DfK2VZOoKBAu5S\n\tQK2FYuUcikDqYcFWJ8DQnubxfE8dvzojHEkXw0sA4igINHDDFX3HJGZtLio+WpEFQtCbfTAG\n\tYZslasz1YZRbwEdSsmO3/kqy5eMnczlm8a21A3fKUo3g8oAZEFM+f4DUNzqIltg31OAB/kZS\n\tenKZQ/SWC8PmLg/ZXBrReYakxXtkP6w3FwMlzOlhGxqhIRNiAJfXJBaRhuUWzPOpEDE9q5YJ\n\tBmqQL2WJm1VSNNVxbXJHpaWMH1sA2R00vmvRrPXGwyIO0IPYeUYQa3gsy6k+En/aMQJd27dp\n\taScf9am9PFICPY5T4ppneeJLif2lyLojo0mcHOV+uyrds9XkLpp14GfTkeKPdPMrLLTsHRfH\n\tfA4I4OBpRrEPiGIZB/0im98MkGY/Mu6qxeZmYLCcgD6qz4idOvfgVOrNh+aA8HzIVR+RMW8H\n\tQGBN9f0E3kfwxuhl3omo6V7lDw8XOdmuWZNC9zPq1UfryVHANYbLGz9KJ4Aw6M+OgBC2JpkD\n\thXMdHUkC+d20dwXrwHTlrJi1YNp6rBc+xald3wsUPOZ5z8moTHUX/uPA/qhGsbkCDQRWBP1m\n\tARAAzijkb+Sau4hAncr1JjOY+KyFEdUNxRy+hqTJdJfaYihxyaj0Ee0P0zEi35CbE6lgU0Uz\n\ttih9fiUbSV3wfsWqg1Ut3/5rTKu7kLFp15kF7eqvV4uezXRD3Qu4yjv/rMmEJbbD4cTvGCYI\n\td6MDC417f7vK3hCbCVIZSp3GXxyC1LU+UQr3fFcOyCwmP9vDUR9JV0BSqHHxRDdpUXE26Dk6\n\tmhf0V1YkspE5St814ETXpEus2urZE5yJIUROlWPIL+hm3NEWfAP06vsQUyLvr/GtbOT79vXl\n\tEn1aulcYyu20dRRxhkQ6iILaURcxIAVJJKPi8dsoMnS8pB0QW12AHWuirPF0g6DiuUfPmrA5\n\tPKe56IGlpkjc8cO51lIxHkWTpCMWigRdPDexKX+Sb+W9QWK/0JjIc4t3KBaiG8O4yRX8ml2R\n\t+rxfAVKM6V769P/hWoRGdgUMgYHFpHGSgEt80OKK5HeUPy2cngDUXzwrqiM5Sz6Od0qw5pCk\n\tNlXqI0W/who0iSVM+8+RmyY0OEkxEcci7rRLsGnM15B5PjLJjh1f2ULYkv8s4SnDwMZ/kE04\n\t/UqCMK/KnX8pwXEMCjz0h6qWNpGwJ0/tYIgQJZh6bqkvBrDogAvuhf60Sogw+mH8b+PBlx1L\n\toeTK396wc+4c3BfiC6pNtUS5GpsPMMjYMk7kVvEAEQEAAYkCPAQYAQoAJgIbDBYhBJAt15g/\n\tvSj943LUeqEeRnIQpGH9BQJdizzIBQkLSKZiAAoJEKEeRnIQpGH9eYgQAJpjaWNgqNOnMTmD\n\tMJggbwjIotypzIXfhHNCeTkG7+qCDlSaBPclcPGYrTwCt0YWPU2TgGgJrVhYT20ierN8LUvj\n\t6qOPTd+Uk7NFzL65qkh80ZKNBFddx1AabQpSVQKbdcLb8OFs85kuSvFdgqZwgxA1vl4TFhNz\n\tPZ79NAmXLackAx3sOVFhk4WQaKRshCB7cSl+RIng5S/ThOBlwNlcKG7j7W2MC06BlTbdEkUp\n\tECzuuRBv8wX4OQl+hbWbB/VKIx5HKlLu1eypen/5lNVzSqMMIYkkZcjV2SWQyUGxSwq0O/sx\n\tS0A8/atCHUXOboUsn54qdxrVDaK+6jIAuo8JiRWctP16KjzUM7MO0/+4zllM8EY57rXrj48j\n\tsbEYX0YQnzaj+jO6kJtoZsIaYR7rMMq9aUAjyiaEZpmP1qF/2sYenDx0Fg2BSlLvLvXM0vU8\n\tpQk3kgDu7kb/7PRYrZvBsr21EIQoIjXbZxDz/o7z95frkP71EaICttZ6k9q5oxxA5WC6sTXc\n\tMW8zs8avFNuA9VpXt0YupJd2ijtZy2mpZNG02fFVXhIn4G807G7+9mhuC4XG5rKlBBUXTvPU\n\tAfYnB4JBDLmLzBFavQfvonSfbitgXwCG3vS+9HEwAjU30Bar1PEOmIbiAoMzuKeRm2LVpmq4\n\tWZw01QYHU/GUV/zHJSFk","Organization":"Ideas on Board","Message-ID":"<7253c17f-ccae-7e3d-f527-50a9764eb5c7@ideasonboard.com>","Date":"Tue, 3 Nov 2020 22:47:23 +0000","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101\n\tThunderbird/68.10.0","MIME-Version":"1.0","In-Reply-To":"<20201103155025.5948-3-laurent.pinchart@ideasonboard.com>","Content-Language":"en-GB","Subject":"Re: [libcamera-devel] [PATCH 2/7] qcam: viewfinder_gl: Keep\n\tfragment shader when format doesn't change","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>","Reply-To":"kieran.bingham@ideasonboard.com","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":13618,"web_url":"https://patchwork.libcamera.org/comment/13618/","msgid":"<20201106003705.GB3003904@oden.dyn.berto.se>","date":"2020-11-06T00:37:05","subject":"Re: [libcamera-devel] [PATCH 2/7] qcam: viewfinder_gl: Keep\n\tfragment shader when format doesn't change","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-11-03 17:50:20 +0200, Laurent Pinchart wrote:\n> When ViewFinderGL::setFormat() is called, the fragment shader is deleted\n> and recreated for the new format. This results in unnecessary shader\n> recompilation if only the size is changed and the pixel format remains\n> the same. Keep the existing shader in that case.\n> \n> The null test for fragmentShader_ can be removed, as if the shader\n> program is linked, the fragment shader is guaranteed to exist.\n> \n> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n\nReviewed-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>\n\n> ---\n>  src/qcam/viewfinder_gl.cpp | 16 ++++++++++------\n>  1 file changed, 10 insertions(+), 6 deletions(-)\n> \n> diff --git a/src/qcam/viewfinder_gl.cpp b/src/qcam/viewfinder_gl.cpp\n> index 03a576ba89d2..c07292523504 100644\n> --- a/src/qcam/viewfinder_gl.cpp\n> +++ b/src/qcam/viewfinder_gl.cpp\n> @@ -53,19 +53,23 @@ const QList<libcamera::PixelFormat> &ViewFinderGL::nativeFormats() const\n>  int ViewFinderGL::setFormat(const libcamera::PixelFormat &format,\n>  \t\t\t    const QSize &size)\n>  {\n> -\t/* If the fragment is created remove it and create a new one. */\n> -\tif (fragmentShader_) {\n> +\tif (format != format_) {\n> +\t\t/*\n> +\t\t * If the fragment already exists, remove it and create a new\n> +\t\t * one for the new format.\n> +\t\t */\n>  \t\tif (shaderProgram_.isLinked()) {\n>  \t\t\tshaderProgram_.release();\n>  \t\t\tshaderProgram_.removeShader(fragmentShader_.get());\n>  \t\t\tfragmentShader_.reset();\n>  \t\t}\n> +\n> +\t\tif (!selectFormat(format))\n> +\t\t\treturn -1;\n> +\n> +\t\tformat_ = format;\n>  \t}\n>  \n> -\tif (!selectFormat(format))\n> -\t\treturn -1;\n> -\n> -\tformat_ = format;\n>  \tsize_ = size;\n>  \n>  \tupdateGeometry();\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":"<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 F41B3BE082\n\tfor <parsemail@patchwork.libcamera.org>;\n\tFri,  6 Nov 2020 00:37:08 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 8799762D0E;\n\tFri,  6 Nov 2020 01:37:08 +0100 (CET)","from mail-lj1-x243.google.com (mail-lj1-x243.google.com\n\t[IPv6:2a00:1450:4864:20::243])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 6548662CAF\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tFri,  6 Nov 2020 01:37:07 +0100 (CET)","by mail-lj1-x243.google.com with SMTP id v18so1681577ljc.3\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tThu, 05 Nov 2020 16:37:07 -0800 (PST)","from localhost (h-209-203.A463.priv.bahnhof.se. [155.4.209.203])\n\tby smtp.gmail.com with ESMTPSA id\n\tr23sm340010lfe.228.2020.11.05.16.37.05\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tThu, 05 Nov 2020 16:37:06 -0800 (PST)"],"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=\"XKtXYFD9\"; 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=yAwl+a1903W8nES6IVrntpA3tJSTA8ZbP1o6s0bmGcg=;\n\tb=XKtXYFD9+0k1BaimPkPUM79Qdwv2IRziwfilVfjGawGizUGvKbLB0tP3RvbXPX+Ah5\n\tT8y2IABNCNc8kHjB5Q3RgtQZxOLOq7M+Z500HsIK/CvLXSOKsSv2wgRfsYAJ9fhzFHoh\n\trYwsJL++3V7kpsS75puzp0sL5fSyvxUtOM8T15j2bxCR+DUquQS0BFoUBMfNKX2KjNMc\n\tKobd3zl+u5qnq0KqVDvCFxIsvk25nhTeFfgiM0ckN8T9VXamCQ7lJUaGJYmQlUcWYuwS\n\t3MvhZArtCCpUBDs8k1mhMaKmma8zkUwrVnvd5JfwUdibtaVrGleBnxXtAZcYUlkfVq8N\n\tuJvA==","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=yAwl+a1903W8nES6IVrntpA3tJSTA8ZbP1o6s0bmGcg=;\n\tb=dZGqhyfAGvwphRmYZB/qbbXCTheURSC7mEb/WwtzJXw0Xrx+Q7xXw8mCQ8rV/gLxO1\n\tgkUGxZUeavE5xcWHrYMjm2CyXEC+eh+t3yiFeSlgJJRE7SU0yS7J1tBHtA9cD46yMtZS\n\tahKfWa8G3AiYjY+3kGfCbFHMlMZVcDbtA7mokZ6pEB6TBugvCVjJuf3GqOOVo9qlM4gG\n\t4mZyI7hLdPERoP2foKb230fXZsVgFZAShFOb22lOhMuW64aSkVv51jmgUC80uMxuRmXr\n\t7HWWlwAEktoHk7/4fyhS3DuPHhVA0IO9T+F2CwzbeDrjGkbRqg+/tvOtqhTByS5LwcaD\n\tQ+KQ==","X-Gm-Message-State":"AOAM533xtyQFdGKxg6D9CTcbzGoF8+i3sAMQKBrFWtoo4X8uWaFZErZ1\n\tGPJyyeJVQdgh75Ao+Hkn5IniDg==","X-Google-Smtp-Source":"ABdhPJzpsnEIKC8lxAN8BtjGFRuqC1e0SRg/Xqr3zKGsPyDu/jRUGzB14zz1hcLXI4BoHoaPYHFHCw==","X-Received":"by 2002:a2e:819a:: with SMTP id\n\te26mr1917187ljg.469.1604623026800; \n\tThu, 05 Nov 2020 16:37:06 -0800 (PST)","Date":"Fri, 6 Nov 2020 01:37:05 +0100","From":"Niklas =?iso-8859-1?q?S=F6derlund?= <niklas.soderlund@ragnatech.se>","To":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","Message-ID":"<20201106003705.GB3003904@oden.dyn.berto.se>","References":"<20201103155025.5948-1-laurent.pinchart@ideasonboard.com>\n\t<20201103155025.5948-3-laurent.pinchart@ideasonboard.com>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20201103155025.5948-3-laurent.pinchart@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH 2/7] qcam: viewfinder_gl: Keep\n\tfragment shader when format doesn't change","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":"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\" <libcamera-devel-bounces@lists.libcamera.org>"}}]