mirror of
https://github.com/IfcOpenShell/IfcOpenShell.git
synced 2026-09-27 18:57:17 +00:00
ifcviewer: fix pick-pass cull corruption and cached-model ID collisions
Two stability bugs: 1. Clicking an object left the scene with wrong shading until the camera moved. The pick pass re-culls every model with its own parameters (min_pixel_radius=0, no HiZ) and overwrites each model's visible_ssbo and indirect buffer. The next render() saw an unchanged camera, skipped the cull via the have_cached_cull_ shortcut, and drew the stale pick-pass buffers. Fix: invalidate have_cached_cull_ at the end of pickObjectAt(). 2. Loading two sidecar-cached models made the second model's picked properties resolve to the first model's elements. Sidecars store raw object_id / model_id values from the session that wrote them, and both files start at object_id=1, so element_map_ entries collided. Fix: on load, rebase every PackedElementInfo and InstanceCpu by (next_object_id_ - min_id_in_sidecar) and overwrite model_id with the freshly-assigned handle before the elements hit element_map_. Also document both in the README — the pick-pass note under 3A contribution culling, the sidecar rebase under the sidecar format section.
This commit is contained in:
@@ -248,11 +248,28 @@ void MainWindow::applySidecarData(ModelId mid, SidecarData data) {
|
|||||||
QElapsedTimer t;
|
QElapsedTimer t;
|
||||||
t.start();
|
t.start();
|
||||||
|
|
||||||
// Update next_object_id_ past all objects in this model before the
|
// Sidecars store raw object_ids and model_ids from the session that wrote
|
||||||
// extracted `elements` is moved out of `data`.
|
// them. On load we must rebase both onto the current session's ID space,
|
||||||
for (const auto& elem : data.elements) {
|
// or two cached models collide (both starting at object_id=1, both
|
||||||
if (elem.object_id >= next_object_id_)
|
// claiming the original model_id). Offset by (next_object_id_ - min_id)
|
||||||
next_object_id_ = elem.object_id + 1;
|
// so the first cached object takes the next free slot.
|
||||||
|
uint32_t min_oid = UINT32_MAX;
|
||||||
|
for (const auto& pe : data.elements) {
|
||||||
|
if (pe.object_id < min_oid) min_oid = pe.object_id;
|
||||||
|
}
|
||||||
|
uint32_t oid_offset = 0;
|
||||||
|
if (!data.elements.empty() && min_oid < UINT32_MAX) {
|
||||||
|
oid_offset = next_object_id_ - min_oid;
|
||||||
|
}
|
||||||
|
for (auto& pe : data.elements) {
|
||||||
|
pe.object_id += oid_offset;
|
||||||
|
pe.model_id = mid;
|
||||||
|
if (pe.object_id >= next_object_id_)
|
||||||
|
next_object_id_ = pe.object_id + 1;
|
||||||
|
}
|
||||||
|
for (auto& inst : data.instances) {
|
||||||
|
inst.object_id += oid_offset;
|
||||||
|
inst.model_id = mid;
|
||||||
}
|
}
|
||||||
|
|
||||||
// Hand off geometry to GPU in a single call.
|
// Hand off geometry to GPU in a single call.
|
||||||
|
|||||||
@@ -307,6 +307,14 @@ uint32_t + char[] string table
|
|||||||
Staleness check: `source_file_size` vs actual file size. Mismatched →
|
Staleness check: `source_file_size` vs actual file size. Mismatched →
|
||||||
reject and rebuild. Endianness marker rejects cross-arch caches.
|
reject and rebuild. Endianness marker rejects cross-arch caches.
|
||||||
|
|
||||||
|
Sidecars store the raw `object_id` / `model_id` values from the session
|
||||||
|
that wrote them. On load they are rebased onto the current session's ID
|
||||||
|
space (`object_id += next_object_id_ - min_id_in_sidecar`, `model_id`
|
||||||
|
overwritten with the freshly-assigned handle) before the elements hit
|
||||||
|
`element_map_` or the viewport. Without this, two cached models loaded
|
||||||
|
back-to-back collide — both start at `object_id=1` and the second model's
|
||||||
|
property lookups return the first model's data.
|
||||||
|
|
||||||
### GPU Instancing pipeline (the central pillar)
|
### GPU Instancing pipeline (the central pillar)
|
||||||
|
|
||||||
Everything above plugs into a single data-flow, worth documenting on its
|
Everything above plugs into a single data-flow, worth documenting on its
|
||||||
@@ -451,6 +459,13 @@ and per-instance level. Short-circuits when the camera is inside the
|
|||||||
AABB so nothing-you're-standing-next-to is ever lost. Pick pass uses
|
AABB so nothing-you're-standing-next-to is ever lost. Pick pass uses
|
||||||
threshold 0 so sub-pixel objects remain clickable.
|
threshold 0 so sub-pixel objects remain clickable.
|
||||||
|
|
||||||
|
Because the pick pass re-runs the cull with its own parameters (no
|
||||||
|
contribution cull, no HiZ) and writes into each model's shared
|
||||||
|
`visible_ssbo` / indirect buffer, `pickObjectAt()` must invalidate
|
||||||
|
`have_cached_cull_` on exit. Otherwise the next `render()` sees an
|
||||||
|
unchanged camera, skips the cull, and draws the pick-pass buffers —
|
||||||
|
the user sees obviously-wrong shading until they nudge the camera.
|
||||||
|
|
||||||
Sphere-based (centre = AABB midpoint, radius = half-diagonal,
|
Sphere-based (centre = AABB midpoint, radius = half-diagonal,
|
||||||
r_px = focal_px · radius / distance). Loses a little precision on
|
r_px = focal_px · radius / distance). Loses a little precision on
|
||||||
very elongated bounds vs. 8-corner projection, but costs ~5× less per
|
very elongated bounds vs. 8-corner projection, but costs ~5× less per
|
||||||
|
|||||||
@@ -1282,6 +1282,13 @@ uint32_t ViewportWindow::pickObjectAt(int x, int y) {
|
|||||||
|
|
||||||
renderPickPass();
|
renderPickPass();
|
||||||
|
|
||||||
|
// The pick pass overwrote each model's visible_ssbo / indirect_buffer with
|
||||||
|
// pick-specific cull params (no contribution cull, no HiZ). Invalidate
|
||||||
|
// the cached cull so the next render() rebuilds them with main-render
|
||||||
|
// params; otherwise the viewport draws with stale pick-pass buffers and
|
||||||
|
// shading looks wrong until the camera moves.
|
||||||
|
have_cached_cull_ = false;
|
||||||
|
|
||||||
int px = x * devicePixelRatio();
|
int px = x * devicePixelRatio();
|
||||||
int py = (height() - y) * devicePixelRatio();
|
int py = (height() - y) * devicePixelRatio();
|
||||||
uint32_t pixel = 0;
|
uint32_t pixel = 0;
|
||||||
|
|||||||
Reference in New Issue
Block a user