From 07bf1bd87b5f837424762089386a5e7903fe7d25 Mon Sep 17 00:00:00 2001 From: Sergey Sharybin Date: Mon, 28 Jul 2025 16:54:10 +0200 Subject: [PATCH] OpenSubdiv: Switch away from GLSL on Apple Use MTLPatchShaderSource to provide the patch basis shader source on all Apple platforms. The immediate advantage of this change is ability to use GPU subdivision on iOS. Another advantage is that it moves us further away from frameworks which got deprecated by Apple and it might save us some headache in the future. Also tweak backend-specific defines to match definitions from OpenSubdiv. The annoying difference is that OSD_PATCH_BASIS_METAL is defined by the OpenSubdiv as 1 in the very beginning of the base code, which is not done for the OSD_PATCH_BASIS_GLSL is not defined by the OpenSubdiv at all. Ref #143445 --- TODO: - [X] Check it works correctly on macOS - [x] Check it works correctly on Linux Pull Request: https://projects.blender.org/blender/blender/pulls/143462 --- .../internal/evaluator/evaluator_capi.cc | 10 +++++++++- .../internal/evaluator/gpu_compute_evaluator.cc | 15 ++++++++++++--- source/blender/draw/intern/shaders/subdiv_info.hh | 4 +++- 3 files changed, 24 insertions(+), 5 deletions(-) diff --git a/intern/opensubdiv/internal/evaluator/evaluator_capi.cc b/intern/opensubdiv/internal/evaluator/evaluator_capi.cc index 96c72f960fc..0a0154c0d9b 100644 --- a/intern/opensubdiv/internal/evaluator/evaluator_capi.cc +++ b/intern/opensubdiv/internal/evaluator/evaluator_capi.cc @@ -6,7 +6,11 @@ #include "opensubdiv_evaluator_capi.hh" -#include +#ifdef __APPLE__ +# include +#else +# include +#endif #include "MEM_guardedalloc.h" @@ -34,7 +38,11 @@ const char *openSubdiv_getGLSLPatchBasisSource() /* Using a global string to avoid dealing with memory allocation/ownership. */ static std::string patch_basis_source; if (patch_basis_source.empty()) { +#ifdef __APPLE__ + patch_basis_source = OpenSubdiv::Osd::MTLPatchShaderSource::GetPatchBasisShaderSource(); +#else patch_basis_source = OpenSubdiv::Osd::GLSLPatchShaderSource::GetPatchBasisShaderSource(); +#endif } return patch_basis_source.c_str(); } diff --git a/intern/opensubdiv/internal/evaluator/gpu_compute_evaluator.cc b/intern/opensubdiv/internal/evaluator/gpu_compute_evaluator.cc index da48a91dadb..00e78e2580c 100644 --- a/intern/opensubdiv/internal/evaluator/gpu_compute_evaluator.cc +++ b/intern/opensubdiv/internal/evaluator/gpu_compute_evaluator.cc @@ -9,7 +9,6 @@ #include #include #include -#include #include #include @@ -336,8 +335,13 @@ static GPUShader *compile_eval_stencil_shader(BufferDescriptor const &srcDesc, using namespace blender::gpu::shader; ShaderCreateInfo info("opensubdiv_compute_eval"); info.local_group_size(workGroupSize, 1, 1); + + /* Ensure the basis code has access to proper backend specification define: it is not guaranteed + * that the code provided by OpenSubdiv specifies it. For example, it doesn't for GLSL but it + * does for Metal. Additionally, for Metal OpenSubdiv defines OSD_PATCH_BASIS_METAL as 1, so do + * the same here to avoid possible warning about value being re-defined. */ if (GPU_backend_get_type() == GPU_BACKEND_METAL) { - info.define("OSD_PATCH_BASIS_METAL"); + info.define("OSD_PATCH_BASIS_METAL", "1"); } else { info.define("OSD_PATCH_BASIS_GLSL"); @@ -438,8 +442,13 @@ static GPUShader *compile_eval_patches_shader(BufferDescriptor const &srcDesc, using namespace blender::gpu::shader; ShaderCreateInfo info("opensubdiv_compute_eval"); info.local_group_size(workGroupSize, 1, 1); + + /* Ensure the basis code has access to proper backend specification define: it is not guaranteed + * that the code provided by OpenSubdiv specifies it. For example, it doesn't for GLSL but it + * does for Metal. Additionally, for Metal OpenSubdiv defines OSD_PATCH_BASIS_METAL as 1, so do + * the same here to avoid possible warning about value being re-defined. */ if (GPU_backend_get_type() == GPU_BACKEND_METAL) { - info.define("OSD_PATCH_BASIS_METAL"); + info.define("OSD_PATCH_BASIS_METAL", "1"); } else { info.define("OSD_PATCH_BASIS_GLSL"); diff --git a/source/blender/draw/intern/shaders/subdiv_info.hh b/source/blender/draw/intern/shaders/subdiv_info.hh index 41c474b5867..aae06686111 100644 --- a/source/blender/draw/intern/shaders/subdiv_info.hh +++ b/source/blender/draw/intern/shaders/subdiv_info.hh @@ -30,7 +30,9 @@ GPU_SHADER_CREATE_END() * \{ */ #ifdef __APPLE__ -# define SUBDIV_PATCH_EVALUATION_BASIS_DEFINES() DEFINE("OSD_PATCH_BASIS_METAL") +/* Match definition from OPenSubdiv which defines OSD_PATCH_BASIS_METAL as 1. Matching it here + * avoids possible re-definition warning at runtime. */ +# define SUBDIV_PATCH_EVALUATION_BASIS_DEFINES() DEFINE_VALUE("OSD_PATCH_BASIS_METAL", "1") #else # define SUBDIV_PATCH_EVALUATION_BASIS_DEFINES() DEFINE("OSD_PATCH_BASIS_GLSL") #endif