[{"id":24564,"web_url":"https://patchwork.libcamera.org/comment/24564/","msgid":"<166049713157.15821.10786827242703471911@Monstersaurus>","date":"2022-08-14T17:12:11","subject":"Re: [libcamera-devel] [PATCH v3 2/6] pipeline: simple: Implements\n\tShader Handling","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Hi Kunal,\n\nQuoting Kunal Agarwal via libcamera-devel (2022-08-14 17:07:43)\n> Shader Class which includes functions for building,\n> activating, deleting and compiling shaders\n\nGreat, Thanks for working on this, I've been looking forwards to how we\ncan integrate shaders directly into pipelines!\n\n> Signed-off-by: Kunal Agarwal \n> ---\n> src/libcamera/pipeline/simple/shader.cpp | 110 +++++++++++++++++++++++\n> src/libcamera/pipeline/simple/shader.h | 34 +++++++\n> 2 files changed, 144 insertions(+)\n> create mode 100644 src/libcamera/pipeline/simple/shader.cpp\n> create mode 100644 src/libcamera/pipeline/simple/shader.h\n> \n> diff --git a/src/libcamera/pipeline/simple/shader.cpp b/src/libcamera/pipeline/simple/shader.cpp\n> new file mode 100644\n> index 00000000..f0079618\n> --- /dev/null\n> +++ b/src/libcamera/pipeline/simple/shader.cpp\n> @@ -0,0 +1,110 @@\n> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> +/*\n> + * Copyright (C) 2022, Kunal Agarwal\n> + *\n> + * shader.cpp - Shader Handling\n> + */\n> +\n> +#include \"shader.h\"\n> +\n> +#include \n> +#include \n> +\n> +#include \n> +\n> +namespace libcamera {\n> +\n> +LOG_DECLARE_CATEGORY(SimplePipeline)\n\nIt may depend on how the development goes, but I think I'd see the\n'Shader' support classes in core libcamera, along with suitable tests\nthat load and test under test/, and 'used' by the Simple Pipeline\nhandler.\n\n\nThat said, it probably doesn't hurt for it to start here, and move to\ncore later if / when it gets expanded for it's use cases.\n\n\n> +\n> +/* Reads a text file and outputs a string with everything in the text file */\n> +static std::string get_file_contents(const char *filename)\n> +{\n> + std::string fullname = std::string(\"/home/pi/Desktop/compile/libcamera/src/libcamera/pipeline/simple/shader/\") + filename;\n\nPerhaps this needs to be a libcamera defined data path?\n\nThe path /home/pi/Desktop ... doesn't map to anything useful on my\nmachine...\n\n> +\n> + File file(fullname);\n> + if (!file.open(File::OpenModeFlag::ReadOnly))\n> + return \"\";\n> +\n> + Span data = file.map();\n> + return std::string(reinterpret_cast(data.data()), data.size());\n\nAt the point this function returns, I think \"File file\" will destruct,\nand close - and then from that point on - this data is an invalid\nuse-after-free.\n\n\n> +}\n> +\n> +/* Constructor that build the Shader Program from 2 different shaders */\n> +void ShaderProgram::callShader(const char *vertexFile, const char *fragmentFile)\n> +{\n> + /* Read vertexFile and fragmentFile and store the strings */\n> + std::string vertexCode = get_file_contents(vertexFile);\n> + std::string fragmentCode = get_file_contents(fragmentFile);\n> + const char *vertexSource = vertexCode.c_str();\n> + const char *fragmentSource = fragmentCode.c_str();\n> +\n> + /* Create the vertex shader, set its source code and compile it. */\n> + GLuint vertexShader = glCreateShader(GL_VERTEX_SHADER);\n> + glShaderSource(vertexShader, 1, &vertexSource, NULL);\n> + glCompileShader(vertexShader);\n> + compileErrors(vertexShader, \"VERTEX\");\n> +\n> + /* Create the fragment shader, set its source code and compile it. */\n> + GLuint fragmentShader = glCreateShader(GL_FRAGMENT_SHADER);\n> + glShaderSource(fragmentShader, 1, &fragmentSource, NULL);\n> + glCompileShader(fragmentShader);\n> + compileErrors(fragmentShader, \"FRAGMENT\");\n> +\n> + /* Create Shader Program Object and get its reference */\n> + id_ = glCreateProgram();\n> +\n> + /* Attach and wrap-up/link the Vertex and Fragment Shaders to the Shader Program */\n> + glAttachShader(id_, vertexShader);\n> + glAttachShader(id_, fragmentShader);\n> + glLinkProgram(id_);\n> +\n> + /* Checks if Shaders linked succesfully */\n> + compileErrors(id_, \"PROGRAM\");\n> +\n> + /* Delete the Vertex and Fragment Shader objects. Here, they are flagged for deletion\n> + and will not be deleted until they are detached from the program object. This frees\n> + up the memory used to store the shader source. */\n\n\t/*\n\t * Multiline comment style should look like\n\t * this, and be wrapped at ~80 characters.\n\t */\n\nIt sounds like the Vertex, and Fragment shader objects are reference\ncounted by OpenGL and so all we're really doing here is releasing our\nreference, but they'll persist while they are in used by an active\nshader/program?\n\n\n> + glDeleteShader(vertexShader);\n> + glDeleteShader(fragmentShader);\n> +}\n> +\n> +/* Activates the Shader Program */\n> +void ShaderProgram::activate()\n> +{\n> + glUseProgram(id_);\n> +}\n> +\n> +/* Deletes the Shader Program */\n> +void ShaderProgram::deleteProgram()\n> +{\n> + glDeleteProgram(id_);\n> +}\n> +\n> +/* Checks if the different Shaders have compiled properly */\n> +void ShaderProgram::compileErrors(unsigned int shader, const char *type)\n> +{\n> + /* Stores status of compilation */\n> + GLint hasCompiled;\n> + GLint logLength;\n> + /* Character array to store error message in */\n> + glGetShaderiv(shader, GL_INFO_LOG_LENGTH, &logLength);\n> + char *infoLog = new char[logLength];\n> + if (strcmp(type, \"PROGRAM\") != 0) {\n> + glGetShaderiv(shader, GL_COMPILE_STATUS, &hasCompiled);\n> + if (hasCompiled == GL_FALSE) {\n> + glGetShaderInfoLog(shader, logLength, NULL, infoLog);\n> + LOG(SimplePipeline, Error) << \"SHADER_COMPILATION_ERROR for:\"\n> + << type << \"\\t\"\n> + << infoLog;\n> + }\n> + } else {\n> + glGetProgramiv(shader, GL_LINK_STATUS, &hasCompiled);\n> + if (hasCompiled == GL_FALSE) {\n> + glGetProgramInfoLog(shader, logLength, NULL, infoLog);\n> + LOG(SimplePipeline, Error) << \"SHADER_LINKING_ERROR for:\"\n> + << type << \"\\t\"\n> + << infoLog;\n> + }\n> + }\n> +}\n> +} /* namespace libcamera */\n> diff --git a/src/libcamera/pipeline/simple/shader.h b/src/libcamera/pipeline/simple/shader.h\n> new file mode 100644\n> index 00000000..921e4040\n> --- /dev/null\n> +++ b/src/libcamera/pipeline/simple/shader.h\n> @@ -0,0 +1,34 @@\n> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> +/*\n> + * Copyright (C) 2022, Kunal Agarwal\n> + *\n> + * shader.h - Shader Handling\n> + */\n> +\n> +#pragma once\n> +\n> +#include \n> +#include \n> +\n> +#include \n> +\n> +namespace libcamera {\n> +\n> +class ShaderProgram\n> +{\n> +public:\n> + void callShader(const char *vertexFile, const char *fragmentFile);\n> +\n> + void activate();\n> +\n> + void deleteProgram();\n> +\n> + int id() const { return id_; };\n> +\n> +private:\n> + /* Reference ID of the Shader Program */\n> + GLuint id_;\n> + void compileErrors(unsigned int shader, const char *type);\n> +};\n> +\n> +} /* namespace libcamera */\n> -- \n> 2.25.1\n>","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 9AB44BE173\n\tfor ;\n\tSun, 14 Aug 2022 17:12:16 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id E061061FC0;\n\tSun, 14 Aug 2022 19:12:15 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 4E71D61FBA\n\tfor ;\n\tSun, 14 Aug 2022 19:12:14 +0200 (CEST)","from pendragon.ideasonboard.com\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 C1B2E30A;\n\tSun, 14 Aug 2022 19:12:13 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1660497135;\n\tbh=FUE7ux9xeVjoCYtk800LB4/zzOQQdFJFODpr8hAlCD8=;\n\th=In-Reply-To:References:To:Date:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:\n\tFrom;\n\tb=pF69zeB7jEBf0IAusnyjhCNzEcuK+DQuvtUlwjWQMel/9WZ0dwmxEdTCPNE69FSQI\n\tdUq+yn/Lsf3LNNzTlVfheQ4SaTgHKTfgJhPlJPyTUGwgh0ZhABewv7PEVTNa6t9R5+\n\txcFSDChjcZzp+Oyd6dyuE1H/nGj8sAHNVA2qRYMEYpRDt/lxE3tQmctaDOpfs4Cuj6\n\t6F5kvIQJc1pDndhf2H5Nq8BNNQkbTKP0vGWv/ecN+2VEOlFU83UJTSaECDU2T5LzP1\n\tB4XQh3RG6JRa5vvOGU0+aDyD5KaWes5KOiROvsLEOmInY/e77d0Z6ZCm6D+dlAFnhG\n\tf6iUCxvkwsNzQ==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1660497133;\n\tbh=FUE7ux9xeVjoCYtk800LB4/zzOQQdFJFODpr8hAlCD8=;\n\th=In-Reply-To:References:Subject:From:To:Date:From;\n\tb=s9lNMcWbXzmsGT3eLcIQXnpT/N+7/OMYWCGy3z52h/dV+0K3MXZn8wlBum0LP7eio\n\t5J3uJxY9sfqPMDBuvfCVqjy4NUd+6DQVsPL+nyXgWI21mQCfFufL/txNbqbxE68i0D\n\tdzh29q/DObvJWLVx1prdyRZuX28vyuPn/vTWrqa0="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"s9lNMcWb\"; dkim-atps=neutral","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<20220814160747.52093-2-kunalagarwal1072002@gmail.com>","References":"<20220814160747.52093-1-kunalagarwal1072002@gmail.com>\n\t<20220814160747.52093-2-kunalagarwal1072002@gmail.com>","To":"Kunal Agarwal ,\n\tlibcamera-devel@lists.libcamera.org","Date":"Sun, 14 Aug 2022 18:12:11 +0100","Message-ID":"<166049713157.15821.10786827242703471911@Monstersaurus>","User-Agent":"alot/0.10","Subject":"Re: [libcamera-devel] [PATCH v3 2/6] pipeline: simple: Implements\n\tShader Handling","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","From":"Kieran Bingham via libcamera-devel\n\t","Reply-To":"Kieran Bingham ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]