From 162526287e8674cacad373a95f5c0ffb6159e3d8 Mon Sep 17 00:00:00 2001 From: Frank Yellin Date: Thu, 19 Sep 2024 08:29:29 -0700 Subject: [PATCH 1/2] Fix some small errors --- tests/test_set_override.py | 16 ++++++--- tests/test_wgpu_vertex_instance.py | 57 +++++++++++++++++++----------- wgpu/backends/wgpu_native/_api.py | 6 ---- wgpu/resources/codegen_report.md | 4 +-- 4 files changed, 49 insertions(+), 34 deletions(-) diff --git a/tests/test_set_override.py b/tests/test_set_override.py index 8e0fc138..51aa65bc 100644 --- a/tests/test_set_override.py +++ b/tests/test_set_override.py @@ -2,7 +2,7 @@ import wgpu.utils from tests.testutils import can_use_wgpu_lib, run_tests -from wgpu import TextureFormat +from wgpu import GPUValidationError, TextureFormat if not can_use_wgpu_lib: pytest.skip("Skipping tests that need the wgpu lib", allow_module_level=True) @@ -197,10 +197,16 @@ def test_override_compute_constants(runner): def test_numbered_constants_must_be_overridden_by_number(runner): overrides = {"c": 23, "d": 24} - # This does absolutely nothing. It doesn't even error. - assert [1, 2, 3, 0, 1, 2, 3, 0] == runner.run_test( - render=True, vertex_constants=overrides, fragment_constants=overrides - ) + try: + # In naga, the bad constant is ignored. + # In the JS implementation, this throws an exception, which I think is the + # correct behavior. So just in case this ever gets fixed, we accept either. + result = runner.run_test( + render=True, vertex_constants=overrides, fragment_constants=overrides + ) + except GPUValidationError: + return + assert [1, 2, 3, 0, 1, 2, 3, 0] == result if __name__ == "__main__": diff --git a/tests/test_wgpu_vertex_instance.py b/tests/test_wgpu_vertex_instance.py index 95e0a369..ecda57dc 100644 --- a/tests/test_wgpu_vertex_instance.py +++ b/tests/test_wgpu_vertex_instance.py @@ -67,7 +67,8 @@ class Runner: - REQUIRED_FEATURES = ["multi-draw-indirect", "indirect-first-instance"] + REQUIRED_FEATURES = ["indirect-first-instance"] + OPTIONAL_FEATURES = ["multi-draw-indirect"] # we'll be adding more @classmethod def is_usable(cls): @@ -76,7 +77,11 @@ def is_usable(cls): def __init__(self): adapter = wgpu.gpu.request_adapter(power_preference="high-performance") - self.device = adapter.request_device(required_features=self.REQUIRED_FEATURES) + features = [ + *self.REQUIRED_FEATURES, + *[x for x in self.OPTIONAL_FEATURES if x in adapter.features], + ] + self.device = adapter.request_device(required_features=features) self.output_texture = self.device.create_texture( # Actual size is immaterial. Could just be 1x1 size=[128, 128], @@ -158,7 +163,8 @@ def __init__(self): # We're going to want to try calling these draw functions from a buffer, and it # would be nice to test that these buffers have an offset self.draw_data_buffer = self.device.create_buffer_with_data( - data=np.uint32([0, 0, *self.draw_args1, *self.draw_args2]), usage="INDIRECT" + data=np.uint32([0, 0, *self.draw_args1, *self.draw_args2]), + usage="INDIRECT", ) self.draw_data_buffer_indexed = self.device.create_buffer_with_data( data=np.uint32([0, 0, *self.draw_indexed_args1, *self.draw_indexed_args2]), @@ -181,7 +187,7 @@ def create_render_bundle_encoder(self, draw_function): draw_function(render_bundle_encoder) return render_bundle_encoder.finish() - def run_draw_test(self, expected_result, draw_function): + def run_draw_test(self, draw_function, indexed, *, expected_result=None): encoder = self.device.create_command_encoder() encoder.clear_buffer(self.counter_buffer) this_pass = encoder.begin_render_pass(**self.render_pass_descriptor) @@ -194,12 +200,17 @@ def run_draw_test(self, expected_result, draw_function): count = self.device.queue.read_buffer(self.counter_buffer).cast("i")[0] if count > MAX_INFO: pytest.fail("Too many data points written to output buffer") + if count == 0: + pytest.fail("No data points written to output buffer") # Get the result as a series of tuples info_view = self.device.queue.read_buffer(self.data_buffer, size=count * 2 * 4) - info = np.frombuffer(info_view, dtype=np.uint32).reshape(-1, 2) - info = [tuple(info[i]) for i in range(len(info))] - info_set = set(info) - assert len(info) == len(info_set) + info = info_view.cast("I", (count, 2)).tolist() + info_set = set(tuple(x) for x in info) + if expected_result is None: + if indexed: + expected_result = self.expected_result_draw_indexed + else: + expected_result = self.expected_result_draw assert info_set == expected_result @@ -217,7 +228,7 @@ def draw(encoder): encoder.draw(*runner.draw_args1) encoder.draw(*runner.draw_args2) - runner.run_draw_test(runner.expected_result_draw, draw) + runner.run_draw_test(draw, False) def test_draw_indirect(runner): @@ -225,7 +236,7 @@ def draw(encoder): encoder.draw_indirect(runner.draw_data_buffer, 8) encoder.draw_indirect(runner.draw_data_buffer, 8 + 16) - runner.run_draw_test(runner.expected_result_draw, draw) + runner.run_draw_test(draw, False) def test_draw_mixed(runner): @@ -233,14 +244,17 @@ def draw(encoder): encoder.draw(*runner.draw_args1) encoder.draw_indirect(runner.draw_data_buffer, 8 + 16) - runner.run_draw_test(runner.expected_result_draw, draw) + runner.run_draw_test(draw, False) def test_multi_draw_indirect(runner): + if "multi-draw-indirect" not in runner.device.features: + pytest.skip("Must have 'multi-draw-indirect' to run") + def draw(encoder): multi_draw_indirect(encoder, runner.draw_data_buffer, offset=8, count=2) - runner.run_draw_test(runner.expected_result_draw, draw) + runner.run_draw_test(draw, False) def test_draw_via_encoder(runner): @@ -252,8 +266,7 @@ def draw(encoder): for _ in range(2): # We run this test twice to verify that encoders are reusable. runner.run_draw_test( - runner.expected_result_draw, - lambda encoder: encoder.execute_bundles([render_bundle_encoder]), + lambda encoder: encoder.execute_bundles([render_bundle_encoder]), False ) @@ -269,10 +282,10 @@ def draw2(encoder): render_bundle_encoder2 = runner.create_render_bundle_encoder(draw2) runner.run_draw_test( - runner.expected_result_draw, lambda encoder: encoder.execute_bundles( [render_bundle_encoder1, render_bundle_encoder2] ), + False, ) @@ -281,7 +294,7 @@ def draw(encoder): encoder.draw_indexed(*runner.draw_indexed_args1) encoder.draw_indexed(*runner.draw_indexed_args2) - runner.run_draw_test(runner.expected_result_draw_indexed, draw) + runner.run_draw_test(draw, True) def test_draw_indexed_indirect(runner): @@ -289,7 +302,7 @@ def draw(encoder): encoder.draw_indexed_indirect(runner.draw_data_buffer_indexed, 8) encoder.draw_indexed_indirect(runner.draw_data_buffer_indexed, 8 + 20) - runner.run_draw_test(runner.expected_result_draw_indexed, draw) + runner.run_draw_test(draw, True) def test_draw_indexed_mixed(runner): @@ -297,16 +310,19 @@ def draw(encoder): encoder.draw_indexed_indirect(runner.draw_data_buffer_indexed, 8) encoder.draw_indexed(*runner.draw_indexed_args2) - runner.run_draw_test(runner.expected_result_draw_indexed, draw) + runner.run_draw_test(draw, True) def test_multi_draw_indexed_indirect(runner): + if "multi-draw-indirect" not in runner.device.features: + pytest.skip("Must have 'multi-draw-indirect' to run") + def draw(encoder): multi_draw_indexed_indirect( encoder, runner.draw_data_buffer_indexed, offset=8, count=2 ) - runner.run_draw_test(runner.expected_result_draw_indexed, draw) + runner.run_draw_test(draw, True) def test_draw_indexed_via_encoder(runner): @@ -317,8 +333,7 @@ def draw(encoder): render_bundle_encoder = runner.create_render_bundle_encoder(draw) for _ in range(2): runner.run_draw_test( - runner.expected_result_draw_indexed, - lambda encoder: encoder.execute_bundles([render_bundle_encoder]), + lambda encoder: encoder.execute_bundles([render_bundle_encoder]), True ) diff --git a/wgpu/backends/wgpu_native/_api.py b/wgpu/backends/wgpu_native/_api.py index d1a989d0..621dd54c 100644 --- a/wgpu/backends/wgpu_native/_api.py +++ b/wgpu/backends/wgpu_native/_api.py @@ -3031,12 +3031,6 @@ def _multi_draw_indexed_indirect(self, buffer, offset, count): self._internal, buffer._internal, int(offset), int(count) ) - def _release(self): - if self._internal is not None and libf is not None: - self._internal, internal = None, self._internal - # H: void f(WGPURenderPassEncoder renderPassEncoder) - libf.wgpuRenderPassEncoderRelease(internal) - class GPURenderBundleEncoder( classes.GPURenderBundleEncoder, diff --git a/wgpu/resources/codegen_report.md b/wgpu/resources/codegen_report.md index 23ec4b46..4f9a16d2 100644 --- a/wgpu/resources/codegen_report.md +++ b/wgpu/resources/codegen_report.md @@ -20,7 +20,7 @@ * Diffs for GPUQueue: add read_buffer, add read_texture, hide copy_external_image_to_texture * Validated 37 classes, 112 methods, 45 properties ### Patching API for backends/wgpu_native/_api.py -* Validated 37 classes, 101 methods, 0 properties +* Validated 37 classes, 100 methods, 0 properties ## Validating backends/wgpu_native/_api.py * Enum field FeatureName.texture-compression-bc-sliced-3d missing in wgpu.h * Enum field FeatureName.clip-distances missing in wgpu.h @@ -35,6 +35,6 @@ * Enum CanvasAlphaMode missing in wgpu.h * Enum CanvasToneMappingMode missing in wgpu.h * Wrote 236 enum mappings and 47 struct-field mappings to wgpu_native/_mappings.py -* Validated 134 C function calls +* Validated 133 C function calls * Not using 70 C functions * Validated 81 C structs From 5173abda659181930f267241d88e30d390266401 Mon Sep 17 00:00:00 2001 From: Frank Yellin Date: Thu, 19 Sep 2024 12:17:12 -0700 Subject: [PATCH 2/2] Add multi_draw_indirect_count --- tests/test_set_override.py | 24 +++++++++++++----------- 1 file changed, 13 insertions(+), 11 deletions(-) diff --git a/tests/test_set_override.py b/tests/test_set_override.py index 51aa65bc..10ae6799 100644 --- a/tests/test_set_override.py +++ b/tests/test_set_override.py @@ -2,7 +2,7 @@ import wgpu.utils from tests.testutils import can_use_wgpu_lib, run_tests -from wgpu import GPUValidationError, TextureFormat +from wgpu import TextureFormat if not can_use_wgpu_lib: pytest.skip("Skipping tests that need the wgpu lib", allow_module_level=True) @@ -196,17 +196,19 @@ def test_override_compute_constants(runner): def test_numbered_constants_must_be_overridden_by_number(runner): + # Note. The naga implementation ignores unknown constants passed as an override. + # The JS implementation throws an exception. The JS implementation matches the + # specification, + # + # We will test for current naga behavior, but if this test fails in the future, + # it should be replaced with something like the following: + # + # with pytest.raises(GPUValidationError): + # runner.run_test(....) overrides = {"c": 23, "d": 24} - try: - # In naga, the bad constant is ignored. - # In the JS implementation, this throws an exception, which I think is the - # correct behavior. So just in case this ever gets fixed, we accept either. - result = runner.run_test( - render=True, vertex_constants=overrides, fragment_constants=overrides - ) - except GPUValidationError: - return - assert [1, 2, 3, 0, 1, 2, 3, 0] == result + assert [1, 2, 3, 0, 1, 2, 3, 0] == runner.run_test( + render=True, vertex_constants=overrides, fragment_constants=overrides + ) if __name__ == "__main__":