From 6bb8bf6497a4dd4624f55f7b2263d9b8a5cceb30 Mon Sep 17 00:00:00 2001 From: fyellin Date: Wed, 16 Oct 2024 04:03:10 -0700 Subject: [PATCH] Structure checking (#615) * Test on Python 3.13 Remove ruff flakiness Better showing of deltas when codegen fails * Allow refactoring in _api.py. apipatcher know understands helper functions. * A few quick typos, while I'm at it. * Forgot to make corresponding changes to _classes.py * Forgot to make corresponding changes to _classes.py * Removed 3.8 Replaced ruff temporary file with stdin/stdout * Change requirements to >= 3.9 * Undo 3.8 => 3.9 change * Typo * Typo --- codegen/apipatcher.py | 67 ++++++- wgpu/_classes.py | 6 +- wgpu/backends/wgpu_native/_api.py | 314 +++++++++++++++--------------- wgpu/resources/codegen_report.md | 2 +- 4 files changed, 224 insertions(+), 165 deletions(-) diff --git a/codegen/apipatcher.py b/codegen/apipatcher.py index fc898a84..5449cbda 100644 --- a/codegen/apipatcher.py +++ b/codegen/apipatcher.py @@ -3,10 +3,13 @@ spec (IDL), and the backend implementations from the base API. """ -from codegen.utils import print, format_code, to_snake_case, to_camel_case, Patcher -from codegen.idlparser import get_idl_parser, Attribute -from codegen.files import file_cache +import ast +from collections import defaultdict +from functools import cache +from codegen.files import file_cache +from codegen.idlparser import Attribute, get_idl_parser +from codegen.utils import Patcher, format_code, print, to_camel_case, to_snake_case # In wgpu-py, we make some args optional, that are not optional in the # IDL. Reasons may be because it makes sense to be able to omit them, @@ -620,6 +623,8 @@ def apply(self, code): all_structs = set() ignore_structs = {"Extent3D", "Origin3D"} + structure_checks = self._get_structure_checks() + for classname, i1, i2 in self.iter_classes(): if classname not in idl.classes: continue @@ -639,12 +644,8 @@ def apply(self, code): method_structs.update(self._get_sub_structs(idl, structname)) all_structs.update(method_structs) # Collect structs being checked - checked = set() - for line in code.splitlines(): - line = line.lstrip() - if line.startswith("check_struct("): - name = line.split("(")[1].split(",")[0].strip('"') - checked.add(name) + checked = structure_checks[classname, methodname] + # Test that a matching check is done unchecked = method_structs.difference(checked) unchecked = list(sorted(unchecked.difference(ignore_structs))) @@ -674,3 +675,51 @@ def _get_sub_structs(self, idl, structname): if structname2 in idl.structs: structnames.update(self._get_sub_structs(idl, structname2)) return structnames + + @staticmethod + def _get_structure_checks(): + """ + Returns a map + (class_name, method_name) -> + mapping each top-level method in _api.py to the calls to check_struct made by + that method or by any helper methods called by that method. + + For now, the helper function must be methods within the same class. This code + does not yet deal with global functions or with methods in superclasses. + """ + module = ast.parse(file_cache.read("backends/wgpu_native/_api.py")) + # We only care about top-level classes and their top-level methods. + top_level_methods = { + # (class_name, method_name) -> method_ast + (class_ast.name, method_ast.name): method_ast + for class_ast in module.body + if isinstance(class_ast, ast.ClassDef) + for method_ast in class_ast.body + if isinstance(method_ast, (ast.FunctionDef, ast.AsyncFunctionDef)) + } + + # (class_name, method_name) -> list of helper methods + method_helper_calls = defaultdict(list) + # (class_name, method_name) -> list of structures checked + structure_checks = defaultdict(list) + + for key, method_ast in top_level_methods.items(): + for node in ast.walk(method_ast): + if isinstance(node, ast.Call): + name = ast.unparse(node.func) + if name.startswith("self._"): + method_helper_calls[key].append(name[5:]) + if name == "check_struct": + assert isinstance(node.args[0], ast.Constant) + struct_name = node.args[0].value + assert isinstance(struct_name, str) + structure_checks[key].append(struct_name) + + @cache + def get_function_checks(class_name, method_name): + result = set(structure_checks[class_name, method_name]) + for helper_method_name in method_helper_calls[class_name, method_name]: + result.update(get_function_checks(class_name, helper_method_name)) + return sorted(result) + + return {key: get_function_checks(*key) for key in top_level_methods.keys()} diff --git a/wgpu/_classes.py b/wgpu/_classes.py index 7d571bdd..db0b8d6c 100644 --- a/wgpu/_classes.py +++ b/wgpu/_classes.py @@ -92,7 +92,7 @@ class GPU: def request_adapter_sync( self, *, - power_preference: enums.GPUPowerPreference = None, + power_preference: enums.PowerPreference = None, force_fallback_adapter: bool = False, canvas=None, ): @@ -114,7 +114,7 @@ def request_adapter_sync( async def request_adapter_async( self, *, - power_preference: enums.GPUPowerPreference = None, + power_preference: enums.PowerPreference = None, force_fallback_adapter: bool = False, canvas=None, ): @@ -1079,7 +1079,7 @@ def create_render_pipeline( 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. - primitive (structs.PrimitiveState): Describes the the primitive-related properties + primitive (structs.PrimitiveState): Describes the primitive-related properties of the pipeline. If `strip_index_format` is present (which means the primitive topology is a strip), and the drawCall is indexed, the vertex index list is split into sub-lists using the maximum value of this diff --git a/wgpu/backends/wgpu_native/_api.py b/wgpu/backends/wgpu_native/_api.py index a0f9933c..d3dde5af 100644 --- a/wgpu/backends/wgpu_native/_api.py +++ b/wgpu/backends/wgpu_native/_api.py @@ -18,9 +18,7 @@ from __future__ import annotations import os -import ctypes import logging -import ctypes.util from weakref import WeakKeyDictionary from typing import List, Dict, Union, Optional @@ -319,7 +317,7 @@ class GPU(classes.GPU): def request_adapter_sync( self, *, - power_preference: enums.GPUPowerPreference = None, + power_preference: enums.PowerPreference = None, force_fallback_adapter: bool = False, canvas=None, ): @@ -338,7 +336,7 @@ def request_adapter_sync( async def request_adapter_async( self, *, - power_preference: enums.GPUPowerPreference = None, + power_preference: enums.PowerPreference = None, force_fallback_adapter: bool = False, canvas=None, ): @@ -1238,10 +1236,7 @@ def create_bind_group_layout( c_entries_list = [] for entry in entries: check_struct("BindGroupLayoutEntry", entry) - buffer = {} - sampler = {} - texture = {} - storage_texture = {} + buffer = sampler = texture = storage_texture = () if "buffer" in entry: # Note, it might be an empty dictionary info = entry["buffer"] check_struct("BufferBindingLayout", info) @@ -1475,7 +1470,7 @@ def create_shader_module( compilation_hints: List[structs.ShaderModuleCompilationHint] = [], ): if compilation_hints: - for hint in compilation_hints.values(): + for hint in compilation_hints: check_struct("ShaderModuleCompilationHint", hint) if isinstance(code, str): looks_like_wgsl = any( @@ -1695,28 +1690,10 @@ def _create_render_pipeline( check_struct("MultisampleState", multisample) check_struct("PrimitiveState", primitive) - c_vertex_buffer_layout_list = [] - for buffer_des in vertex.get("buffers", ()): - c_attributes_list = [] - for attribute in buffer_des["attributes"]: - # H: format: WGPUVertexFormat, offset: int, shaderLocation: int - c_attribute = new_struct( - "WGPUVertexAttribute", - format=attribute["format"], - offset=attribute["offset"], # this offset is required - shaderLocation=attribute["shader_location"], - ) - c_attributes_list.append(c_attribute) - c_attributes_array = ffi.new("WGPUVertexAttribute []", c_attributes_list) - # H: arrayStride: int, stepMode: WGPUVertexStepMode, attributeCount: int, attributes: WGPUVertexAttribute * - c_vertex_buffer_descriptor = new_struct( - "WGPUVertexBufferLayout", - arrayStride=buffer_des["array_stride"], - stepMode=buffer_des.get("step_mode", "vertex"), - attributes=c_attributes_array, - attributeCount=len(c_attributes_list), - ) - c_vertex_buffer_layout_list.append(c_vertex_buffer_descriptor) + c_vertex_buffer_layout_list = [ + self._create_vertex_buffer_layout(buffer_des) + for buffer_des in vertex.get("buffers", ()) + ] c_vertex_buffer_descriptors_array = ffi.new( "WGPUVertexBufferLayout []", c_vertex_buffer_layout_list ) @@ -1745,43 +1722,7 @@ def _create_render_pipeline( c_depth_stencil_state = ffi.NULL if depth_stencil: - if depth_stencil.get("format", None) is None: - raise ValueError("depth_stencil needs format") - stencil_front = depth_stencil.get("stencil_front", {}) - check_struct("StencilFaceState", stencil_front) - # H: compare: WGPUCompareFunction, failOp: WGPUStencilOperation, depthFailOp: WGPUStencilOperation, passOp: WGPUStencilOperation - c_stencil_front = new_struct( - "WGPUStencilFaceState", - compare=stencil_front.get("compare", "always"), - failOp=stencil_front.get("fail_op", "keep"), - depthFailOp=stencil_front.get("depth_fail_op", "keep"), - passOp=stencil_front.get("pass_op", "keep"), - ) - stencil_back = depth_stencil.get("stencil_back", {}) - check_struct("StencilFaceState", stencil_back) - # H: compare: WGPUCompareFunction, failOp: WGPUStencilOperation, depthFailOp: WGPUStencilOperation, passOp: WGPUStencilOperation - c_stencil_back = new_struct( - "WGPUStencilFaceState", - compare=stencil_back.get("compare", "always"), - failOp=stencil_back.get("fail_op", "keep"), - depthFailOp=stencil_back.get("depth_fail_op", "keep"), - passOp=stencil_back.get("pass_op", "keep"), - ) - # H: nextInChain: WGPUChainedStruct *, format: WGPUTextureFormat, depthWriteEnabled: WGPUBool/int, depthCompare: WGPUCompareFunction, stencilFront: WGPUStencilFaceState, stencilBack: WGPUStencilFaceState, stencilReadMask: int, stencilWriteMask: int, depthBias: int, depthBiasSlopeScale: float, depthBiasClamp: float - c_depth_stencil_state = new_struct_p( - "WGPUDepthStencilState *", - format=depth_stencil["format"], - depthWriteEnabled=bool(depth_stencil.get("depth_write_enabled", False)), - depthCompare=depth_stencil.get("depth_compare", "always"), - stencilFront=c_stencil_front, - stencilBack=c_stencil_back, - stencilReadMask=depth_stencil.get("stencil_read_mask", 0xFFFFFFFF), - stencilWriteMask=depth_stencil.get("stencil_write_mask", 0xFFFFFFFF), - depthBias=depth_stencil.get("depth_bias", 0), - depthBiasSlopeScale=depth_stencil.get("depth_bias_slope_scale", 0), - depthBiasClamp=depth_stencil.get("depth_bias_clamp", 0), - # not used: nextInChain - ) + c_depth_stencil_state = self._create_depth_stencil_state(depth_stencil) # H: nextInChain: WGPUChainedStruct *, count: int, mask: int, alphaToCoverageEnabled: WGPUBool/int c_multisample_state = new_struct( @@ -1794,39 +1735,10 @@ def _create_render_pipeline( c_fragment_state = ffi.NULL if fragment is not None: - c_color_targets_list = [] - for target in fragment["targets"]: - if not target.get("blend", None): - c_blend = ffi.NULL - else: - c_alpha_blend, c_color_blend = [ - # H: operation: WGPUBlendOperation, srcFactor: WGPUBlendFactor, dstFactor: WGPUBlendFactor - new_struct( - "WGPUBlendComponent", - srcFactor=blend.get("src_factor", "one"), - dstFactor=blend.get("dst_factor", "zero"), - operation=blend.get("operation", "add"), - ) - for blend in ( - target["blend"]["alpha"], - target["blend"]["color"], - ) - ] - # H: color: WGPUBlendComponent, alpha: WGPUBlendComponent - c_blend = new_struct_p( - "WGPUBlendState *", - color=c_color_blend, - alpha=c_alpha_blend, - ) - # H: nextInChain: WGPUChainedStruct *, format: WGPUTextureFormat, blend: WGPUBlendState *, writeMask: WGPUColorWriteMaskFlags/int - c_color_state = new_struct( - "WGPUColorTargetState", - format=target["format"], - blend=c_blend, - writeMask=target.get("write_mask", 0xF), - # not used: nextInChain - ) - c_color_targets_list.append(c_color_state) + c_color_targets_list = [ + self._create_color_target_state(target) + for target in fragment["targets"] + ] c_color_targets_array = ffi.new( "WGPUColorTargetState []", c_color_targets_list ) @@ -1905,6 +1817,101 @@ def finalizer(id): "create_render_pipeline", awaitable_result, callback, poll_func, finalizer ) + def _create_color_target_state(self, target): + if not target.get("blend", None): + c_blend = ffi.NULL + else: + c_alpha_blend, c_color_blend = [ + # H: operation: WGPUBlendOperation, srcFactor: WGPUBlendFactor, dstFactor: WGPUBlendFactor + new_struct( + "WGPUBlendComponent", + srcFactor=blend.get("src_factor", "one"), + dstFactor=blend.get("dst_factor", "zero"), + operation=blend.get("operation", "add"), + ) + for blend in ( + target["blend"]["alpha"], + target["blend"]["color"], + ) + ] + # H: color: WGPUBlendComponent, alpha: WGPUBlendComponent + c_blend = new_struct_p( + "WGPUBlendState *", + color=c_color_blend, + alpha=c_alpha_blend, + ) + # H: nextInChain: WGPUChainedStruct *, format: WGPUTextureFormat, blend: WGPUBlendState *, writeMask: WGPUColorWriteMaskFlags/int + c_color_state = new_struct( + "WGPUColorTargetState", + format=target["format"], + blend=c_blend, + writeMask=target.get("write_mask", 0xF), + # not used: nextInChain + ) + return c_color_state + + def _create_vertex_buffer_layout(self, buffer_des): + c_attributes_list = [] + for attribute in buffer_des["attributes"]: + # H: format: WGPUVertexFormat, offset: int, shaderLocation: int + c_attribute = new_struct( + "WGPUVertexAttribute", + format=attribute["format"], + offset=attribute["offset"], # this offset is required + shaderLocation=attribute["shader_location"], + ) + c_attributes_list.append(c_attribute) + c_attributes_array = ffi.new("WGPUVertexAttribute []", c_attributes_list) + # H: arrayStride: int, stepMode: WGPUVertexStepMode, attributeCount: int, attributes: WGPUVertexAttribute * + c_vertex_buffer_descriptor = new_struct( + "WGPUVertexBufferLayout", + arrayStride=buffer_des["array_stride"], + stepMode=buffer_des.get("step_mode", "vertex"), + attributes=c_attributes_array, + attributeCount=len(c_attributes_list), + ) + return c_vertex_buffer_descriptor + + def _create_depth_stencil_state(self, depth_stencil): + if depth_stencil.get("format", None) is None: + raise ValueError("depth_stencil needs format") + stencil_front = depth_stencil.get("stencil_front", {}) + check_struct("StencilFaceState", stencil_front) + # H: compare: WGPUCompareFunction, failOp: WGPUStencilOperation, depthFailOp: WGPUStencilOperation, passOp: WGPUStencilOperation + c_stencil_front = new_struct( + "WGPUStencilFaceState", + compare=stencil_front.get("compare", "always"), + failOp=stencil_front.get("fail_op", "keep"), + depthFailOp=stencil_front.get("depth_fail_op", "keep"), + passOp=stencil_front.get("pass_op", "keep"), + ) + stencil_back = depth_stencil.get("stencil_back", {}) + check_struct("StencilFaceState", stencil_back) + # H: compare: WGPUCompareFunction, failOp: WGPUStencilOperation, depthFailOp: WGPUStencilOperation, passOp: WGPUStencilOperation + c_stencil_back = new_struct( + "WGPUStencilFaceState", + compare=stencil_back.get("compare", "always"), + failOp=stencil_back.get("fail_op", "keep"), + depthFailOp=stencil_back.get("depth_fail_op", "keep"), + passOp=stencil_back.get("pass_op", "keep"), + ) + # H: nextInChain: WGPUChainedStruct *, format: WGPUTextureFormat, depthWriteEnabled: WGPUBool/int, depthCompare: WGPUCompareFunction, stencilFront: WGPUStencilFaceState, stencilBack: WGPUStencilFaceState, stencilReadMask: int, stencilWriteMask: int, depthBias: int, depthBiasSlopeScale: float, depthBiasClamp: float + c_depth_stencil_state = new_struct_p( + "WGPUDepthStencilState *", + format=depth_stencil["format"], + depthWriteEnabled=bool(depth_stencil.get("depth_write_enabled", False)), + depthCompare=depth_stencil.get("depth_compare", "always"), + stencilFront=c_stencil_front, + stencilBack=c_stencil_back, + stencilReadMask=depth_stencil.get("stencil_read_mask", 0xFFFFFFFF), + stencilWriteMask=depth_stencil.get("stencil_write_mask", 0xFFFFFFFF), + depthBias=depth_stencil.get("depth_bias", 0), + depthBiasSlopeScale=depth_stencil.get("depth_bias_slope_scale", 0), + depthBiasClamp=depth_stencil.get("depth_bias_clamp", 0), + # not used: nextInChain + ) + return c_depth_stencil_state + def create_command_encoder(self, *, label: str = ""): # H: nextInChain: WGPUChainedStruct *, label: char * struct = new_struct_p( @@ -2174,15 +2181,12 @@ def read_mapped(self, buffer_offset=None, size=None, *, copy=True): if copy: # Copy the data. The memoryview created above becomes invalid when the buffer - # is unmapped, so we don't want to pass that memory to the user. - data = memoryview((ctypes.c_uint8 * size)()).cast("B") - data[:] = src_m - return data + # is unmapped. bytearray() makes a copy of the data; memoryview() creates a + # view on the bytearray. + return memoryview(bytearray(src_m)).cast("B") else: - # Return view on the actually mapped data - data = src_m - if hasattr(data, "toreadonly"): # Py 3.8+ - data = data.toreadonly() + # Return view on the actual mapped data. + data = src_m.toreadonly() self._mapped_memoryviews.append(data) return data @@ -2649,44 +2653,12 @@ def begin_render_pass( objects_to_keep_alive = {} - c_color_attachments_list = [] - for color_attachment in color_attachments: - check_struct("RenderPassColorAttachment", color_attachment) - texture_view = color_attachment["view"] - if not isinstance(texture_view, GPUTextureView): - raise TypeError("Color attachment view must be a GPUTextureView.") - texture_view_id = texture_view._internal - objects_to_keep_alive[texture_view_id] = texture_view - c_resolve_target = ( - ffi.NULL - if color_attachment.get("resolve_target", None) is None - else color_attachment["resolve_target"]._internal - ) # this is a TextureViewId or null - clear_value = color_attachment.get("clear_value", (0, 0, 0, 0)) - if isinstance(clear_value, dict): - check_struct("Color", clear_value) - clear_value = _tuple_from_color(clear_value) - # H: r: float, g: float, b: float, a: float - c_clear_value = new_struct( - "WGPUColor", - r=clear_value[0], - g=clear_value[1], - b=clear_value[2], - a=clear_value[3], + c_color_attachments_list = [ + self._create_render_pass_color_attachment( + color_attachment, objects_to_keep_alive ) - # H: nextInChain: WGPUChainedStruct *, view: WGPUTextureView, depthSlice: int, resolveTarget: WGPUTextureView, loadOp: WGPULoadOp, storeOp: WGPUStoreOp, clearValue: WGPUColor - c_attachment = new_struct( - "WGPURenderPassColorAttachment", - view=texture_view_id, - resolveTarget=c_resolve_target, - loadOp=color_attachment["load_op"], - storeOp=color_attachment["store_op"], - clearValue=c_clear_value, - depthSlice=lib.WGPU_DEPTH_SLICE_UNDEFINED, # not implemented yet - # not used: resolveTarget - # not used: nextInChain - ) - c_color_attachments_list.append(c_attachment) + for color_attachment in color_attachments + ] c_color_attachments_array = ffi.new( "WGPURenderPassColorAttachment []", c_color_attachments_list ) @@ -2694,10 +2666,8 @@ def begin_render_pass( c_depth_stencil_attachment = ffi.NULL if depth_stencil_attachment is not None: check_struct("RenderPassDepthStencilAttachment", depth_stencil_attachment) - c_depth_stencil_attachment = ( - self._create_c_render_pass_depth_stencil_attachment( - depth_stencil_attachment - ) + c_depth_stencil_attachment = self._create_render_pass_stencil_attachment( + depth_stencil_attachment ) c_occlusion_query_set = ffi.NULL @@ -2723,8 +2693,48 @@ def begin_render_pass( encoder._objects_to_keep_alive = objects_to_keep_alive return encoder + def _create_render_pass_color_attachment( + self, color_attachment, objects_to_keep_alive + ): + check_struct("RenderPassColorAttachment", color_attachment) + texture_view = color_attachment["view"] + if not isinstance(texture_view, GPUTextureView): + raise TypeError("Color attachment view must be a GPUTextureView.") + texture_view_id = texture_view._internal + objects_to_keep_alive[texture_view_id] = texture_view + c_resolve_target = ( + ffi.NULL + if color_attachment.get("resolve_target", None) is None + else color_attachment["resolve_target"]._internal + ) # this is a TextureViewId or null + clear_value = color_attachment.get("clear_value", (0, 0, 0, 0)) + if isinstance(clear_value, dict): + check_struct("Color", clear_value) + clear_value = _tuple_from_color(clear_value) + # H: r: float, g: float, b: float, a: float + c_clear_value = new_struct( + "WGPUColor", + r=clear_value[0], + g=clear_value[1], + b=clear_value[2], + a=clear_value[3], + ) + # H: nextInChain: WGPUChainedStruct *, view: WGPUTextureView, depthSlice: int, resolveTarget: WGPUTextureView, loadOp: WGPULoadOp, storeOp: WGPUStoreOp, clearValue: WGPUColor + c_attachment = new_struct( + "WGPURenderPassColorAttachment", + view=texture_view_id, + resolveTarget=c_resolve_target, + loadOp=color_attachment["load_op"], + storeOp=color_attachment["store_op"], + clearValue=c_clear_value, + depthSlice=lib.WGPU_DEPTH_SLICE_UNDEFINED, # not implemented yet + # not used: resolveTarget + # not used: nextInChain + ) + return c_attachment + # Pulled out from create_render_pass because it was too large. - def _create_c_render_pass_depth_stencil_attachment(self, ds_attachment): + def _create_render_pass_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 @@ -3525,7 +3535,7 @@ def read_texture(self, source, data_layout, size): # and Numpy is not a dependency. if extra_stride or ori_offset: data_length2 = ori_stride * size[1] * size[2] + ori_offset - data2 = memoryview((ctypes.c_uint8 * data_length2)()).cast(data.format) + data2 = memoryview(bytearray(data_length2)).cast(data.format) for i in range(size[1] * size[2]): row = data[i * full_stride : i * full_stride + ori_stride] i_start = ori_offset + i * ori_stride diff --git a/wgpu/resources/codegen_report.md b/wgpu/resources/codegen_report.md index b49feff6..e2aeec66 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, 113 methods, 0 properties +* Validated 37 classes, 117 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