Repository navigation
Vulkan: fix stale retained-IR geometry; draw SoText2 on the on-top pass - #84
Conversation
SoText2 is a screen-space annotation: the GL path rasterises it with glDrawPixels from the raster position, so scene geometry never occludes it. Mirror that in the IR by disabling the depth test and forcing SO_DEPTH_ALWAYS; the backend routes depth-disabled commands to its on-top pass (after opaque and transparent), so 2D text no longer z-fights coplanar geometry such as the face a numeric sketch label sits on. Pinning the pass to OPAQUE keeps it out of the transparent work list, so the on-top pass draws it exactly once.
The Vulkan retained renderer could keep drawing stale geometry while the OpenGL path was correct, because two caches trusted inputs that the Sketcher's per-drag-step writes never update: - SoShape's retained-IR tessellation cache was keyed only on the shape's own notify() and SoComplexity, while the vertex data is read from inherited traversal state. A sibling SoCoordinate3 / SoNormal / SoTextureCoordinate2 rewrite (or an SoVertexProperty edit) therefore never invalidated it, so the retained path replayed the previous geometry forever. The cache now also keys on the vertex source it was built from: the SoCoordinateElement node id/num, the SoVertexProperty node id, and SoNormalElement / SoMultiTextureCoordinateElement compared via copyMatchInfo()/matches() (the SoCache identity mechanism, restricted to those elements so nothing else over-invalidates). - The Vulkan backend skipped the content hash for retained geometry when pointer identity matched, but a rebuild routinely receives the address of the streams it just freed (the frame's beginFrame() and notify() release them immediately before the new build), so the GPU vertex buffer was never re-uploaded. SoGeometryDesc gains a monotonic retainedGeneration, stamped into VulkanCachedCommand::geometryGeneration and compared in identityMatches; the pointer shortcut now additionally requires an equal, non-zero generation, and geometry without one falls back to hashing. Also snapshot the build id together with the emitted runs, so a concurrent rebuild on another viewport cannot stamp them with a foreign id. Regression tests: - testsuite/IRRetainedCacheTest.cpp (CoinTests): cache hit on idle frames, rebuild on sibling coordinate/normal/texcoord rewrites, no over-invalidation on material-only changes, and the ABA consumer-key case where the allocator returns an identical pointer with new content. - testsuite/VulkanGeometryStalenessTest.cpp: offscreen frames with pixel readback assert the drawn line actually moves when only inherited state changes; skips (77) without a Vulkan device.
The raster Vulkan backend's interaction LOD (reduced detail while the camera moves) dropped every SoDrawStyle lineWidth > 1 line to the plain 1px path. That is right for the edge-heavy sets the feature exists for, but it also removed the configured width of small sets for as long as frames kept coming -- most visibly the Sketcher grid's every-nth division lines (GridDivLineWidth, default 2, about 246 segments), whose width collapsed while the pointer moved and only came back once the frame stream stopped. Share one "large line set" scale with the CPU expansion pre-pass, which already splits there, and downgrade only at or above it: lineSegmentCount() and kLargeLineSetMinSegments in the wide-line predicate. Stippled lines and overlay redraws keep the wide path exactly as before, and a small non-stippled set is drawn by the GPU-instanced vertex shader, so keeping it wide does not bring back the CPU expansion that the LOD exists to avoid.
|
Appended The wide-line predicate now shares the expansion pre-pass's "large line set" scale ( Consumed by brianmk/FreeCAD-vulkan#146; the predicate test (which fails against the previous rule) lives out of tree in brianmk/FreeCAD-DevTools#5 under |
Picks up the Coin change that keys SoShape's retained-IR cache on the vertex sources it was built from and adds SoGeometryDesc::retainedGeneration, so the Vulkan backend can detect a rebuild whose stream pointers were recycled by the allocator. (brianmk/coin#84)
Picks up the Coin change that keys SoShape's retained-IR cache on the vertex sources it was built from and adds SoGeometryDesc::retainedGeneration, so the Vulkan backend can detect a rebuild whose stream pointers were recycled by the allocator. (brianmk/coin#84)
|
Added
Measured with a preview open while the camera moves ( |
Two independent Vulkan fixes on top of
coin-Integrate, both with regression coverage.1.
Vulkan: draw screen-space SoText2 on the on-top passSoText2is a screen-space annotation: the GL path rasterises it withglDrawPixelsfrom the raster position, so scene geometry never occludes it. The IR path now mirrors that (depth test disabled,SO_DEPTH_ALWAYS); the backend routes depth-disabled commands to its on-top pass, so 2D text no longer z-fights coplanar geometry such as the face a numeric sketch label sits on.2.
Coin: fix stale retained-IR geometry on the Vulkan pathThe Vulkan retained renderer could keep drawing stale geometry while the OpenGL path was correct, because two caches trusted inputs that a per-frame coordinate write never updates:
SoShape's retained-IR tessellation cache was keyed only on the shape's ownnotify()andSoComplexity, while vertex data is read from inherited traversal state. A siblingSoCoordinate3/SoNormal/SoTextureCoordinate2rewrite (or anSoVertexPropertyedit) therefore never invalidated it, so the retained path replayed the previous geometry forever. The cache now also keys on the vertex source it was built from: theSoCoordinateElementnode id/num, theSoVertexPropertynode id, andSoNormalElement/SoMultiTextureCoordinateElementcompared viacopyMatchInfo()/matches()(theSoCacheidentity mechanism, restricted to those elements so nothing else over-invalidates).beginFrame()andnotify()release them immediately before the new build), so the GPU vertex buffer was never re-uploaded.SoGeometryDescgains a monotonicretainedGeneration, stamped intoVulkanCachedCommand::geometryGenerationand compared inidentityMatches; the pointer shortcut now additionally requires an equal, non-zero generation, and geometry without one falls back to hashing.The build id is also snapshotted together with the emitted runs, so a concurrent rebuild on another viewport cannot stamp them with a foreign id.
Regression tests
testsuite/IRRetainedCacheTest.cpp(inCoinTests): cache hit on idle frames, rebuild on sibling coordinate/normal/texcoord rewrites, no over-invalidation on material-only changes, and the ABA consumer-key case where the allocator returns an identical pointer with new content.testsuite/VulkanGeometryStalenessTest.cpp(requires-vulkan): offscreen frames with pixel readback assert the drawn line actually moves when only inherited state changes; skips (77) without a Vulkan device.Verification
Note: the
Vulkan: publish unshifted clip planes for pickingchange from this branch was already merged as #83 and is not included here.