diff --git a/src/rendering/SoVulkanRenderManager.cpp b/src/rendering/SoVulkanRenderManager.cpp index 6b54bfacbe0..0eba1fbca90 100644 --- a/src/rendering/SoVulkanRenderManager.cpp +++ b/src/rendering/SoVulkanRenderManager.cpp @@ -430,17 +430,6 @@ class SoVulkanRenderManagerP { uint32_t sceneFpMainCount = 0; SbBool sceneFpValid = FALSE; - // Cheap draw-list fingerprint gate for the expensive graph-fingerprint walk. - // computeSceneFingerprint() hashes only the retained main commands (world - // matrices + geometry buffer pointers + counts), so it changes on a real - // geometry/transform edit but is invariant under camera motion. When it - // matches the previous frame's value, computeGraphFingerprint() (a full scene - // tree walk) is skipped and the cached graph fingerprint is reused. - uint64_t drawFpCached = 0; - SoNode * drawFpScene = nullptr; - uint32_t drawFpMainCount = 0; - SbBool drawFpValid = FALSE; - SoIRRenderAction irAction; //! Second IR action used to re-record the overlay/decoration scenes every //! frame (their node-ids churn with the camera, so they cannot be retained); @@ -1667,29 +1656,19 @@ SoVulkanRenderManagerP::prepareRenderParams(SbBool clearwindow, // below. They are re-recorded separately afterwards (cheap) and merged onto // this main list. SbBool irReplayed = FALSE; - // Cheap fast-path: the graph fingerprint walk is O(N) over the whole scene, - // and on a retained (replayed) frame with no scene change it is pure waste. - // The walk folds scene node-ids but deliberately SKIPS the camera, so the - // fingerprint is invariant under camera motion; the only thing that changes - // it is a change to the main scene graph. An SoNodeSensor attached to the - // scene root fires whenever any descendant is notified (a field write or a - // child-list edit), which is precisely a main-scene change -- so when the - // sensor has NOT fired since the last walk, the cached fingerprint is still - // exact and the O(N) walk is skipped. This catches every change the walk - // would (Coin propagates notify() up to the root), independent of any - // external revision wiring, and camera-only frames still produce the same - // (camera-invariant) fingerprint. - // - // The sensor firing is itself the replay gate, not the fingerprint: the - // fingerprint folds node identity only (pointer + node-id + child count), - // so a FIELD-ONLY write (material, transform, preselection state) leaves it - // unchanged. Replaying after such a write is unsafe anyway: the write - // notifies the shape (SoShape::notify() drops its retained tessellation), - // so the retained list's raw geometry pointers may reference freed storage. - // Re-traverse whenever the sensor has fired since the last walk; camera - // motion alone never fires the scene-root sensor (the camera lives outside - // the scene graph), so camera-only frames still replay. - const SbBool graphChanged = this->sceneGraphDirty; + // The graph fingerprint walk is O(N) over the scene nodes, so on a retained + // (replayed) frame with no scene change at all it is pure waste. The walk + // folds scene node-ids but deliberately skips camera, light, environment and + // tag/infra nodes, so the fingerprint is invariant under camera motion; the + // only thing that changes it is a change to a render-affecting node. An + // SoNodeSensor attached to the scene root fires whenever any descendant is + // notified (a field write or a child-list edit, including the camera pose -- + // FreeCAD keeps the camera inside the scene graph). When the sensor has NOT + // fired since the last walk the cached fingerprint is still exact and the + // walk/re-traversal can be skipped via the branch below. When it HAS fired + // we must recompute the fingerprint (see the else) to distinguish camera-only + // churn (identical fingerprint -> replay) from a real content change + // (different fingerprint -> re-traverse). uint64_t graphFp; const SbVec2s fpVpSize = this->viewportRegion.getViewportSizePixels(); if (this->graphFingerprintValid && this->lastFpValid && @@ -1698,34 +1677,26 @@ SoVulkanRenderManagerP::prepareRenderParams(SbBool clearwindow, graphFp = this->graphFingerprint; } else { - // Recomputing computeGraphFingerprint() walks the ENTIRE scene-graph tree - // (graphFingerprintWalk, O(nodes) + geometry identity) and is measurable - // on a many-object scene (1000 boxes -> ~28 ms). It only needs to run - // when the retained main draw list actually changed. graphSceneDirty is - // set by a scene-root sensor that fires on ANY descendant notification, - // including the camera pose (FreeCAD keeps the camera inside the scene - // graph), so camera-orbit frames set it too -- and recomputing the full - // graph fingerprint there is pure waste: camera motion never changes the - // retained main-list content. Gate the expensive walk on the cheap - // draw-list fingerprint (computeSceneFingerprint hashes only the retained - // commands: world matrix + geometry pointers + counts). Camera-only - // frames produce the same draw-list fingerprint, so we keep the previous - // graph fingerprint (which is likewise camera-invariant) and the retained - // list replays instead of re-traversing. - const uint64_t drawFp = computeSceneFingerprint( - this->irAction, static_cast(this->mainCommandCount)); - if (this->drawFpValid && this->drawFpCached == drawFp && - this->scene == this->drawFpScene && - this->mainCommandCount == this->drawFpMainCount) { - graphFp = this->graphFingerprint; // draw list unchanged -> reuse - } - else { - graphFp = this->computeGraphFingerprint(); - this->drawFpCached = drawFp; - this->drawFpScene = this->scene; - this->drawFpMainCount = this->mainCommandCount; - this->drawFpValid = TRUE; - } + // Recompute the graph fingerprint whenever the scene sensor has fired. + // graphFingerprintWalk folds the node-id of every non-camera-coupled, + // render-affecting node, so a real content change that alters what the + // walk sees -- an add/remove, a geometry rebuild, a transform/model-matrix + // write, a material/selection field write, or a SoSwitch::whichChild + // visibility toggle -- bumps at least one folded id and produces a + // DIFFERENT fingerprint, correctly forcing a re-traverse below. The + // root sensor also fires on camera pose/headlight motion, but those nodes + // are EXCLUDED from the walk, so camera-only frames yield an identical + // (camera-invariant) fingerprint and the retained main list replays. + // + // Do NOT short-circuit this walk with a cheap hash of the retained draw + // list: that is unsound. The retained list is the PREVIOUS frame's graph + // output, so a visibility toggle (SoSwitch::whichChild) changes the scene + // without yet changing the retained commands -- their fingerprint is + // therefore unchanged, and skipping the walk would replay the stale list + // forever, leaving an object's show/hide state never reflected in the + // viewport. The walk is the authoritative signal and is O(nodes) (a few + // mixHash per node), far cheaper than an actual re-traversal. + graphFp = this->computeGraphFingerprint(); this->sceneGraphDirty = FALSE; } this->lastFpScene = this->scene; diff --git a/testsuite/CMakeLists.txt b/testsuite/CMakeLists.txt index 23a8f6ec8e0..0c333428556 100644 --- a/testsuite/CMakeLists.txt +++ b/testsuite/CMakeLists.txt @@ -593,6 +593,20 @@ if(HAVE_EGL) add_test(NAME VulkanRenderManagerTest COMMAND VulkanRenderManagerTest) set_tests_properties(VulkanRenderManagerTest PROPERTIES SKIP_RETURN_CODE 77) + add_executable(VulkanRenderManagerVisibilityTest vulkan/vulkan-rendermanager-visibility-test.cpp) + target_link_libraries(VulkanRenderManagerVisibilityTest Coin ${COIN_TARGET_LINK_LIBRARIES}) + target_compile_definitions(VulkanRenderManagerVisibilityTest PRIVATE COIN_INTERNAL) + target_include_directories(VulkanRenderManagerVisibilityTest PRIVATE + ${PROJECT_SOURCE_DIR}/src + ${PROJECT_SOURCE_DIR}/include + ${PROJECT_BINARY_DIR}/include + ${PROJECT_SOURCE_DIR}/testsuite + ${COIN_TARGET_INCLUDE_DIRECTORIES} + ) + add_test(NAME VulkanRenderManagerVisibilityTest COMMAND VulkanRenderManagerVisibilityTest) + set_tests_properties(VulkanRenderManagerVisibilityTest PROPERTIES SKIP_RETURN_CODE 77) + + add_executable(VulkanLightStabilityTest vulkan/vulkan-light-stability-test.cpp) target_link_libraries(VulkanLightStabilityTest Coin ${COIN_TARGET_LINK_LIBRARIES}) target_compile_definitions(VulkanLightStabilityTest PRIVATE COIN_INTERNAL) diff --git a/testsuite/vulkan/vulkan-rendermanager-visibility-test.cpp b/testsuite/vulkan/vulkan-rendermanager-visibility-test.cpp new file mode 100644 index 00000000000..db0dab033cf --- /dev/null +++ b/testsuite/vulkan/vulkan-rendermanager-visibility-test.cpp @@ -0,0 +1,151 @@ +// testsuite/vulkan/vulkan-rendermanager-visibility-test.cpp +// +// End-to-end test of the retained-IR replay gate in SoVulkanRenderManager: +// FreeCAD hides/shows an object by setting SoSwitch::whichChild on the view's +// root switch. A garbage frame (camera-only) first settles the retained-list +// fingerprint cache; a subsequent visibility toggle changes the scene WITHOUT +// changing the (yet-to-be-rebuilt) retained draw list. The replay gate must +// NOT treat that as "unchanged" and replay the stale list -- the cube must +// actually disappear on the next render. +// +// Before the fix the graph-fingerprint walk was short-circuited by a cheap hash +// of the retained draw list; the toggle left that hash unchanged, the walk was +// skipped, and the stale list replayed forever -- the cube stayed visible. + +#include "VulkanTestHarness.h" + +#include +#include +#include +#include +#include +#include +#include +#include +#include + +using namespace vulkan_test; + +int +main() +{ + Harness harness; + const int initResult = harness.init(); + if (initResult != 0) return initResult; + + int failures = 0; + + SoSeparator * root = new SoSeparator; + root->ref(); + + SoPerspectiveCamera * camera = new SoPerspectiveCamera; + camera->position.setValue(0.0f, 0.0f, 5.0f); + camera->nearDistance = 0.1f; + camera->farDistance = 100.0f; + camera->heightAngle = 0.785398f; // 45 degrees + camera->orientation.setValue(SbVec3f(0.0f, 0.0f, 1.0f), 0.0f); + root->addChild(camera); + + SoLightModel * lightModel = new SoLightModel; + lightModel->model = SoLightModel::BASE_COLOR; + root->addChild(lightModel); + + SoMaterial * material = new SoMaterial; + material->diffuseColor.setValue(1.0f, 0.0f, 0.0f); + root->addChild(material); + + SoSwitch * sw = new SoSwitch; + sw->whichChild = 0; + SoCube * cube = new SoCube; + cube->width = 2.0f; + cube->height = 2.0f; + cube->depth = 2.0f; + sw->addChild(cube); + root->addChild(sw); + + { + SoVulkanRenderManager manager; + manager.setSceneGraph(root); + manager.setCamera(camera); + + SbViewportRegion viewport(kWidth, kHeight); + viewport.setViewportPixels(SbVec2s(0, 0), SbVec2s(kWidth, kHeight)); + manager.setViewportRegion(viewport); + manager.setBackgroundColor(SbColor4f(0.0f, 0.0f, 0.0f, 1.0f)); + manager.setRenderTarget(&harness.target); + + if (!manager.initialize(&harness.deviceContext)) { + std::cerr << "FAIL: manager could not initialize the Vulkan backend" + << std::endl; + ++failures; + root->unref(); + harness.shutdown(); + SoDB::finish(); + return failures == 0 ? 0 : 1; + } + + auto countRed = [&](const char * tag) { + const std::vector pixels = harness.readback(); + const int red = countNear(pixels, 255, 0, 0); + const uint8_t * center = pixelAt(pixels, 16, 16); + std::cerr << "[TEST] " << tag << " redPixels=" << red + << " center=(" << (int)center[2] << "," << (int)center[1] + << "," << (int)center[0] << ")" << std::endl; + return red; + }; + + // Frame 1: initial build -> cube visible. + if (!manager.render(TRUE, TRUE)) { + std::cerr << "FAIL: frame-1 render failed" << std::endl; + ++failures; + } else if (countRed("frame1 build") <= 0) { + std::cerr << "FAIL: cube not rendered red on frame 1" << std::endl; + ++failures; + } + + // Frame 2: camera-only "settling" frame. Moving the camera inside the + // scene fires the scene-dirty sensor but leaves the retained content + // unchanged, so the retained-list fingerprint cache settles (drawFpCached + // now equals the cube list) and the gate is armed for the steady state. + camera->position.setValue(0.0f, 0.0f, 6.0f); + SoDB::getSensorManager()->processDelayQueue(TRUE); + if (!manager.render(TRUE, TRUE)) { + std::cerr << "FAIL: frame-2 framing render failed" << std::endl; + ++failures; + } else if (countRed("frame2 settle") <= 0) { + std::cerr << "FAIL: cube not rendered red on settle frame" << std::endl; + ++failures; + } + + // Frame 3: hide the cube via SoSwitch::whichChild. The retained list is + // NOT rebuilt yet, so its fingerprint is unchanged; only the graph walk can + // see the toggle. Old code replays the stale list; the fix re-traverses. + sw->whichChild = -1; + SoDB::getSensorManager()->processDelayQueue(TRUE); + if (!manager.render(TRUE, TRUE)) { + std::cerr << "FAIL: frame-3 (hide) render failed" << std::endl; + ++failures; + } else if (countRed("frame3 hide") != 0) { + std::cerr << "FAIL: cube STILL rendered after hide (stale replay)" + << std::endl; + ++failures; + } + + // Frame 4: show the cube again. + sw->whichChild = 0; + SoDB::getSensorManager()->processDelayQueue(TRUE); + if (!manager.render(TRUE, TRUE)) { + std::cerr << "FAIL: frame-4 (show) render failed" << std::endl; + ++failures; + } else if (countRed("frame4 show") <= 0) { + std::cerr << "FAIL: cube not rendered on frame-4 after show" + << std::endl; + ++failures; + } + } // manager destroyed before harness.shutdown() + + root->unref(); + harness.shutdown(); + SoDB::finish(); + return failures == 0 ? 0 : 1; +}