| Message ID | 20251215093706.573761-1-barnabas.pocze@ideasonboard.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series |
|
| Related | show |
Hi Barnabás, On Mon, Dec 15, 2025 at 10:37:06AM +0100, Barnabás Pőcze wrote: > Meson already takes care of passing the proper absolute or relative > paths to commands. There is no need do more path manipulation. That's because all commands are run from the root of the build directory. It's worth mentioning it here (or maybe just worth remembering for the people who reviewed and merged 19371dee4146b7 :-)). > > So simplify the script by using the paths as-is. This also fixes the > path manipulation issue that prevented libcamera from building as a > subproject. > > Fixes: 19371dee4146b7 ("utils: gen-shader-headers: Add a utility to generate headers from shaders") > Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com> I started simplifying the shader generation by merging the shell and Python scripts together. We can merge this patch first and I'll rebase my work in progress. Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > --- > src/libcamera/meson.build | 2 +- > utils/gen-shader-headers.sh | 14 ++++++-------- > 2 files changed, 7 insertions(+), 9 deletions(-) > > diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build > index 90d434a5a2..575408b2c7 100644 > --- a/src/libcamera/meson.build > +++ b/src/libcamera/meson.build > @@ -188,7 +188,7 @@ libcamera_shader_headers = custom_target( > 'gen-shader-headers', > input : [shader_files], > output : 'glsl_shaders.h', > - command : [gen_shader_headers, meson.project_source_root(), meson.project_build_root(), '@OUTPUT@', '@INPUT@'], > + command : [gen_shader_headers, meson.project_source_root(), '@OUTPUT@', '@INPUT@'], > ) > > libcamera_internal_headers += libcamera_shader_headers > diff --git a/utils/gen-shader-headers.sh b/utils/gen-shader-headers.sh > index 81bf1584c9..92f6f81815 100755 > --- a/utils/gen-shader-headers.sh > +++ b/utils/gen-shader-headers.sh > @@ -3,14 +3,13 @@ > set -e > > usage() { > - echo "Usage: $0 <src_dir> <build_dir> <output_header_name> <shader_file1> [shader_file2 ...]" > + echo "Usage: $0 <src_dir> <output_header> <shader_file1> [shader_file2 ...]" > echo > echo "Generates a C header file containing hex-encoded shader data." > echo > echo "Arguments:" > echo " src_dir Path to the base of the source directory" > - echo " build_dir Directory where shader files are located and header will be written" > - echo " output_header_name Name of the generated header file (relative to build_dir)" > + echo " output_header Path to the generated header file" > echo " shader_file(s) One or more shader files to embed in the header" > exit 1 > } > @@ -21,8 +20,7 @@ if [ $# -lt 4 ]; then > fi > > src_dir="$1"; shift > -build_dir="$1"; shift > -build_path=$build_dir/"$1"; shift > +build_path="$1"; shift > > cat <<EOF > "$build_path" > /* SPDX-License-Identifier: LGPL-2.1-or-later */ > @@ -40,7 +38,7 @@ cat <<EOF >> "$build_path" > EOF > > for file in "$@"; do > - name=$(basename "$build_dir/$file" | tr '.' '_') > + name=$(basename "$file" | tr '.' '_') > echo "[SHADER-GEN] $name" > echo " * unsigned char $name;" >> "$build_path" > done > @@ -49,7 +47,7 @@ echo "*/" >> "$build_path" > > echo "/* Hex encoded shader data */" >> "$build_path" > for file in "$@"; do > - name=$(basename "$build_dir/$file") > - "$src_dir/utils/gen-shader-header.py" "$name" "$build_dir/$file" >> "$build_path" > + name=$(basename "$file") > + "$src_dir/utils/gen-shader-header.py" "$name" "$file" >> "$build_path" > echo >> "$build_path" > done
Quoting Laurent Pinchart (2025-12-15 10:18:54) > Hi Barnabás, > > On Mon, Dec 15, 2025 at 10:37:06AM +0100, Barnabás Pőcze wrote: > > Meson already takes care of passing the proper absolute or relative > > paths to commands. There is no need do more path manipulation. > > That's because all commands are run from the root of the build > directory. It's worth mentioning it here (or maybe just worth > remembering for the people who reviewed and merged 19371dee4146b7 :-)). If we miss test cases we miss test opportunities! -- Kieran > > > > > So simplify the script by using the paths as-is. This also fixes the > > path manipulation issue that prevented libcamera from building as a > > subproject. > > > > Fixes: 19371dee4146b7 ("utils: gen-shader-headers: Add a utility to generate headers from shaders") > > Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com> > > I started simplifying the shader generation by merging the shell and > Python scripts together. We can merge this patch first and I'll rebase > my work in progress. > > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > > > --- > > src/libcamera/meson.build | 2 +- > > utils/gen-shader-headers.sh | 14 ++++++-------- > > 2 files changed, 7 insertions(+), 9 deletions(-) > > > > diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build > > index 90d434a5a2..575408b2c7 100644 > > --- a/src/libcamera/meson.build > > +++ b/src/libcamera/meson.build > > @@ -188,7 +188,7 @@ libcamera_shader_headers = custom_target( > > 'gen-shader-headers', > > input : [shader_files], > > output : 'glsl_shaders.h', > > - command : [gen_shader_headers, meson.project_source_root(), meson.project_build_root(), '@OUTPUT@', '@INPUT@'], > > + command : [gen_shader_headers, meson.project_source_root(), '@OUTPUT@', '@INPUT@'], > > ) > > > > libcamera_internal_headers += libcamera_shader_headers > > diff --git a/utils/gen-shader-headers.sh b/utils/gen-shader-headers.sh > > index 81bf1584c9..92f6f81815 100755 > > --- a/utils/gen-shader-headers.sh > > +++ b/utils/gen-shader-headers.sh > > @@ -3,14 +3,13 @@ > > set -e > > > > usage() { > > - echo "Usage: $0 <src_dir> <build_dir> <output_header_name> <shader_file1> [shader_file2 ...]" > > + echo "Usage: $0 <src_dir> <output_header> <shader_file1> [shader_file2 ...]" > > echo > > echo "Generates a C header file containing hex-encoded shader data." > > echo > > echo "Arguments:" > > echo " src_dir Path to the base of the source directory" > > - echo " build_dir Directory where shader files are located and header will be written" > > - echo " output_header_name Name of the generated header file (relative to build_dir)" > > + echo " output_header Path to the generated header file" > > echo " shader_file(s) One or more shader files to embed in the header" > > exit 1 > > } > > @@ -21,8 +20,7 @@ if [ $# -lt 4 ]; then > > fi > > > > src_dir="$1"; shift > > -build_dir="$1"; shift > > -build_path=$build_dir/"$1"; shift > > +build_path="$1"; shift > > > > cat <<EOF > "$build_path" > > /* SPDX-License-Identifier: LGPL-2.1-or-later */ > > @@ -40,7 +38,7 @@ cat <<EOF >> "$build_path" > > EOF > > > > for file in "$@"; do > > - name=$(basename "$build_dir/$file" | tr '.' '_') > > + name=$(basename "$file" | tr '.' '_') > > echo "[SHADER-GEN] $name" > > echo " * unsigned char $name;" >> "$build_path" > > done > > @@ -49,7 +47,7 @@ echo "*/" >> "$build_path" > > > > echo "/* Hex encoded shader data */" >> "$build_path" > > for file in "$@"; do > > - name=$(basename "$build_dir/$file") > > - "$src_dir/utils/gen-shader-header.py" "$name" "$build_dir/$file" >> "$build_path" > > + name=$(basename "$file") > > + "$src_dir/utils/gen-shader-header.py" "$name" "$file" >> "$build_path" > > echo >> "$build_path" > > done > > -- > Regards, > > Laurent Pinchart
Quoting Laurent Pinchart (2025-12-15 10:18:54) > Hi Barnabás, > > On Mon, Dec 15, 2025 at 10:37:06AM +0100, Barnabás Pőcze wrote: > > Meson already takes care of passing the proper absolute or relative > > paths to commands. There is no need do more path manipulation. > > That's because all commands are run from the root of the build > directory. It's worth mentioning it here (or maybe just worth > remembering for the people who reviewed and merged 19371dee4146b7 :-)). Also - worth remembering that more iterations on that would have incurred yet another 20 patches hitting the list (42 if we hadn't split out the precursor series) - *this* is exactly why I'm happy to have merged 19371dee4146b7 and the others in that series to expand the testing and allow things like this to be identified and fixed on top... Just like this ;-) Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > > So simplify the script by using the paths as-is. This also fixes the > > path manipulation issue that prevented libcamera from building as a > > subproject. > > > > Fixes: 19371dee4146b7 ("utils: gen-shader-headers: Add a utility to generate headers from shaders") > > Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com> > > I started simplifying the shader generation by merging the shell and > Python scripts together. We can merge this patch first and I'll rebase > my work in progress. > > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > > > --- > > src/libcamera/meson.build | 2 +- > > utils/gen-shader-headers.sh | 14 ++++++-------- > > 2 files changed, 7 insertions(+), 9 deletions(-) > > > > diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build > > index 90d434a5a2..575408b2c7 100644 > > --- a/src/libcamera/meson.build > > +++ b/src/libcamera/meson.build > > @@ -188,7 +188,7 @@ libcamera_shader_headers = custom_target( > > 'gen-shader-headers', > > input : [shader_files], > > output : 'glsl_shaders.h', > > - command : [gen_shader_headers, meson.project_source_root(), meson.project_build_root(), '@OUTPUT@', '@INPUT@'], > > + command : [gen_shader_headers, meson.project_source_root(), '@OUTPUT@', '@INPUT@'], > > ) > > > > libcamera_internal_headers += libcamera_shader_headers > > diff --git a/utils/gen-shader-headers.sh b/utils/gen-shader-headers.sh > > index 81bf1584c9..92f6f81815 100755 > > --- a/utils/gen-shader-headers.sh > > +++ b/utils/gen-shader-headers.sh > > @@ -3,14 +3,13 @@ > > set -e > > > > usage() { > > - echo "Usage: $0 <src_dir> <build_dir> <output_header_name> <shader_file1> [shader_file2 ...]" > > + echo "Usage: $0 <src_dir> <output_header> <shader_file1> [shader_file2 ...]" > > echo > > echo "Generates a C header file containing hex-encoded shader data." > > echo > > echo "Arguments:" > > echo " src_dir Path to the base of the source directory" > > - echo " build_dir Directory where shader files are located and header will be written" > > - echo " output_header_name Name of the generated header file (relative to build_dir)" > > + echo " output_header Path to the generated header file" > > echo " shader_file(s) One or more shader files to embed in the header" > > exit 1 > > } > > @@ -21,8 +20,7 @@ if [ $# -lt 4 ]; then > > fi > > > > src_dir="$1"; shift > > -build_dir="$1"; shift > > -build_path=$build_dir/"$1"; shift > > +build_path="$1"; shift > > > > cat <<EOF > "$build_path" > > /* SPDX-License-Identifier: LGPL-2.1-or-later */ > > @@ -40,7 +38,7 @@ cat <<EOF >> "$build_path" > > EOF > > > > for file in "$@"; do > > - name=$(basename "$build_dir/$file" | tr '.' '_') > > + name=$(basename "$file" | tr '.' '_') > > echo "[SHADER-GEN] $name" > > echo " * unsigned char $name;" >> "$build_path" > > done > > @@ -49,7 +47,7 @@ echo "*/" >> "$build_path" > > > > echo "/* Hex encoded shader data */" >> "$build_path" > > for file in "$@"; do > > - name=$(basename "$build_dir/$file") > > - "$src_dir/utils/gen-shader-header.py" "$name" "$build_dir/$file" >> "$build_path" > > + name=$(basename "$file") > > + "$src_dir/utils/gen-shader-header.py" "$name" "$file" >> "$build_path" > > echo >> "$build_path" > > done > > -- > Regards, > > Laurent Pinchart
diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build index 90d434a5a2..575408b2c7 100644 --- a/src/libcamera/meson.build +++ b/src/libcamera/meson.build @@ -188,7 +188,7 @@ libcamera_shader_headers = custom_target( 'gen-shader-headers', input : [shader_files], output : 'glsl_shaders.h', - command : [gen_shader_headers, meson.project_source_root(), meson.project_build_root(), '@OUTPUT@', '@INPUT@'], + command : [gen_shader_headers, meson.project_source_root(), '@OUTPUT@', '@INPUT@'], ) libcamera_internal_headers += libcamera_shader_headers diff --git a/utils/gen-shader-headers.sh b/utils/gen-shader-headers.sh index 81bf1584c9..92f6f81815 100755 --- a/utils/gen-shader-headers.sh +++ b/utils/gen-shader-headers.sh @@ -3,14 +3,13 @@ set -e usage() { - echo "Usage: $0 <src_dir> <build_dir> <output_header_name> <shader_file1> [shader_file2 ...]" + echo "Usage: $0 <src_dir> <output_header> <shader_file1> [shader_file2 ...]" echo echo "Generates a C header file containing hex-encoded shader data." echo echo "Arguments:" echo " src_dir Path to the base of the source directory" - echo " build_dir Directory where shader files are located and header will be written" - echo " output_header_name Name of the generated header file (relative to build_dir)" + echo " output_header Path to the generated header file" echo " shader_file(s) One or more shader files to embed in the header" exit 1 } @@ -21,8 +20,7 @@ if [ $# -lt 4 ]; then fi src_dir="$1"; shift -build_dir="$1"; shift -build_path=$build_dir/"$1"; shift +build_path="$1"; shift cat <<EOF > "$build_path" /* SPDX-License-Identifier: LGPL-2.1-or-later */ @@ -40,7 +38,7 @@ cat <<EOF >> "$build_path" EOF for file in "$@"; do - name=$(basename "$build_dir/$file" | tr '.' '_') + name=$(basename "$file" | tr '.' '_') echo "[SHADER-GEN] $name" echo " * unsigned char $name;" >> "$build_path" done @@ -49,7 +47,7 @@ echo "*/" >> "$build_path" echo "/* Hex encoded shader data */" >> "$build_path" for file in "$@"; do - name=$(basename "$build_dir/$file") - "$src_dir/utils/gen-shader-header.py" "$name" "$build_dir/$file" >> "$build_path" + name=$(basename "$file") + "$src_dir/utils/gen-shader-header.py" "$name" "$file" >> "$build_path" echo >> "$build_path" done
Meson already takes care of passing the proper absolute or relative paths to commands. There is no need do more path manipulation. So simplify the script by using the paths as-is. This also fixes the path manipulation issue that prevented libcamera from building as a subproject. Fixes: 19371dee4146b7 ("utils: gen-shader-headers: Add a utility to generate headers from shaders") Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com> --- src/libcamera/meson.build | 2 +- utils/gen-shader-headers.sh | 14 ++++++-------- 2 files changed, 7 insertions(+), 9 deletions(-)