Skip to content

Commit 2b2e218

Browse files
committed
fix: guard Vulkan source shader effects
1 parent 7eaebd7 commit 2b2e218

7 files changed

Lines changed: 117 additions & 43 deletions

File tree

include/MeshCraft/GraphicsBackendCheck.hpp

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,4 +19,26 @@ inline bool isBackendSupportedAlg(const std::string& backend, bool /*allowOverri
1919
return backend == "EASYGL" || backend == "VULKAN";
2020
}
2121

22+
// MeshCraft's optional post-processes and material-preview currently supply
23+
// source GLSL to CNA ShaderEffect. EASYGL compiles that source, but CNA's
24+
// Vulkan backend currently accepts only a narrow precompiled-SPIR-V
25+
// SpriteBatch path; it does not yet expose the named-uniform and 3D-pipeline
26+
// contract these effects require. Keep this decision separate from the
27+
// editor gate: Vulkan can render the CNA editor and ImGui, just not these
28+
// optional source-shader features.
29+
inline bool supportsTextShaderEffectsAlg(const std::string& backend) {
30+
return backend == "EASYGL";
31+
}
32+
33+
// The selected backend is embedded privately into the MeshCraft executable by
34+
// CMake. The fallback keeps this header useful for standalone algorithm
35+
// tests and non-CMake consumers, where the established default is EASYGL.
36+
inline bool supportsTextShaderEffects() {
37+
#ifdef MESH_CRAFT_GRAPHICS_BACKEND_STR
38+
return supportsTextShaderEffectsAlg(MESH_CRAFT_GRAPHICS_BACKEND_STR);
39+
#else
40+
return true;
41+
#endif
42+
}
43+
2244
} // namespace MeshCraft

plan.md

Lines changed: 21 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -653,9 +653,9 @@ portable through CNA rather than merely hiding its OpenGL dependency.
653653
editor renders correctly, including the CNA ImGui path. Automated preview
654654
screenshots remain blocked only by this host's unavailable display preflight.
655655

656-
### SYS-W8-05 `[IN_PROGRESS]` `P1` · Qualify the CNA-backed editor UI on alternate graphics backends, then remove the EASYGL-only gate
656+
### SYS-W8-05 `[BLOCKED]` `P1` · Qualify the CNA-backed editor UI on alternate graphics backends, then remove the EASYGL-only gate
657657
- **Component:** backend selection in `CMakeLists.txt`/`main.cpp`/`GraphicsBackendCheck.hpp`, ImGui platform initialization, CI configuration, and render-test launchers.
658-
- **Evidence:** The current runtime gate rejects every backend except EASYGL because the active renderer is `imgui_impl_opengl3`. Vulkan/WebGPU cannot share that GL renderer or its native texture IDs even if CNA can render the scene. Their availability and toolchain requirements are owned by CNA and must be measured, not assumed.
658+
- **Evidence:** The renderer now consumes Dear ImGui draw data entirely through CNA and uses opaque CNA texture tokens, so it has no native-OpenGL renderer dependency. Vulkan/WebGPU availability and toolchain requirements are still owned by CNA and must be measured, not assumed.
659659
- **Outcome:** For every alternate CNA backend the sibling CNA checkout actually supports (target order: Vulkan, then WebGPU), select the appropriate SDL/ImGui platform mode while keeping rendering CNA-backed; configure, build, and run the editor without an OpenGL context. Remove the EASYGL-only rejection only for backends with a passing real editor smoke/screenshot test. Keep unsupported backends rejected with a precise capability message rather than an override that launches a blank UI. Revisit Android AUD-042 only after this qualification produces a supported mobile-capable path.
660660
- **Tests:** Add a backend matrix that always performs configure+build and, where a runner/GPU backend is available, runs a real editor screenshot including `ImGui::Image()` previews. Require CNA-native scene tests plus the new UI screenshot checks per enabled backend; retain EASYGL coverage. Do not claim Vulkan/WebGPU support until this matrix has passed on each backend's real runtime.
661661
- **Dependency/rule:** Requires SYS-W8-02 through SYS-W8-04. Any missing CNA backend, SDK, CI runner, or public CNA API is recorded as a concrete blocked subcondition, not bypassed with direct OpenGL or untested `MESH_CRAFT_ALLOW_UNSUPPORTED_BACKEND` launches.
@@ -668,8 +668,25 @@ portable through CNA rather than merely hiding its OpenGL dependency.
668668
launch reached `AMD Radeon 780M (RADV PHOENIX)` and initialized CNA's Vulkan
669669
backend. It also exposed a separate remaining portability gap: the SSAO
670670
depth-prepass feeds text GLSL to `ShaderEffect`, whereas the Vulkan backend
671-
expects SPIR-V (`SPIR-V size must be a multiple of 4 bytes`), so SSAO cannot
672-
be qualified there yet. SYS-W8-05 now exposes CNA's `WEBGPU` target too; it
671+
expects SPIR-V (`SPIR-V size must be a multiple of 4 bytes`). MeshCraft now
672+
detects this capability before constructing an effect: on Vulkan it keeps
673+
the CNA scene/editor/ImGui path running, disables only Bloom, SSAO, skybox
674+
shading, and the material-preview `ImGui::Image`, and labels those UI
675+
limitations rather than emitting a shader-compile error or falling back to
676+
native GL. This is a **CNA API blocker** for full qualification, not a
677+
MeshCraft shader-conversion task: CNA needs a public cross-backend custom
678+
effect contract with (1) GLSL-to-backend compilation or a documented SPIR-V
679+
input path, (2) named uniform and multiple texture-slot binding, and (3) a
680+
caller-selected 3D vertex layout plus depth-enabled pipeline, in addition
681+
to its current narrow SpriteBatch-compatible Vulkan effect path. Until that
682+
exists, the required real Vulkan screenshot including a material-preview
683+
image cannot be produced honestly. The guarded Vulkan launch now reaches
684+
CNA initialization without the prior SPIR-V error, but then receives
685+
`SIGSEGV` in `CNA::Internal::Backends::Vulkan::VulkanGraphicsBackend::RecordCommandBuffer`
686+
while `GraphicsDevice::Present()` records the first frame (Mesa RADV on AMD
687+
Radeon 780M). That is a second, independent CNA Vulkan backend blocker;
688+
its stack is outside MeshCraft, so no direct-GL workaround is permitted.
689+
SYS-W8-05 now exposes CNA's `WEBGPU` target too; it
673690
downloaded CNA's pinned wgpu-native package, but has not yet completed a
674691
local configure or runtime check. CI now has a Vulkan configure/build matrix.
675692
Vulkan is enabled for this manual qualification; WebGPU and all other

src/MeshCraft/MeshCraftApplication.cpp

Lines changed: 17 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
#include "MeshCraft/MeshCraftApplication.hpp"
22
#include "MeshCraft/EditorAlgorithms.hpp"
3+
#include "MeshCraft/GraphicsBackendCheck.hpp"
34
#include "MeshCraftPrivate.hpp"
45

56
#include <imgui.h>
@@ -148,6 +149,18 @@ void MeshCraftApplication::LoadContent() {
148149

149150
auto& gd = getGraphicsDeviceProperty();
150151

152+
if (!supportsTextShaderEffects()) {
153+
// Do not feed GLSL source to CNA's Vulkan ShaderEffect path: it
154+
// accepts SPIR-V only and cannot represent MeshCraft's named uniforms
155+
// or 3D depth-prepass pipeline yet. The editor, scene, and CNA ImGui
156+
// renderer continue normally; only optional source-shader features
157+
// are unavailable on this backend.
158+
bloomEnabled_ = false;
159+
ssaoEnabled_ = false;
160+
std::cerr << "[MeshCraft] Source-GLSL ShaderEffects are unavailable on this backend; "
161+
"Bloom, SSAO, skybox shading, and material preview are disabled.\n";
162+
}
163+
151164
gridRenderer_ = std::make_unique<Renderer::GridRenderer>(gd);
152165
sceneRenderer_ = std::make_unique<Renderer::SceneRenderer>(gd);
153166
spriteBatch_ = std::make_unique<Graphics::SpriteBatch>(gd);
@@ -1172,7 +1185,7 @@ namespace MeshCraft {
11721185

11731186
void MeshCraftApplication::initBloom(int w, int h)
11741187
{
1175-
if (w <= 0 || h <= 0) return;
1188+
if (!supportsTextShaderEffects() || w <= 0 || h <= 0) return;
11761189
auto& gd = getGraphicsDeviceProperty();
11771190

11781191
bloomRtA_.emplace(gd, w, h);
@@ -1193,6 +1206,7 @@ void MeshCraftApplication::initBloom(int w, int h)
11931206

11941207
void MeshCraftApplication::initSkybox()
11951208
{
1209+
if (!supportsTextShaderEffects()) return;
11961210
auto& gd = getGraphicsDeviceProperty();
11971211
skyboxFx_.emplace(gd, kBloomVertSrc, kSkyboxFragSrc);
11981212
if (!skyboxFx_->IsEffectValid()) {
@@ -1328,7 +1342,7 @@ void MeshCraftApplication::applyBloom(
13281342

13291343
void MeshCraftApplication::initSsao(int w, int h)
13301344
{
1331-
if (w <= 0 || h <= 0 || !sceneRenderer_ || !sceneRenderer_->depthPassAvailable()) return;
1345+
if (!supportsTextShaderEffects() || w <= 0 || h <= 0 || !sceneRenderer_ || !sceneRenderer_->depthPassAvailable()) return;
13321346
auto& gd = getGraphicsDeviceProperty();
13331347
ssaoDepthRt_.emplace(gd, w, h, false, SurfaceFormat::Color, DepthFormat::Depth24);
13341348
ssaoRt_.emplace(gd, w, h);
@@ -1461,7 +1475,7 @@ void MeshCraftApplication::renderShadowDebugFbo(const Matrix& lightView, const M
14611475
// D7: Material preview sphere (128×128 FBO, SDF Blinn-Phong shader)
14621476
// ---------------------------------------------------------------------------
14631477
void MeshCraftApplication::initMatPreview() {
1464-
if (matPreviewRt_) return; // already initialised
1478+
if (!supportsTextShaderEffects() || matPreviewRt_) return; // already initialised/unavailable
14651479

14661480
auto& gd = getGraphicsDeviceProperty();
14671481
matPreviewRt_.emplace(gd, kMatPreviewRes, kMatPreviewRes);

src/MeshCraft/MeshCraftApplication_UiLeftPanel.cpp

Lines changed: 15 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
#include "MeshCraft/EditorAlgorithms.hpp"
2+
#include "MeshCraft/GraphicsBackendCheck.hpp"
23
#include "MeshCraft/MeshCraftApplication.hpp"
34
#include "MeshCraftPrivate.hpp"
45
#include "MeshCraft/Scene/SceneHierarchyPanel.hpp"
@@ -1431,16 +1432,20 @@ void MeshCraftApplication::drawLeftPanel(float panelY, float panelH)
14311432
ImGui::Separator();
14321433

14331434
// D7: Material preview sphere
1434-
initMatPreview();
1435-
renderMatPreview(mat.baseColor[0], mat.baseColor[1], mat.baseColor[2],
1436-
mat.roughness, mat.metallic);
1437-
if (matPreviewTextureToken_) {
1438-
float avail = ImGui::GetContentRegionAvail().x;
1439-
float sz = std::min(avail, (float)kMatPreviewRes);
1440-
float off = (avail - sz) * 0.5f;
1441-
if (off > 0.f) ImGui::SetCursorPosX(ImGui::GetCursorPosX() + off);
1442-
ImGui::Image(static_cast<ImTextureID>(matPreviewTextureToken_), ImVec2(sz, sz));
1443-
ImGui::Spacing();
1435+
if (supportsTextShaderEffects()) {
1436+
initMatPreview();
1437+
renderMatPreview(mat.baseColor[0], mat.baseColor[1], mat.baseColor[2],
1438+
mat.roughness, mat.metallic);
1439+
if (matPreviewTextureToken_) {
1440+
float avail = ImGui::GetContentRegionAvail().x;
1441+
float sz = std::min(avail, (float)kMatPreviewRes);
1442+
float off = (avail - sz) * 0.5f;
1443+
if (off > 0.f) ImGui::SetCursorPosX(ImGui::GetCursorPosX() + off);
1444+
ImGui::Image(static_cast<ImTextureID>(matPreviewTextureToken_), ImVec2(sz, sz));
1445+
ImGui::Spacing();
1446+
}
1447+
} else {
1448+
ImGui::TextDisabled("Preview requires cross-backend ShaderEffect support");
14441449
}
14451450

14461451
// F8: an edit to a material merged in from an <include> must

src/MeshCraft/MeshCraftApplication_UiMenuBar.cpp

Lines changed: 27 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
#include "MeshCraft/MeshCraftApplication.hpp"
2+
#include "MeshCraft/GraphicsBackendCheck.hpp"
23
#include "MeshCraftPrivate.hpp"
34

45
#include <imgui.h>
@@ -551,28 +552,32 @@ float MeshCraftApplication::drawMenuBar()
551552
ImGui::MenuItem("Edge Overlay", "Alt+W", &showEdgeOverlay_);
552553
ImGui::MenuItem("Wireframe Mode", nullptr, &showWireframeMode_);
553554
ImGui::MenuItem("Stats Overlay", nullptr, &showStatsOverlay_);
554-
ImGui::MenuItem("Bloom (emissive glow)", nullptr, &bloomEnabled_);
555-
if (bloomEnabled_) {
556-
ImGui::SetNextItemWidth(140);
557-
// AlwaysClamp: without it, Ctrl+Click lets a typed value go
558-
// out of [min,max] (incl. negative), which the composite
559-
// shader has no other guard against (STAB-0325).
560-
ImGui::SliderFloat(" Strength##bloom", &bloomStrength_, 0.5f, 8.0f, "%.1f",
561-
ImGuiSliderFlags_AlwaysClamp);
562-
}
563-
ImGui::MenuItem("SSAO (ambient occlusion)", nullptr, &ssaoEnabled_);
564-
if (ssaoEnabled_) {
565-
ImGui::SetNextItemWidth(140);
566-
// AlwaysClamp: same out-of-bounds-via-Ctrl+Click risk as
567-
// bloom strength above (STAB-0326).
568-
ImGui::SliderFloat(" Strength##ssao", &ssaoStrength_, 0.0f, 1.0f, "%.2f",
569-
ImGuiSliderFlags_AlwaysClamp);
570-
ImGui::SetNextItemWidth(140);
571-
// Same fix applied here too: a negative/zero radius from an
572-
// unclamped Ctrl+Click entry would break the SSAO sample
573-
// kernel, same root cause as the two strength sliders above.
574-
ImGui::SliderFloat(" Radius##ssao", &ssaoRadius_, 0.05f, 2.0f, "%.2f",
575-
ImGuiSliderFlags_AlwaysClamp);
555+
if (supportsTextShaderEffects()) {
556+
ImGui::MenuItem("Bloom (emissive glow)", nullptr, &bloomEnabled_);
557+
if (bloomEnabled_) {
558+
ImGui::SetNextItemWidth(140);
559+
// AlwaysClamp: without it, Ctrl+Click lets a typed value go
560+
// out of [min,max] (incl. negative), which the composite
561+
// shader has no other guard against (STAB-0325).
562+
ImGui::SliderFloat(" Strength##bloom", &bloomStrength_, 0.5f, 8.0f, "%.1f",
563+
ImGuiSliderFlags_AlwaysClamp);
564+
}
565+
ImGui::MenuItem("SSAO (ambient occlusion)", nullptr, &ssaoEnabled_);
566+
if (ssaoEnabled_) {
567+
ImGui::SetNextItemWidth(140);
568+
// AlwaysClamp: same out-of-bounds-via-Ctrl+Click risk as
569+
// bloom strength above (STAB-0326).
570+
ImGui::SliderFloat(" Strength##ssao", &ssaoStrength_, 0.0f, 1.0f, "%.2f",
571+
ImGuiSliderFlags_AlwaysClamp);
572+
ImGui::SetNextItemWidth(140);
573+
// Same fix applied here too: a negative/zero radius from an
574+
// unclamped Ctrl+Click entry would break the SSAO sample
575+
// kernel, same root cause as the two strength sliders above.
576+
ImGui::SliderFloat(" Radius##ssao", &ssaoRadius_, 0.05f, 2.0f, "%.2f",
577+
ImGuiSliderFlags_AlwaysClamp);
578+
}
579+
} else {
580+
ImGui::TextDisabled("Bloom and SSAO require cross-backend ShaderEffect support");
576581
}
577582
ImGui::MenuItem("Shadow Map Debug", nullptr, &shadowDebugEnabled_);
578583
ImGui::MenuItem("Snap to Grid", nullptr, &snapEnabled_);

src/MeshCraft/Renderer/SceneRenderer.cpp

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
#include "MeshCraft/Renderer/CsgCacheAlg.hpp"
33
#include "MeshCraft/Renderer/PrimitiveTessellationAlg.hpp"
44
#include "MeshCraft/EditorAlgorithms.hpp"
5+
#include "MeshCraft/GraphicsBackendCheck.hpp"
56
#include <iostream>
67

78
#include <Microsoft/Xna/Framework/Graphics/BufferUsage.hpp>
@@ -412,10 +413,12 @@ SceneRenderer::SceneRenderer(GraphicsDevice& device)
412413
effect_->setPreferPerPixelLightingProperty(true); // smoother on curved surfaces if CNA supports
413414
effect_->setLightingEnabledProperty(false); // off by default; enabled per draw in drawMeshTextured
414415

415-
depthEffect_.emplace(device_, kDepthPassVertSrc, kDepthPassFragSrc);
416-
if (!depthEffect_->IsEffectValid()) {
417-
std::cerr << "[SSAO] Failed to compile depth-prepass shader\n";
418-
depthEffect_.reset();
416+
if (supportsTextShaderEffects()) {
417+
depthEffect_.emplace(device_, kDepthPassVertSrc, kDepthPassFragSrc);
418+
if (!depthEffect_->IsEffectValid()) {
419+
std::cerr << "[SSAO] Failed to compile depth-prepass shader\n";
420+
depthEffect_.reset();
421+
}
419422
}
420423

421424
buildUnitBox();

test/graphics_backend_check_test.cpp

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
#include <string>
1010

1111
using MeshCraft::isBackendSupportedAlg;
12+
using MeshCraft::supportsTextShaderEffectsAlg;
1213

1314
static int failures = 0;
1415
static void check(bool cond, const std::string& msg) {
@@ -34,6 +35,13 @@ int main() {
3435
}
3536
check(!isBackendSupportedAlg("UNKNOWN", false), "unknown backend is rejected");
3637

38+
check(supportsTextShaderEffectsAlg("EASYGL"),
39+
"EASYGL supports MeshCraft's source-GLSL ShaderEffects");
40+
for (const char* backend : {"VULKAN", "SDL_RENDERER", "BGFX", "WEBGPU"}) {
41+
check(!supportsTextShaderEffectsAlg(backend),
42+
std::string(backend) + " does not claim unsupported source-GLSL ShaderEffects");
43+
}
44+
3745
if (failures == 0) { std::cout << "All graphics-backend-check tests passed.\n"; return 0; }
3846
std::cerr << failures << " graphics-backend-check test(s) failed.\n";
3947
return 1;

0 commit comments

Comments
 (0)