From 7526933016c70b73ae7598abc8f3ba1a52f35ac1 Mon Sep 17 00:00:00 2001 From: Frank Yellin Date: Wed, 2 Oct 2024 20:21:36 -0700 Subject: [PATCH 1/7] Make depth_stencil_attachment follow the spec --- tests/test_wgpu_native_render.py | 22 +++----- tests/test_wgpu_occlusion_query.py | 3 -- wgpu/backends/wgpu_native/_api.py | 81 ++++++++++++++++++++++++------ wgpu/resources/codegen_report.md | 2 +- 4 files changed, 74 insertions(+), 34 deletions(-) diff --git a/tests/test_wgpu_native_render.py b/tests/test_wgpu_native_render.py index 9c6988e0..1afc96c6 100644 --- a/tests/test_wgpu_native_render.py +++ b/tests/test_wgpu_native_render.py @@ -557,18 +557,6 @@ def cb(renderpass): format=depth_stencil_tex_format, depth_write_enabled=True, depth_compare=wgpu.CompareFunction.less_equal, - # stencil_front={ - # "compare": wgpu.CompareFunction.equal, - # "fail_op": wgpu.StencilOperation.keep, - # "depth_fail_op": wgpu.StencilOperation.keep, - # "pass_op": wgpu.StencilOperation.keep, - # }, - # stencil_back={ - # "compare": wgpu.CompareFunction.equal, - # "fail_op": wgpu.StencilOperation.keep, - # "depth_fail_op": wgpu.StencilOperation.keep, - # "pass_op": wgpu.StencilOperation.keep, - # }, stencil_read_mask=0, stencil_write_mask=0, depth_bias=0, @@ -581,10 +569,16 @@ def cb(renderpass): depth_clear_value=0.1, depth_load_op=wgpu.LoadOp.clear, depth_store_op=wgpu.StoreOp.store, - stencil_load_op=wgpu.LoadOp.load, - stencil_store_op=wgpu.StoreOp.store, ) + if "stencil" in depth_stencil_tex_format: + depth_stencil_attachment["stencil_load_op"] = wgpu.LoadOp.load + depth_stencil_attachment["stencil_store_op"] = wgpu.StoreOp.store + + if "stencil" in depth_stencil_tex_format: + depth_stencil_attachment["stencil_load_op"] = wgpu.LoadOp.load + depth_stencil_attachment["stencil_store_op"] = wgpu.StoreOp.store + # Render render_args = device, shader_source, pipeline_layout, bind_group # render_to_screen(*render_args, renderpass_callback=cb, depth_stencil_state=depth_stencil_state, depth_stencil_attachment=depth_stencil_attachment) diff --git a/tests/test_wgpu_occlusion_query.py b/tests/test_wgpu_occlusion_query.py index 734b4805..6ad29ed8 100644 --- a/tests/test_wgpu_occlusion_query.py +++ b/tests/test_wgpu_occlusion_query.py @@ -141,9 +141,6 @@ def draw_square(result, x_offset=0.0, y_offset=0.0, z=0.5, reverse=False): "depth_clear_value": 1.0, "depth_load_op": "clear", "depth_store_op": "store", - "stencil_clear_value": 1.0, - "stencil_load_op": "clear", - "stencil_store_op": "store", } render_pass = command_encoder.begin_render_pass( diff --git a/wgpu/backends/wgpu_native/_api.py b/wgpu/backends/wgpu_native/_api.py index 8e7f7279..2e6a8f89 100644 --- a/wgpu/backends/wgpu_native/_api.py +++ b/wgpu/backends/wgpu_native/_api.py @@ -21,6 +21,7 @@ import ctypes import logging import ctypes.util +import warnings from weakref import WeakKeyDictionary from typing import List, Dict, Union, Optional @@ -2694,22 +2695,10 @@ def begin_render_pass( c_depth_stencil_attachment = ffi.NULL if depth_stencil_attachment is not None: check_struct("RenderPassDepthStencilAttachment", depth_stencil_attachment) - depth_clear_value = depth_stencil_attachment.get("depth_clear_value", 0) - stencil_clear_value = depth_stencil_attachment.get("stencil_clear_value", 0) - # H: view: WGPUTextureView, depthLoadOp: WGPULoadOp, depthStoreOp: WGPUStoreOp, depthClearValue: float, depthReadOnly: WGPUBool/int, stencilLoadOp: WGPULoadOp, stencilStoreOp: WGPUStoreOp, stencilClearValue: int, stencilReadOnly: WGPUBool/int - c_depth_stencil_attachment = new_struct_p( - "WGPURenderPassDepthStencilAttachment *", - view=depth_stencil_attachment["view"]._internal, - depthLoadOp=depth_stencil_attachment["depth_load_op"], - depthStoreOp=depth_stencil_attachment["depth_store_op"], - depthClearValue=float(depth_clear_value), - depthReadOnly=depth_stencil_attachment.get("depth_read_only", False), - stencilLoadOp=depth_stencil_attachment["stencil_load_op"], - stencilStoreOp=depth_stencil_attachment["stencil_store_op"], - stencilClearValue=int(stencil_clear_value), - stencilReadOnly=depth_stencil_attachment.get( - "stencil_read_only", False - ), + c_depth_stencil_attachment = ( + self._create_c_render_pass_depth_stencil_attachment( + depth_stencil_attachment + ) ) c_occlusion_query_set = ffi.NULL @@ -2735,6 +2724,66 @@ def begin_render_pass( encoder._objects_to_keep_alive = objects_to_keep_alive return encoder + # Pulled out from create_render_pass because it was too large. + @staticmethod + def _create_c_render_pass_depth_stencil_attachment(ds_attachment): + view = ds_attachment["view"] + depth_read_only = stencil_read_only = False + depth_load_op = depth_store_op = stencil_load_op = stencil_store_op = 0 + depth_clear_value = stencil_clear_value = 0 + depth_keys_okay = stencil_keys_okay = False + # All depth texture formats have "depth" in their name. + if "depth" in view.texture.format: + depth_read_only = ds_attachment.get("depth_read_only", False) + if not depth_read_only: + depth_keys_okay = True + depth_load_op = ds_attachment["depth_load_op"] + depth_store_op = ds_attachment["depth_store_op"] + if depth_load_op == "clear": + depth_clear_value = ds_attachment["depth_clear_value"] + # All stencil texture formats all have "stencil" in their name + if "stencil" in view.texture.format: + stencil_read_only = ds_attachment.get("stencil_read_only", False) + if not stencil_read_only: + stencil_keys_okay = True + stencil_load_op = ds_attachment["stencil_load_op"] + stencil_store_op = ds_attachment["stencil_store_op"] + # We only need this if load_op == "clear". It has a default value. + stencil_clear_value = ds_attachment.get("stencil_clear_value", 0) + + # By the spec, we shouldn't allow "depth_load_op" or "depth_store_op" + # unless we have a non-read-only depth format. Likewise, "stencil_load_op" + # and "stencil_store_op" aren't allowed unless we have a non-read-only stencil + # format. + # But until now, they were required, even if not needed. So let's make it a + # warning for now, with the possibility of making it an error in the future. + unexpected_keys = [ + *(("depth_load_op", "depth_store_op") if not depth_keys_okay else ()), + *(("stencil_load_op", "stencil_store_op") if not stencil_keys_okay else ()), + ] + for key in unexpected_keys: + if key in ds_attachment: + warnings.warn( + f"Unexpected key {key} in depth_stencil_attachment", + DeprecationWarning, + stacklevel=2, + ) + + # H: view: WGPUTextureView, depthLoadOp: WGPULoadOp, depthStoreOp: WGPUStoreOp, depthClearValue: float, depthReadOnly: WGPUBool/int, stencilLoadOp: WGPULoadOp, stencilStoreOp: WGPUStoreOp, stencilClearValue: int, stencilReadOnly: WGPUBool/int + c_depth_stencil_attachment = new_struct_p( + "WGPURenderPassDepthStencilAttachment *", + view=view._internal, + depthLoadOp=depth_load_op, + depthStoreOp=depth_store_op, + depthClearValue=float(depth_clear_value), + depthReadOnly=depth_read_only, + stencilLoadOp=stencil_load_op, + stencilStoreOp=stencil_store_op, + stencilClearValue=int(stencil_clear_value), + stencilReadOnly=stencil_read_only, + ) + return c_depth_stencil_attachment + def clear_buffer( self, buffer: GPUBuffer, offset: int = 0, size: Optional[int] = None ): diff --git a/wgpu/resources/codegen_report.md b/wgpu/resources/codegen_report.md index 35cc493c..b49feff6 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, 124 methods, 46 properties ### Patching API for backends/wgpu_native/_api.py -* Validated 37 classes, 112 methods, 0 properties +* Validated 37 classes, 113 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 From 2996745fd56ecb4ae96e4d8e423756862b948e7f Mon Sep 17 00:00:00 2001 From: Frank Yellin Date: Fri, 4 Oct 2024 17:10:41 -0700 Subject: [PATCH 2/7] Use wgpu.logger.warning() Simplify test_wgpu_native_render.py Add test that warnings occur, but only once. --- tests/test_wgpu_native_render.py | 180 +++++++++++++----------------- wgpu/backends/wgpu_native/_api.py | 20 ++-- 2 files changed, 87 insertions(+), 113 deletions(-) diff --git a/tests/test_wgpu_native_render.py b/tests/test_wgpu_native_render.py index 1afc96c6..8c439f9d 100644 --- a/tests/test_wgpu_native_render.py +++ b/tests/test_wgpu_native_render.py @@ -3,6 +3,7 @@ """ import ctypes + import numpy as np import sys @@ -21,7 +22,7 @@ skip("These tests fail on dx12 for some reason", allow_module_level=True) -default_vertex_shader = """ +DEFAULT_SHADER = """ @vertex fn vs_main(@builtin(vertex_index) vertex_index : u32) -> @builtin(position) vec4 { var positions: array, 4> = array, 4>( @@ -33,8 +34,18 @@ let p: vec3 = positions[vertex_index]; return vec4(p, 1.0); } + +@fragment +fn fs_main() -> @location(0) vec4 { + return vec4(1.0, 0.499, 0.0, 1.0); +} """ +DEPTH_STENCIL_TEX_FORMATS = [ + wgpu.TextureFormat.depth16unorm, + wgpu.TextureFormat.depth24plus_stencil8, + wgpu.TextureFormat.depth32float, +] # %% Simple square @@ -48,20 +59,12 @@ def test_render_orange_square(use_render_bundle): # NOTE: the 0.499 instead of 0.5 is to make sure the resulting value is 127. # With 0.5 some drivers would produce 127 and others 128. - fragment_shader = """ - @fragment - fn fs_main() -> @location(0) vec4 { - return vec4(1.0, 0.499, 0.0, 1.0); - } - """ - shader_source = default_vertex_shader + fragment_shader - # Bindings and layout bind_group = None pipeline_layout = device.create_pipeline_layout(bind_group_layouts=[]) # Render - render_args = device, shader_source, pipeline_layout, bind_group + render_args = device, DEFAULT_SHADER, pipeline_layout, bind_group # render_to_screen(*render_args) a = render_to_texture( *render_args, size=(64, 64), use_render_bundle=use_render_bundle @@ -89,14 +92,6 @@ def test_render_orange_square_indexed(use_render_bundle): device = get_default_device() - fragment_shader = """ - @fragment - fn fs_main() -> @location(0) vec4 { - return vec4(1.0, 0.499, 0.0, 1.0); - } - """ - shader_source = default_vertex_shader + fragment_shader - # Bindings and layout bind_group = None pipeline_layout = device.create_pipeline_layout(bind_group_layouts=[]) @@ -109,7 +104,7 @@ def test_render_orange_square_indexed(use_render_bundle): ) # Render - render_args = device, shader_source, pipeline_layout, bind_group + render_args = device, DEFAULT_SHADER, pipeline_layout, bind_group # render_to_screen(*render_args, topology=wgpu.PrimitiveTopology.triangle_list, ibo=ibo) a = render_to_texture( *render_args, @@ -138,14 +133,6 @@ def test_render_orange_square_indirect(use_render_bundle): device = get_default_device() - fragment_shader = """ - @fragment - fn fs_main() -> @location(0) vec4 { - return vec4(1.0, 0.499, 0.0, 1.0); - } - """ - shader_source = default_vertex_shader + fragment_shader - # Bindings and layout bind_group = None pipeline_layout = device.create_pipeline_layout(bind_group_layouts=[]) @@ -158,7 +145,7 @@ def test_render_orange_square_indirect(use_render_bundle): ) # Render - render_args = device, shader_source, pipeline_layout, bind_group + render_args = device, DEFAULT_SHADER, pipeline_layout, bind_group # render_to_screen(*render_args, indirect_buffer=indirect_buffer) a = render_to_texture( *render_args, @@ -186,14 +173,6 @@ def test_render_orange_square_indexed_indirect(use_render_bundle): device = get_default_device() - fragment_shader = """ - @fragment - fn fs_main() -> @location(0) vec4 { - return vec4(1.0, 0.499, 0.0, 1.0); - } - """ - shader_source = default_vertex_shader + fragment_shader - # Bindings and layout bind_group = None pipeline_layout = device.create_pipeline_layout(bind_group_layouts=[]) @@ -213,7 +192,7 @@ def test_render_orange_square_indexed_indirect(use_render_bundle): ) # Render - render_args = device, shader_source, pipeline_layout, bind_group + render_args = device, DEFAULT_SHADER, pipeline_layout, bind_group # render_to_screen(*render_args, topology=wgpu.PrimitiveTopology.triangle_list, ibo=ibo, indirect_buffer=indirect_buffer) a = render_to_texture( *render_args, @@ -309,14 +288,6 @@ def test_render_orange_square_color_attachment1(use_render_bundle): device = get_default_device() - fragment_shader = """ - @fragment - fn fs_main() -> @location(0) vec4 { - return vec4(1.0, 0.499, 0.0, 1.0); - } - """ - shader_source = default_vertex_shader + fragment_shader - # Bindings and layout bind_group = None pipeline_layout = device.create_pipeline_layout(bind_group_layouts=[]) @@ -329,7 +300,7 @@ def test_render_orange_square_color_attachment1(use_render_bundle): } # Render - render_args = device, shader_source, pipeline_layout, bind_group + render_args = device, DEFAULT_SHADER, pipeline_layout, bind_group # render_to_screen(*render_args, color_attachment=ca) a = render_to_texture( *render_args, @@ -360,14 +331,6 @@ def test_render_orange_square_color_attachment2(use_render_bundle): device = get_default_device() - fragment_shader = """ - @fragment - fn fs_main() -> @location(0) vec4 { - return vec4(1.0, 0.499, 0.0, 1.0); - } - """ - shader_source = default_vertex_shader + fragment_shader - # Bindings and layout bind_group = None pipeline_layout = device.create_pipeline_layout(bind_group_layouts=[]) @@ -379,7 +342,7 @@ def test_render_orange_square_color_attachment2(use_render_bundle): } # Render - render_args = device, shader_source, pipeline_layout, bind_group + render_args = device, DEFAULT_SHADER, pipeline_layout, bind_group # render_to_screen(*render_args, color_attachment=ca) a = render_to_texture( *render_args, @@ -411,14 +374,6 @@ def test_render_orange_square_viewport(use_render_bundle): device = get_default_device() - fragment_shader = """ - @fragment - fn fs_main() -> @location(0) vec4 { - return vec4(1.0, 0.499, 0.0, 1.0); - } - """ - shader_source = default_vertex_shader + fragment_shader - def cb(renderpass): renderpass.set_viewport(10, 20, 32, 32, 0, 1) @@ -427,7 +382,7 @@ def cb(renderpass): pipeline_layout = device.create_pipeline_layout(bind_group_layouts=[]) # Render - render_args = device, shader_source, pipeline_layout, bind_group + render_args = device, DEFAULT_SHADER, pipeline_layout, bind_group # render_to_screen(*render_args, renderpass_callback=cb) a = render_to_texture( *render_args, @@ -455,14 +410,6 @@ def test_render_orange_square_scissor(use_render_bundle): device = get_default_device() - fragment_shader = """ - @fragment - fn fs_main() -> @location(0) vec4 { - return vec4(1.0, 0.499, 0.0, 1.0); - } - """ - shader_source = default_vertex_shader + fragment_shader - def cb(renderpass): renderpass.set_scissor_rect(0, 0, 32, 32) # Else set blend color. Does not change output, but covers the call. @@ -473,7 +420,7 @@ def cb(renderpass): pipeline_layout = device.create_pipeline_layout(bind_group_layouts=[]) # Render - render_args = device, shader_source, pipeline_layout, bind_group + render_args = device, DEFAULT_SHADER, pipeline_layout, bind_group # render_to_screen(*render_args, renderpass_callback=cb) a = render_to_texture( *render_args, @@ -496,26 +443,9 @@ def cb(renderpass): @pytest.mark.parametrize("use_render_bundle", [True, False]) -def test_render_orange_square_depth16unorm(use_render_bundle): +@pytest.mark.parametrize("depth_stencil_tex_format", DEPTH_STENCIL_TEX_FORMATS) +def test_render_orange_square_with_depth(depth_stencil_tex_format, use_render_bundle): """Render an orange square, but disable half of it using a depth test using 16 bits.""" - _render_orange_square_depth(wgpu.TextureFormat.depth16unorm, use_render_bundle) - - -@pytest.mark.parametrize("use_render_bundle", [True, False]) -def test_render_orange_square_depth24plus_stencil8(use_render_bundle): - """Render an orange square, but disable half of it using a depth test using 24 bits.""" - _render_orange_square_depth( - wgpu.TextureFormat.depth24plus_stencil8, use_render_bundle - ) - - -@pytest.mark.parametrize("use_render_bundle", [True, False]) -def test_render_orange_square_depth32float(use_render_bundle): - """Render an orange square, but disable half of it using a depth test using 32 bits.""" - _render_orange_square_depth(wgpu.TextureFormat.depth32float, use_render_bundle) - - -def _render_orange_square_depth(depth_stencil_tex_format, use_render_bundle): device = get_default_device() shader_source = """ @@ -544,7 +474,7 @@ def cb(renderpass): bind_group = None pipeline_layout = device.create_pipeline_layout(bind_group_layouts=[]) - # Create dept-stencil texture + # Create depth-stencil texture depth_stencil_texture = device.create_texture( size=(64, 64, 1), # when rendering to texture # size=(640, 480, 1), # when rendering to screen @@ -557,11 +487,6 @@ def cb(renderpass): format=depth_stencil_tex_format, depth_write_enabled=True, depth_compare=wgpu.CompareFunction.less_equal, - stencil_read_mask=0, - stencil_write_mask=0, - depth_bias=0, - depth_bias_slope_scale=0.0, - depth_bias_clamp=0.0, ) depth_stencil_attachment = dict( @@ -574,10 +499,7 @@ def cb(renderpass): if "stencil" in depth_stencil_tex_format: depth_stencil_attachment["stencil_load_op"] = wgpu.LoadOp.load depth_stencil_attachment["stencil_store_op"] = wgpu.StoreOp.store - - if "stencil" in depth_stencil_tex_format: - depth_stencil_attachment["stencil_load_op"] = wgpu.LoadOp.load - depth_stencil_attachment["stencil_store_op"] = wgpu.StoreOp.store + # The default values of depth_stencil_state are fine. # Render render_args = device, shader_source, pipeline_layout, bind_group @@ -675,5 +597,59 @@ def test_render_orange_dots(use_render_bundle): assert np.all(dot[:, :, 3] == 255) # alpha +@pytest.mark.parametrize("depth_stencil_tex_format", DEPTH_STENCIL_TEX_FORMATS) +def test_stencil_depth_warning(depth_stencil_tex_format, monkeypatch): + # Verify that when you use stencil_load_op or stencil_store_op, and there is + # no stencil, you'll get a warning. But only once. + + # The only way we can see warnings is by monkeypatching wgpu.logger.warning + warnings = [] + monkeypatch.setattr(wgpu.logger, "warning", lambda msg: warnings.append(msg)) + + # We need a new device for each run, since each device keeps track of which warnings + # it's issued. + adapter = wgpu.gpu.request_adapter_sync(power_preference="high-performance") + device = adapter.request_device_sync() + + # Create depth-stencil texture + depth_stencil_texture = device.create_texture( + size=(64, 64, 1), # when rendering to texture + format=depth_stencil_tex_format, + usage=wgpu.TextureUsage.RENDER_ATTACHMENT, + ) + + depth_stencil_attachment = dict( + view=depth_stencil_texture.create_view(), + depth_clear_value=0.1, + depth_load_op=wgpu.LoadOp.clear, + depth_store_op=wgpu.StoreOp.store, + stencil_load_op=wgpu.LoadOp.clear, + stencil_store_op=wgpu.StoreOp.store, + ) + + command_encoder = device.create_command_encoder() + # This call to begin_render_pass may create warnings. + render_pass = command_encoder.begin_render_pass( + color_attachments=[], depth_stencil_attachment=depth_stencil_attachment + ) + render_pass.end() + + if "stencil" in depth_stencil_tex_format: + assert not warnings + else: + assert len(warnings) == 2 + warnings.sort() + assert "stencil_load_op" in warnings[0] + assert "stencil_store_op" in warnings[1] + warnings.clear() + + # The warnings are issued only once. + render_pass = command_encoder.begin_render_pass( + color_attachments=[], depth_stencil_attachment=depth_stencil_attachment + ) + render_pass.end() + assert not warnings + + if __name__ == "__main__": run_tests(globals()) diff --git a/wgpu/backends/wgpu_native/_api.py b/wgpu/backends/wgpu_native/_api.py index 2e6a8f89..0beeb8c7 100644 --- a/wgpu/backends/wgpu_native/_api.py +++ b/wgpu/backends/wgpu_native/_api.py @@ -21,7 +21,6 @@ import ctypes import logging import ctypes.util -import warnings from weakref import WeakKeyDictionary from typing import List, Dict, Union, Optional @@ -1950,7 +1949,7 @@ def create_render_bundle_encoder( render_bundle_id = libf.wgpuDeviceCreateRenderBundleEncoder( self._internal, render_bundle_encoder_descriptor ) - return GPURenderBundleEncoder(label, render_bundle_id, self) + return GPURenderBundleEncoder(label, render_bundle_id, self._device) def create_query_set(self, *, label: str = "", type: enums.QueryType, count: int): return self._create_query_set(label, type, count, None) @@ -1995,7 +1994,7 @@ def _create_query_set(self, label, type, count, statistics): # H: WGPUQuerySet f(WGPUDevice device, WGPUQuerySetDescriptor const * descriptor) query_id = libf.wgpuDeviceCreateQuerySet(self._internal, query_set_descriptor) - return GPUQuerySet(label, query_id, self._internal, type, count) + return GPUQuerySet(label, query_id, self._device, type, count) def _get_lost_sync(self): check_can_use_sync_variants() @@ -2720,13 +2719,12 @@ def begin_render_pass( # H: WGPURenderPassEncoder f(WGPUCommandEncoder commandEncoder, WGPURenderPassDescriptor const * descriptor) raw_encoder = libf.wgpuCommandEncoderBeginRenderPass(self._internal, struct) - encoder = GPURenderPassEncoder(label, raw_encoder, self) + encoder = GPURenderPassEncoder(label, raw_encoder, self._device) encoder._objects_to_keep_alive = objects_to_keep_alive return encoder # Pulled out from create_render_pass because it was too large. - @staticmethod - def _create_c_render_pass_depth_stencil_attachment(ds_attachment): + def _create_c_render_pass_depth_stencil_attachment(self, ds_attachment): view = ds_attachment["view"] depth_read_only = stencil_read_only = False depth_load_op = depth_store_op = stencil_load_op = stencil_store_op = 0 @@ -2763,11 +2761,11 @@ def _create_c_render_pass_depth_stencil_attachment(ds_attachment): ] for key in unexpected_keys: if key in ds_attachment: - warnings.warn( - f"Unexpected key {key} in depth_stencil_attachment", - DeprecationWarning, - stacklevel=2, - ) + if not getattr(self._device, f"warned_about_{key}", False): + from wgpu import logger + + logger.warning(f"Unexpected key {key} in depth_stencil_attachment") + setattr(self._device, f"warned_about_{key}", True) # H: view: WGPUTextureView, depthLoadOp: WGPULoadOp, depthStoreOp: WGPUStoreOp, depthClearValue: float, depthReadOnly: WGPUBool/int, stencilLoadOp: WGPULoadOp, stencilStoreOp: WGPUStoreOp, stencilClearValue: int, stencilReadOnly: WGPUBool/int c_depth_stencil_attachment = new_struct_p( From 457b123825ced3c5dbbc3224eb0d164c2e3bdfa3 Mon Sep 17 00:00:00 2001 From: Frank Yellin Date: Fri, 4 Oct 2024 18:02:59 -0700 Subject: [PATCH 3/7] Was some confusion, and some objects had a Device in their _device field while others had a DeviceImpl. Made it uniformly a Device. Added test to ensure this was the case. Fixed some typos. --- tests/test_wgpu_native_query_set.py | 2 +- tests_mem/test_destroy.py | 4 ++-- tests_mem/testutils.py | 16 ++++++++++++++-- wgpu/backends/wgpu_native/_api.py | 8 ++++---- 4 files changed, 21 insertions(+), 9 deletions(-) diff --git a/tests/test_wgpu_native_query_set.py b/tests/test_wgpu_native_query_set.py index 00ed8fd8..982c019f 100644 --- a/tests/test_wgpu_native_query_set.py +++ b/tests/test_wgpu_native_query_set.py @@ -86,7 +86,7 @@ def test_query_set(): ) assert query_set.count == query_count assert query_set.type == query_type - assert query_set._device == device._internal + assert query_set._device == device assert query_set.label == query_label query_buf_size = 8 * query_set.count diff --git a/tests_mem/test_destroy.py b/tests_mem/test_destroy.py index 6af78aea..20d4a1bb 100644 --- a/tests_mem/test_destroy.py +++ b/tests_mem/test_destroy.py @@ -28,7 +28,7 @@ def test_destroy_device(n): for i in range(n): d = adapter.request_device_sync() d.destroy() - # NOTE: destroy is not yet implemented in wgpu-natice - this does not actually do anything yet + # NOTE: destroy is not yet implemented in wgpu-native - this does not actually do anything yet yield d @@ -38,7 +38,7 @@ def test_destroy_query_set(n): for i in range(n): qs = DEVICE.create_query_set(type=wgpu.QueryType.occlusion, count=2) qs.destroy() - # NOTE: destroy is not yet implemented in wgpu-natice - this does not actually do anything yet + # NOTE: destroy is not yet implemented in wgpu-native - this does not actually do anything yet yield qs diff --git a/tests_mem/testutils.py b/tests_mem/testutils.py index bcf007ff..1458a990 100644 --- a/tests_mem/testutils.py +++ b/tests_mem/testutils.py @@ -7,7 +7,7 @@ import psutil import wgpu from wgpu._diagnostics import int_repr - +from wgpu.backends.wgpu_native import GPUDevice, GPUObjectBase p = psutil.Process() @@ -211,10 +211,22 @@ def core_test_func(): assert len(objects) == n_objects # Test that all objects are of the same class. - # (this for-loop is a bit weird, but its to avoid leaking refs to objects) + # (this for-loop is a bit weird, but it's to avoid leaking refs to objects) cls = objects[0].__class__ assert all(isinstance(objects[i], cls) for i in range(len(objects))) + # Test that everything that's a subclass of GPUObjectBase is either a + # GPUDevice and has None it its _device field, or is something else and has + # a GPUDevice in the _device field. + if issubclass(cls, GPUObjectBase): + if issubclass(cls, GPUDevice): + assert all(objects[i]._device is None for i in range(len(objects))) + else: + assert all( + isinstance(objects[i]._device, GPUDevice) + for i in range(len(objects)) + ) + # Test that class matches function name (should prevent a group of copy-paste errors) assert ob_name == cls.__name__[3:] diff --git a/wgpu/backends/wgpu_native/_api.py b/wgpu/backends/wgpu_native/_api.py index 0beeb8c7..6bee5d12 100644 --- a/wgpu/backends/wgpu_native/_api.py +++ b/wgpu/backends/wgpu_native/_api.py @@ -1946,10 +1946,10 @@ def create_render_bundle_encoder( # not used: nextInChain ) # H: WGPURenderBundleEncoder f(WGPUDevice device, WGPURenderBundleEncoderDescriptor const * descriptor) - render_bundle_id = libf.wgpuDeviceCreateRenderBundleEncoder( + render_bundle_encoder_id = libf.wgpuDeviceCreateRenderBundleEncoder( self._internal, render_bundle_encoder_descriptor ) - return GPURenderBundleEncoder(label, render_bundle_id, self._device) + return GPURenderBundleEncoder(label, render_bundle_encoder_id, self) def create_query_set(self, *, label: str = "", type: enums.QueryType, count: int): return self._create_query_set(label, type, count, None) @@ -1994,7 +1994,7 @@ def _create_query_set(self, label, type, count, statistics): # H: WGPUQuerySet f(WGPUDevice device, WGPUQuerySetDescriptor const * descriptor) query_id = libf.wgpuDeviceCreateQuerySet(self._internal, query_set_descriptor) - return GPUQuerySet(label, query_id, self._device, type, count) + return GPUQuerySet(label, query_id, self, type, count) def _get_lost_sync(self): check_can_use_sync_variants() @@ -2619,7 +2619,7 @@ def begin_compute_pass( ) # H: WGPUComputePassEncoder f(WGPUCommandEncoder commandEncoder, WGPUComputePassDescriptor const * descriptor) raw_encoder = libf.wgpuCommandEncoderBeginComputePass(self._internal, struct) - encoder = GPUComputePassEncoder(label, raw_encoder, self) + encoder = GPUComputePassEncoder(label, raw_encoder, self._device) return encoder def begin_render_pass( From 078a672d603a53d68d1603a8acfbfabf7decf5bb Mon Sep 17 00:00:00 2001 From: Frank Yellin Date: Sat, 5 Oct 2024 08:55:45 -0700 Subject: [PATCH 4/7] Simplify. Every subclass of GPUObjectBase has a device in its _device field. --- tests_mem/testutils.py | 16 ++++++---------- wgpu/_classes.py | 2 +- 2 files changed, 7 insertions(+), 11 deletions(-) diff --git a/tests_mem/testutils.py b/tests_mem/testutils.py index 1458a990..ee3f2b46 100644 --- a/tests_mem/testutils.py +++ b/tests_mem/testutils.py @@ -215,17 +215,13 @@ def core_test_func(): cls = objects[0].__class__ assert all(isinstance(objects[i], cls) for i in range(len(objects))) - # Test that everything that's a subclass of GPUObjectBase is either a - # GPUDevice and has None it its _device field, or is something else and has - # a GPUDevice in the _device field. + # Test that everything that's a subclass of GPUObjectBase has a device + # in its _device field. if issubclass(cls, GPUObjectBase): - if issubclass(cls, GPUDevice): - assert all(objects[i]._device is None for i in range(len(objects))) - else: - assert all( - isinstance(objects[i]._device, GPUDevice) - for i in range(len(objects)) - ) + assert all( + isinstance(objects[i]._device, GPUDevice) + for i in range(len(objects)) + ) # Test that class matches function name (should prevent a group of copy-paste errors) assert ob_name == cls.__name__[3:] diff --git a/wgpu/_classes.py b/wgpu/_classes.py index f665f69c..9ca0a5ed 100644 --- a/wgpu/_classes.py +++ b/wgpu/_classes.py @@ -701,7 +701,7 @@ class GPUDevice(GPUObjectBase): """ def __init__(self, label, internal, adapter, features, limits, queue): - super().__init__(label, internal, None) + super().__init__(label, internal, self) assert isinstance(adapter, GPUAdapter) assert isinstance(features, set) From 653e31dca01d5fb25def8a7b8c81f942069ed44a Mon Sep 17 00:00:00 2001 From: Frank Yellin Date: Sun, 6 Oct 2024 00:12:40 -0700 Subject: [PATCH 5/7] Minor typos. --- wgpu/_classes.py | 46 +++++++++++++++---------------- wgpu/backends/wgpu_native/_api.py | 10 +++---- 2 files changed, 28 insertions(+), 28 deletions(-) diff --git a/wgpu/_classes.py b/wgpu/_classes.py index 9ca0a5ed..7d571bdd 100644 --- a/wgpu/_classes.py +++ b/wgpu/_classes.py @@ -73,8 +73,8 @@ object_tracker = diagnostics.object_counts.tracker # The 'optional' value is used as the default value for optional arguments in the following two cases: -# * The method accepts a descriptior that is optional, so we make all arguments (i.e. descriptor fields) optional, and this one does not have a default value. -# * In wgpu-py we decided that this argument should be optonal, even though it's currently not according to the WebGPU spec. +# * The method accepts a descriptor that is optional, so we make all arguments (i.e. descriptor fields) optional, and this one does not have a default value. +# * In wgpu-py we decided that this argument should be optional, even though it's currently not according to the WebGPU spec. optional = None @@ -618,7 +618,7 @@ async def request_device_async( """Request a `GPUDevice` from the adapter. Arguments: - label (str): A human readable label. Optional. + label (str): A human-readable label. Optional. required_features (list of str): the features (extensions) that you need. Default []. required_limits (dict): the various limits that you need. Default {}. default_queue (structs.QueueDescriptor): Descriptor for the default queue. Optional. @@ -800,7 +800,7 @@ def create_buffer( """Create a `GPUBuffer` object. Arguments: - label (str): A human readable label. Optional. + label (str): A human-readable label. Optional. size (int): The size of the buffer in bytes. usage (flags.BufferUsage): The ways in which this buffer will be used. mapped_at_creation (bool): Whether the buffer is initially mapped. @@ -815,7 +815,7 @@ def create_buffer_with_data(self, *, label="", data, usage: "flags.BufferUsage") writes the given data to it, and then unmaps the buffer. Arguments: - label (str): A human readable label. Optional. + label (str): A human-readable label. Optional. data: Any object supporting the Python buffer protocol (this includes bytes, bytearray, ctypes arrays, numpy arrays, etc.). usage (flags.BufferUsage): The ways in which this buffer will be used. @@ -858,7 +858,7 @@ def create_texture( """Create a `GPUTexture` object. Arguments: - label (str): A human readable label. Optional. + label (str): A human-readable label. Optional. size (tuple or dict): The texture size as a 3-tuple or a `structs.Extent3D`. mip_level_count (int): The number of mip leveles. Default 1. sample_count (int): The number of samples. Default 1. @@ -870,7 +870,7 @@ def create_texture( a performance penalty. See https://gpuweb.github.io/gpuweb/#texture-format-caps for a - list of available texture formats. Note that less formats are + list of available texture formats. Note that fewer formats are available for storage usage. """ raise NotImplementedError() @@ -894,7 +894,7 @@ def create_sampler( """Create a `GPUSampler` object. Samplers specify how a texture is sampled. Arguments: - label (str): A human readable label. Optional. + label (str): A human-readable label. Optional. address_mode_u (enums.AddressMode): What happens when sampling beyond the x edge. Default "clamp-to-edge". address_mode_v (enums.AddressMode): What happens when sampling beyond the y edge. @@ -923,7 +923,7 @@ def create_bind_group_layout( docs on bind groups for details. Arguments: - label (str): A human readable label. Optional. + label (str): A human-readable label. Optional. entries (list): A list of `structs.BindGroupLayoutEntry` dicts. Each contains either a `structs.BufferBindingLayout`, `structs.SamplerBindingLayout`, `structs.TextureBindingLayout`, @@ -962,7 +962,7 @@ def create_bind_group( `pass.set_bind_group()` to attach a group of resources. Arguments: - label (str): A human readable label. Optional. + label (str): A human-readable label. Optional. layout (GPUBindGroupLayout): The layout (abstract representation) for this bind group. entries (list): A list of `structs.BindGroupEntry` dicts. The ``resource`` field @@ -1002,7 +1002,7 @@ def create_pipeline_layout( used in `create_render_pipeline()` or `create_compute_pipeline()`. Arguments: - label (str): A human readable label. Optional. + label (str): A human-readable label. Optional. bind_group_layouts (list): A list of `GPUBindGroupLayout` objects. """ raise NotImplementedError() @@ -1022,7 +1022,7 @@ def create_shader_module( as well as GLSL (experimental). Arguments: - label (str): A human readable label. Optional. + label (str): A human-readable label. Optional. code (str | bytes): The shader code, as WGSL, GLSL or SpirV. For GLSL code, the label must be given and contain the word 'comp', 'vert' or 'frag'. For SpirV the code must be bytes. @@ -1041,7 +1041,7 @@ def create_compute_pipeline( """Create a `GPUComputePipeline` object. Arguments: - label (str): A human readable label. Optional. + label (str): A human-readable label. Optional. layout (GPUPipelineLayout): object created with `create_pipeline_layout()`. compute (structs.ProgrammableStage): Binds shader module and entrypoint. """ @@ -1075,7 +1075,7 @@ def create_render_pipeline( """Create a `GPURenderPipeline` object. Arguments: - label (str): A human readable label. Optional. + label (str): A human-readable label. Optional. layout (GPUPipelineLayout): The layout for the new pipeline. vertex (structs.VertexState): Describes the vertex shader entry point of the pipeline and its input buffer layouts. @@ -1228,7 +1228,7 @@ def create_command_encoder(self, *, label: str = ""): at once to the GPU. Arguments: - label (str): A human readable label. Optional. + label (str): A human-readable label. Optional. """ raise NotImplementedError() @@ -1250,7 +1250,7 @@ def create_render_bundle_encoder( performance by removing the overhead of repeating the commands. Arguments: - label (str): A human readable label. Optional. + label (str): A human-readable label. Optional. color_formats (list): A list of the `GPUTextureFormats` of the color attachments for this pass or bundle. depth_stencil_format (GPUTextureFormat): The format of the depth/stencil attachment for this pass or bundle. sample_count (int): The number of samples per pixel in the attachments for this pass or bundle. Default 1. @@ -1389,7 +1389,7 @@ async def map_async( def unmap(self): """Unmaps the buffer. - Unmaps the mapped range of the GPUBuffer and makes it's contents + Unmaps the mapped range of the GPUBuffer and makes its contents available for use by the GPU again. """ raise NotImplementedError() @@ -1574,7 +1574,7 @@ def create_view( same format and dimension as the texture. Arguments: - label (str): A human readable label. Optional. + label (str): A human-readable label. Optional. format (enums.TextureFormat): What channels it stores and how. dimension (enums.TextureViewDimension): The dimensionality of the texture view. aspect (enums.TextureAspect): Whether this view is used for depth, stencil, or all. @@ -1920,7 +1920,7 @@ def begin_compute_pass( `GPUComputePassEncoder` object. Arguments: - label (str): A human readable label. Optional. + label (str): A human-readable label. Optional. timestamp_writes: unused """ raise NotImplementedError() @@ -1940,7 +1940,7 @@ def begin_render_pass( `GPURenderPassEncoder` object. Arguments: - label (str): A human readable label. Optional. + label (str): A human-readable label. Optional. color_attachments (list): List of `structs.RenderPassColorAttachment` dicts. depth_stencil_attachment (structs.RenderPassDepthStencilAttachment): Describes the depth stencil attachment. Default None. occlusion_query_set (GPUQuerySet): Default None. @@ -2043,7 +2043,7 @@ def finish(self, *, label: str = ""): submit to a `GPUQueue`. Arguments: - label (str): A human readable label. Optional. + label (str): A human-readable label. Optional. """ raise NotImplementedError() @@ -2242,7 +2242,7 @@ def finish(self, *, label: str = ""): """Finish recording and return a `GPURenderBundle`. Arguments: - label (str): A human readable label. Optional. + label (str): A human-readable label. Optional. """ raise NotImplementedError() @@ -2609,7 +2609,7 @@ def proxy_method(self, *args, **kwargs): return getattr(self, name)(*args, **kwargs) proxy_method.__name__ = name + "_backwards_compat_proxy" - proxy_method.__doc__ = f"Backwards compatibile method for {name}()" + proxy_method.__doc__ = f"Backwards compatible method for {name}()" return proxy_method m = globals() diff --git a/wgpu/backends/wgpu_native/_api.py b/wgpu/backends/wgpu_native/_api.py index 6bee5d12..a0f9933c 100644 --- a/wgpu/backends/wgpu_native/_api.py +++ b/wgpu/backends/wgpu_native/_api.py @@ -43,7 +43,7 @@ logger = logging.getLogger("wgpu") -# The API is prettu well defined +# The API is pretty well defined __all__ = classes.__all__.copy() @@ -323,7 +323,7 @@ def request_adapter_sync( force_fallback_adapter: bool = False, canvas=None, ): - """Async version of ``request_adapter_async()``. + """Sync version of ``request_adapter_async()``. This is the implementation based on wgpu-native. """ check_can_use_sync_variants() @@ -753,7 +753,7 @@ def _create_texture_screen(self): # Or if this is the second attempt. raise RuntimeError(f"Cannot get surface texture ({status}).") - # I don't expect this to happen, but lets check just in case. + # I don't expect this to happen, but let's check just in case. if not texture_id: raise RuntimeError("Cannot get surface texture (no texture)") @@ -2109,7 +2109,7 @@ def callback(status, user_data): awaitable_result["result"] = status def poll_func(): - # Doing nothing here will result in timeouts. I.e. unlike request_device, this method is truely async. + # Doing nothing here will result in timeouts. I.e. unlike request_device, this method is truly async. # H: WGPUBool f(WGPUDevice device, WGPUBool wait, WGPUWrappedSubmissionIndex const * wrappedSubmissionIndex) libf.wgpuDevicePoll(self._device._internal, False, ffi.NULL) @@ -2467,7 +2467,7 @@ def set_bind_group( class GPUDebugCommandsMixin(classes.GPUDebugCommandsMixin): - # whole class is likely going to solved better: https://github.com/pygfx/wgpu-py/pull/546 + # whole class is likely going to be solved better: https://github.com/pygfx/wgpu-py/pull/546 def push_debug_group(self, group_label: str): c_group_label = to_c_string(group_label) # H: void wgpuCommandEncoderPushDebugGroup(WGPUCommandEncoder commandEncoder, char const * groupLabel) From 24a39a35173b852ecfe5a8b6bf8a20f0ec1d4e32 Mon Sep 17 00:00:00 2001 From: Frank Yellin Date: Mon, 7 Oct 2024 09:09:27 -0700 Subject: [PATCH 6/7] Change requested by @almarklein --- tests_mem/testutils.py | 11 +++++++---- wgpu/_classes.py | 2 +- 2 files changed, 8 insertions(+), 5 deletions(-) diff --git a/tests_mem/testutils.py b/tests_mem/testutils.py index 8683b2aa..069a338c 100644 --- a/tests_mem/testutils.py +++ b/tests_mem/testutils.py @@ -218,10 +218,13 @@ def core_test_func(): # Test that everything that's a subclass of GPUObjectBase has a device # in its _device field. if issubclass(cls, GPUObjectBase): - assert all( - isinstance(objects[i]._device, GPUDevice) - for i in range(len(objects)) - ) + if issubclass(cls, GPUDevice): + assert all(objects[i]._device is None for i in range(len(objects))) + else: + assert all( + isinstance(objects[i]._device, GPUDevice) + for i in range(len(objects)) + ) # Test that class matches function name (should prevent a group of copy-paste errors) assert ob_name == cls.__name__[3:] diff --git a/wgpu/_classes.py b/wgpu/_classes.py index 7d571bdd..bf9bceb8 100644 --- a/wgpu/_classes.py +++ b/wgpu/_classes.py @@ -701,7 +701,7 @@ class GPUDevice(GPUObjectBase): """ def __init__(self, label, internal, adapter, features, limits, queue): - super().__init__(label, internal, self) + super().__init__(label, internal, None) assert isinstance(adapter, GPUAdapter) assert isinstance(features, set) From 817a6093dea65a73f57d5361cebaae18195f61ff Mon Sep 17 00:00:00 2001 From: Frank Yellin Date: Tue, 8 Oct 2024 03:53:18 -0700 Subject: [PATCH 7/7] Revert change --- tests_mem/testutils.py | 11 ++++------- wgpu/_classes.py | 2 +- 2 files changed, 5 insertions(+), 8 deletions(-) diff --git a/tests_mem/testutils.py b/tests_mem/testutils.py index 069a338c..8683b2aa 100644 --- a/tests_mem/testutils.py +++ b/tests_mem/testutils.py @@ -218,13 +218,10 @@ def core_test_func(): # Test that everything that's a subclass of GPUObjectBase has a device # in its _device field. if issubclass(cls, GPUObjectBase): - if issubclass(cls, GPUDevice): - assert all(objects[i]._device is None for i in range(len(objects))) - else: - assert all( - isinstance(objects[i]._device, GPUDevice) - for i in range(len(objects)) - ) + assert all( + isinstance(objects[i]._device, GPUDevice) + for i in range(len(objects)) + ) # Test that class matches function name (should prevent a group of copy-paste errors) assert ob_name == cls.__name__[3:] diff --git a/wgpu/_classes.py b/wgpu/_classes.py index bf9bceb8..7d571bdd 100644 --- a/wgpu/_classes.py +++ b/wgpu/_classes.py @@ -701,7 +701,7 @@ class GPUDevice(GPUObjectBase): """ def __init__(self, label, internal, adapter, features, limits, queue): - super().__init__(label, internal, None) + super().__init__(label, internal, self) assert isinstance(adapter, GPUAdapter) assert isinstance(features, set)