From 901189ce22662f02e16aec814b86a9794b4e95ea Mon Sep 17 00:00:00 2001 From: Pavlo Penenko Date: Thu, 19 Dec 2024 19:10:14 -0500 Subject: [PATCH 01/10] WIP on adding diffs to the test --- .gitmodules | 3 +++ metashade | 2 +- tests/ref | 1 + tests/test_generate.py | 11 +++++++++-- 4 files changed, 14 insertions(+), 3 deletions(-) create mode 160000 tests/ref diff --git a/.gitmodules b/.gitmodules index 4861d1d..ff1a2c6 100644 --- a/.gitmodules +++ b/.gitmodules @@ -5,3 +5,6 @@ path = glTFSample url = https://github.com/metashade/glTFSample.git branch = metashade_demo +[submodule "tests/ref"] + path = tests/ref + url = https://github.com/metashade/metashade-glTFSample-testrefs.git diff --git a/metashade b/metashade index e3fddf9..1427a99 160000 --- a/metashade +++ b/metashade @@ -1 +1 @@ -Subproject commit e3fddf9511e75b31897bb494c6dd37e5c35f4ca8 +Subproject commit 1427a99527dfd2485bec14e92ffd7645f82696e8 diff --git a/tests/ref b/tests/ref new file mode 160000 index 0000000..46fae35 --- /dev/null +++ b/tests/ref @@ -0,0 +1 @@ +Subproject commit 46fae35c79b5b76617f034f6c338ce2f0bbbf2dc diff --git a/tests/test_generate.py b/tests/test_generate.py index da6015f..4a4cd2f 100644 --- a/tests/test_generate.py +++ b/tests/test_generate.py @@ -12,18 +12,25 @@ # See the License for the specific language governing permissions and # limitations under the License. -import os, sys +import sys from pathlib import Path tests_dir_path = Path(__file__).parent repo_root_dir_path = tests_dir_path.parent + +# Add these directories to PYTHONPATH src_dir_path = (repo_root_dir_path / 'src').resolve() metashade_dir_path = (repo_root_dir_path / 'metashade').resolve() - sys.path += [str(src_dir_path), str(metashade_dir_path)] +from metashade.util.tests import RefDiffer import generate +ref_differ = RefDiffer( + ref_dir = repo_root_dir_path / 'tests' / 'ref', + out_dir_env_var = 'METASHADE_GLTFSAMPLE_PYTEST_OUT_DIR' +) + class TestGenerate: def test_generate(self): gltf_sample_dir_path = repo_root_dir_path / 'glTFSample' From 9a71adfca897d6d5209a06d88a400dc2fca6c6dc Mon Sep 17 00:00:00 2001 From: Pavlo Penenko Date: Sat, 21 Dec 2024 13:50:59 -0500 Subject: [PATCH 02/10] Refactor common compilation code --- src/generate.py | 35 ++++++++++++++++------------------- 1 file changed, 16 insertions(+), 19 deletions(-) diff --git a/src/generate.py b/src/generate.py index 05b9a2f..375b6f5 100644 --- a/src/generate.py +++ b/src/generate.py @@ -55,9 +55,18 @@ class CompileResult(NamedTuple): success : bool @abc.abstractmethod - def compile(self, to_glsl : bool) -> CompileResult: + def _compile(self, to_glsl : bool) -> bool: pass + def compile(self, to_glsl : bool) -> CompileResult: + log = io.StringIO() + log, sys.stdout = sys.stdout, log + + success = self._compile(to_glsl) + + log, sys.stdout = sys.stdout, log + return _Shader.CompileResult(log.getvalue(), success) + def _compile_shader(shader, to_glsl : bool) -> _Shader.CompileResult: ''' Helper function to compile a shader in a process pool. @@ -70,10 +79,7 @@ class _HlslShader(_Shader): def _get_hlsl_profile(): pass - def compile(self, to_glsl : bool) -> _Shader.CompileResult: - log = io.StringIO() - log, sys.stdout = sys.stdout, log - + def _compile(self, to_glsl : bool) -> bool: try: dxc_output_path = Path(self._file_path).with_suffix( '.hlsl.spv' if to_glsl else '.cso' @@ -102,12 +108,9 @@ def compile(self, to_glsl : bool) -> _Shader.CompileResult: entry_point_name = _impl.entry_point_name, output_path = spv_path ) - success = True + return True except subprocess.CalledProcessError as err: - success = False - - log, sys.stdout = sys.stdout, log - return _Shader.CompileResult(log.getvalue(), success) + return False class _HlslVertexShader(_HlslShader): def __init__( @@ -154,10 +157,7 @@ def _generate(self, shader_file, material, primitive): ) class _GlslShader(_Shader): - def compile(self, to_glsl : bool) -> _Shader.CompileResult: - log = io.StringIO() - log, sys.stdout = sys.stdout, log - + def _compile(self, to_glsl : bool) -> bool: try: glsl_output_path = Path(self._file_path).with_suffix('.spv') glslang.compile( @@ -166,12 +166,9 @@ def compile(self, to_glsl : bool) -> _Shader.CompileResult: shader_stage = 'frag', output_path = glsl_output_path ) - success = True + return True except subprocess.CalledProcessError as err: - success = False - - log, sys.stdout = sys.stdout, log - return _Shader.CompileResult(log.getvalue(), success) + return False class _GlslFragmentShader(_GlslShader): def __init__( From 2e5f5bd02fe57cff17d22664bdfee0a6459d4162 Mon Sep 17 00:00:00 2001 From: Pavlo Penenko Date: Sat, 21 Dec 2024 15:31:49 -0500 Subject: [PATCH 03/10] Injecting `RefDiffer` --- src/generate.py | 30 +++++++++++++++++++++++------- 1 file changed, 23 insertions(+), 7 deletions(-) diff --git a/src/generate.py b/src/generate.py index 375b6f5..ff08f68 100644 --- a/src/generate.py +++ b/src/generate.py @@ -21,6 +21,7 @@ from metashade.util import perf, spirv_cross from metashade.hlsl.util import dxc from metashade.glsl.util import glslang, glslc +from metashade.util.tests import RefDiffer import _impl @@ -58,21 +59,28 @@ class CompileResult(NamedTuple): def _compile(self, to_glsl : bool) -> bool: pass - def compile(self, to_glsl : bool) -> CompileResult: + def compile(self, to_glsl : bool, ref_differ : RefDiffer) -> CompileResult: log = io.StringIO() log, sys.stdout = sys.stdout, log + if ref_differ is not None: + ref_differ(self._file_path) + success = self._compile(to_glsl) log, sys.stdout = sys.stdout, log return _Shader.CompileResult(log.getvalue(), success) -def _compile_shader(shader, to_glsl : bool) -> _Shader.CompileResult: +def _compile_shader( + shader, + to_glsl : bool, + ref_differ : RefDiffer +) -> _Shader.CompileResult: ''' Helper function to compile a shader in a process pool. Without it, the pool would not be able to pickle the method. ''' - return shader.compile(to_glsl) + return shader.compile(to_glsl, ref_differ) class _HlslShader(_Shader): @abc.abstractmethod @@ -234,7 +242,8 @@ def generate( compile : bool, to_glsl : bool, skip_codegen : bool, - serial : bool + serial : bool, + ref_differ : RefDiffer ): if not gltf_dir_path.is_dir(): raise NotADirectoryError(gltf_dir_path) @@ -277,7 +286,10 @@ def generate( if serial: for shader in shaders: - result = shader.compile(to_glsl = to_glsl) + result = shader.compile( + to_glsl = to_glsl, + ref_differ = ref_differ + ) if not result.success: num_failed += 1 print(result.log, end = '') @@ -286,7 +298,8 @@ def generate( for result in pool.imap_unordered( functools.partial( _compile_shader, - to_glsl = to_glsl + to_glsl = to_glsl, + ref_differ = ref_differ ), shaders ): @@ -308,6 +321,8 @@ def generate( ) parser.add_argument("--gltf-dir", help = "Path to the source glTF assets") parser.add_argument("--out-dir", help = "Path to the output directory") + parser.add_argument("--ref-dir", help = "Path to the test references") + parser.add_argument( "--compile", action = 'store_true', @@ -337,5 +352,6 @@ def generate( compile = args.compile, to_glsl = args.to_glsl, skip_codegen = args.skip_codegen, - serial = args.serial + serial = args.serial, + ref_differ = RefDiffer(Path(args.ref_dir)) if args.ref_dir else None ) \ No newline at end of file From 1e4df65d35f06de14e8d23f25ba522226ffad326 Mon Sep 17 00:00:00 2001 From: Pavlo Penenko Date: Sat, 21 Dec 2024 15:50:27 -0500 Subject: [PATCH 04/10] Integrate `RefDiffer` with the test --- tests/test_generate.py | 31 ++++++++++++++++++++++--------- 1 file changed, 22 insertions(+), 9 deletions(-) diff --git a/tests/test_generate.py b/tests/test_generate.py index 4a4cd2f..028c46e 100644 --- a/tests/test_generate.py +++ b/tests/test_generate.py @@ -12,7 +12,7 @@ # See the License for the specific language governing permissions and # limitations under the License. -import sys +import os, sys from pathlib import Path tests_dir_path = Path(__file__).parent @@ -26,22 +26,35 @@ from metashade.util.tests import RefDiffer import generate -ref_differ = RefDiffer( - ref_dir = repo_root_dir_path / 'tests' / 'ref', - out_dir_env_var = 'METASHADE_GLTFSAMPLE_PYTEST_OUT_DIR' -) - class TestGenerate: + @classmethod + def setup_class(cls): + out_dir = os.getenv('METASHADE_GLTFSAMPLE_PYTEST_OUT_DIR', None) + ref_dir = repo_root_dir_path / 'tests' / 'ref' / 'content' + + if out_dir is None: + # Don't compare against references explicitly in the script. + # Instead, overwrite the references with the generated files. + # This is useful for diffing or updating the references manually with + # git. + cls._out_dir = ref_dir + cls._ref_differ = None + else: + cls._out_dir = Path(out_dir).resolve() + cls._ref_differ = RefDiffer(ref_dir) + + os.makedirs(cls._out_dir, exist_ok = True) + def test_generate(self): gltf_sample_dir_path = repo_root_dir_path / 'glTFSample' gltf_dir_path = gltf_sample_dir_path / 'media' / 'Cauldron-Media' - out_dir_path = gltf_sample_dir_path / 'build' / 'DX12' / 'metashade-out' generate.generate( gltf_dir_path = gltf_dir_path, - out_dir_path = out_dir_path, + out_dir_path = self._out_dir, compile = True, to_glsl = False, skip_codegen = False, - serial = False + serial = False, + ref_differ = self._ref_differ ) From 50d0d6cf3d6206bbc1e0c887b9fc21682d9b985d Mon Sep 17 00:00:00 2001 From: Pavlo Penenko Date: Sat, 21 Dec 2024 15:55:11 -0500 Subject: [PATCH 05/10] Bump submodules --- metashade | 2 +- tests/ref | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/metashade b/metashade index 1427a99..d37a8e7 160000 --- a/metashade +++ b/metashade @@ -1 +1 @@ -Subproject commit 1427a99527dfd2485bec14e92ffd7645f82696e8 +Subproject commit d37a8e7c6d1401c3fb6b5f997873ca9184d7b2d8 diff --git a/tests/ref b/tests/ref index 46fae35..0e1bd77 160000 --- a/tests/ref +++ b/tests/ref @@ -1 +1 @@ -Subproject commit 46fae35c79b5b76617f034f6c338ce2f0bbbf2dc +Subproject commit 0e1bd776e2df6ef010b452c7f02f4ddbf27164bd From 8116a22b7d7a650b41a70b88f200bb962da67e11 Mon Sep 17 00:00:00 2001 From: Pavlo Penenko Date: Sat, 21 Dec 2024 15:56:31 -0500 Subject: [PATCH 06/10] Bump the refs submodule --- tests/ref | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/ref b/tests/ref index 0e1bd77..68269f4 160000 --- a/tests/ref +++ b/tests/ref @@ -1 +1 @@ -Subproject commit 0e1bd776e2df6ef010b452c7f02f4ddbf27164bd +Subproject commit 68269f4441ee5a36706353beed4cbbd9740ac47d From 98d032753ef19466a5015fb16c8baa9dcaabe618 Mon Sep 17 00:00:00 2001 From: Pavlo Penenko Date: Sat, 21 Dec 2024 16:06:10 -0500 Subject: [PATCH 07/10] Bump `metashade` to its latest `main` --- metashade | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/metashade b/metashade index d37a8e7..3acdadb 160000 --- a/metashade +++ b/metashade @@ -1 +1 @@ -Subproject commit d37a8e7c6d1401c3fb6b5f997873ca9184d7b2d8 +Subproject commit 3acdadba2a8c2ec0d07e22aca4ce9e7905d4f656 From edcdda4765a12054a577f61ae0dd36dcc1ebedc2 Mon Sep 17 00:00:00 2001 From: Pavlo Penenko Date: Sat, 21 Dec 2024 16:16:09 -0500 Subject: [PATCH 08/10] GHA: set `METASHADE_GLTFSAMPLE_PYTEST_OUT_DIR` --- .github/workflows/python-package.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/python-package.yml b/.github/workflows/python-package.yml index 33a9820..068d036 100644 --- a/.github/workflows/python-package.yml +++ b/.github/workflows/python-package.yml @@ -59,7 +59,7 @@ jobs: run: | Add-Content $env:GITHUB_PATH $env:GITHUB_WORKSPACE\.dxc\bin\x64 Add-Content $env:GITHUB_PATH $env:GITHUB_WORKSPACE\.glslang\bin - Add-Content $env:GITHUB_ENV METASHADE_PYTEST_OUT_DIR=$env:GITHUB_WORKSPACE/tests/out + Add-Content $env:GITHUB_ENV METASHADE_GLTFSAMPLE_PYTEST_OUT_DIR=$env:GITHUB_WORKSPACE/tests/out - name: Test with pytest run: | pytest -rP tests From 3fda34212a064084b6bf51178f0164f828dda153 Mon Sep 17 00:00:00 2001 From: Pavlo Penenko Date: Sat, 21 Dec 2024 16:16:33 -0500 Subject: [PATCH 09/10] Debug printouts for the test's reference and output directory paths --- tests/test_generate.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/test_generate.py b/tests/test_generate.py index 028c46e..5b29821 100644 --- a/tests/test_generate.py +++ b/tests/test_generate.py @@ -41,8 +41,10 @@ def setup_class(cls): cls._ref_differ = None else: cls._out_dir = Path(out_dir).resolve() + print(f'Reference directory: {ref_dir}') cls._ref_differ = RefDiffer(ref_dir) + print(f'Test output directory: {cls._out_dir}') os.makedirs(cls._out_dir, exist_ok = True) def test_generate(self): From fef68d940b110e7ec730d9951e72badd04aacd62 Mon Sep 17 00:00:00 2001 From: Pavlo Penenko Date: Sat, 21 Dec 2024 16:23:37 -0500 Subject: [PATCH 10/10] Cosmetic changes to test `RefDiffer` --- src/_impl.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/_impl.py b/src/_impl.py index 269f123..39b6e45 100644 --- a/src/_impl.py +++ b/src/_impl.py @@ -537,6 +537,8 @@ def _sample_material_texture(texture_name : str): # Finally, the pixel shader entry point with sh.entry_point(entry_point_name, sh.PsOut)(psIn = sh.VsOut): + sh // "Test diffing in the test" + sh.Vw = (sh.g_cameraPw - sh.psIn.Pw).normalize() sh.Nw = sh.getNormal(psIn = sh.psIn)