Glassvision/IfcOpenShell
1
0
Fork 0
mirror of https://github.com/IfcOpenShell/IfcOpenShell.git synced 2026-09-09 05:46:51 +00:00
Code Issues Packages Projects Releases Wiki Activity

ifcviewer: overhaul model/object ID tracking

Browse Source 
Rename the two overloaded model identifiers and make object_id
assignment single-authority, fixing a pick -> properties mismatch.

Identifiers:
- Per-model UUID fed_id -> model_id; the uint32 runtime handle
  model_id -> session_model_id (SessionState accessors + mirror hashes
  renamed to match). "fed_id" was a misnomer -- the federation is the
  whole collection, not one model.

object_id assignment (fixes wrong class on click):
- Producers (GeometryStreamer, .ifcview sidecar) now stamp model-LOCAL
  object_ids; ViewportCore::applyCachedModel is the sole authority that
  assigns the session-global id (base + local). Removed
  SceneLoader::next_object_id_, GeometryStreamer::lastObjectId(), and the
  streamer's start_object_id parameter.
- The element table is stamped by the same base on both load paths
  (applySidecarData and onStreamerFinished), so registry ids match the
  ids pick returns. Previously the sidecar path double-rebased instances
  vs the registry (click IfcSite -> showed IfcDoor); the live-stream path
  had the same latent mismatch. Both closed.

Naming / cleanup:
- SceneLoader::addFiles -> queueModels; startStreamLoadFor ->
  loadFromGeometryStreamer; readSidecarMetadataOnly -> readSidecarMetadata.
- Federation::addModel takes an explicit display_name (no QFileInfo
  fallback); callers pass QFileInfo(path).fileName().
- Disambiguate cryptic short locals (d->sidecar, m->model, c->chunk, ...)
  in SceneLoader, Federation, ViewportWindow, AreaMeasurement,
  SectionGizmoRenderer, and the SidecarData/SidecarReadPlan spots in
  ViewportCore.

Tests: 125/125 pass.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
Dion Moult
2026-07-08 14:10:05 +10:00
parent b441fada90
commit 75c9da5098
49 changed files with 914 additions and 885 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/ifcviewer/tests/test_federation.cpp
+39 -33
View File
@@ -60,6 +60,12 @@ QString writeStubFile(const QString& path) {
return QDir::cleanPath(fi.absoluteFilePath());
}
// addModel with the filename as its label — mirrors how the app
// (models/Commands.cpp) calls it now that addModel takes the label explicitly.
QString addLocalModel(Federation& fed, const QString& path) {
return fed.addModel(path, QFileInfo(path).fileName());
}
QJsonObject readJsonFile(const QString& path) {
QFile f(path);
REQUIRE(f.open(QIODevice::ReadOnly));
@@ -88,7 +94,7 @@ TEST_CASE("addModel emits dirty=true; markClean clears it; remove re-dirties", "
QSignalSpy spy(&fed, &Federation::dirtyChanged);
QString abs = writeStubFile(tmp.filePath("a.ifc"));
QString id = fed.addModel(abs);
QString id = addLocalModel(fed, abs);
REQUIRE_FALSE(id.isEmpty());
REQUIRE(fed.isDirty());
REQUIRE(spy.count() == 1);
@@ -108,9 +114,9 @@ TEST_CASE("addModel emits dirty=true; markClean clears it; remove re-dirties", "
TEST_CASE("addModel rejects empty paths and nested .ifcfed sources", "[federation]") {
ensureQApp();
Federation fed;
REQUIRE(fed.addModel("").isEmpty());
REQUIRE(fed.addModel("nested.ifcfed").isEmpty());
REQUIRE(fed.addModel("nested.IfcFed").isEmpty()); // case-insensitive
REQUIRE(addLocalModel(fed, "").isEmpty());
REQUIRE(addLocalModel(fed, "nested.ifcfed").isEmpty());
REQUIRE(addLocalModel(fed, "nested.IfcFed").isEmpty()); // case-insensitive
REQUIRE(fed.models().empty());
REQUIRE_FALSE(fed.isDirty());
}
@@ -152,7 +158,7 @@ TEST_CASE("setModelVisible toggles flag, dirty, and signal; idempotent", "[feder
REQUIRE(tmp.isValid());
Federation fed;
QString id = fed.addModel(writeStubFile(tmp.filePath("a.ifc")));
QString id = addLocalModel(fed, writeStubFile(tmp.filePath("a.ifc")));
REQUIRE_FALSE(id.isEmpty());
REQUIRE(fed.findById(id)->visible); // visible by default
fed.markClean();
@@ -176,7 +182,7 @@ TEST_CASE("setModelVisible toggles flag, dirty, and signal; idempotent", "[feder
REQUIRE(dirty_spy.count() == 0);
REQUIRE(vis_spy.count() == 0);
// Unknown fed_id is a no-op (no crash, no signal).
// Unknown model_id is a no-op (no crash, no signal).
fed.setModelVisible("not-a-real-id", false);
REQUIRE_FALSE(fed.isDirty());
REQUIRE(vis_spy.count() == 0);
@@ -199,7 +205,7 @@ TEST_CASE("save then load round-trips models, transform, visibility, home view",
Federation src;
QString id1 = src.addModel(src1, "Wall");
QString id2 = src.addModel(src2); // default display_name from filename
QString id2 = addLocalModel(src, src2); // filename as label
REQUIRE_FALSE(id1.isEmpty());
REQUIRE_FALSE(id2.isEmpty());
@@ -261,8 +267,8 @@ TEST_CASE("save stores paths relative when under fed_dir, absolute otherwise", "
QString outside = writeStubFile(root.filePath("elsewhere/outside.ifc"));
Federation fed;
fed.addModel(inside);
fed.addModel(outside);
addLocalModel(fed, inside);
addLocalModel(fed, outside);
QString err;
REQUIRE(fed.save(fed_path, &err));
@@ -304,7 +310,7 @@ TEST_CASE("Save-As to a different directory recomputes path relativity", "[feder
QString fed_b = fed_dir_b + "/proj.ifcfed";
Federation fed;
fed.addModel(src);
addLocalModel(fed, src);
QString err;
REQUIRE(fed.save(fed_a, &err));
@@ -514,31 +520,31 @@ TEST_CASE("setModelGroup assigns and reassigns; rejects unknown group",
ensureQApp();
QTemporaryDir tmp;
Federation fed;
QString mid = fed.addModel(writeStubFile(tmp.filePath("a.ifc")));
QString model_id = addLocalModel(fed, writeStubFile(tmp.filePath("a.ifc")));
QString gid = fed.addGroup("G");
fed.markClean();
QSignalSpy spy(&fed, &Federation::modelGroupChanged);
fed.setModelGroup(mid, gid);
REQUIRE(fed.findById(mid)->group_id == gid);
fed.setModelGroup(model_id, gid);
REQUIRE(fed.findById(model_id)->group_id == gid);
REQUIRE(fed.isDirty());
REQUIRE(spy.count() == 1);
// Idempotent.
fed.markClean();
spy.clear();
fed.setModelGroup(mid, gid);
fed.setModelGroup(model_id, gid);
REQUIRE_FALSE(fed.isDirty());
REQUIRE(spy.count() == 0);
// Unknown group is rejected.
fed.setModelGroup(mid, "no-such-group");
REQUIRE(fed.findById(mid)->group_id == gid);
fed.setModelGroup(model_id, "no-such-group");
REQUIRE(fed.findById(model_id)->group_id == gid);
REQUIRE_FALSE(fed.isDirty());
// Reassign back to root.
fed.setModelGroup(mid, QString());
REQUIRE(fed.findById(mid)->group_id.isEmpty());
fed.setModelGroup(model_id, QString());
REQUIRE(fed.findById(model_id)->group_id.isEmpty());
REQUIRE(spy.count() == 1);
}
@@ -547,12 +553,12 @@ TEST_CASE("setGroupVisible affects effective visibility cascade",
ensureQApp();
QTemporaryDir tmp;
Federation fed;
QString mid = fed.addModel(writeStubFile(tmp.filePath("a.ifc")));
QString model_id = addLocalModel(fed, writeStubFile(tmp.filePath("a.ifc")));
QString outer = fed.addGroup("Outer");
QString inner = fed.addGroup("Inner", outer);
fed.setModelGroup(mid, inner);
fed.setModelGroup(model_id, inner);
REQUIRE(fed.isModelEffectivelyVisible(mid));
REQUIRE(fed.isModelEffectivelyVisible(model_id));
REQUIRE(fed.isGroupChainVisible(inner));
// Hide the outer group: inner chain visibility flips, model effective
@@ -560,21 +566,21 @@ TEST_CASE("setGroupVisible affects effective visibility cascade",
fed.setGroupVisible(outer, false);
REQUIRE_FALSE(fed.isGroupChainVisible(outer));
REQUIRE_FALSE(fed.isGroupChainVisible(inner));
REQUIRE_FALSE(fed.isModelEffectivelyVisible(mid));
REQUIRE(fed.findById(mid)->visible);
REQUIRE_FALSE(fed.isModelEffectivelyVisible(model_id));
REQUIRE(fed.findById(model_id)->visible);
// Hiding a model directly while its group is also hidden — still
// effectively hidden.
fed.setModelVisible(mid, false);
REQUIRE_FALSE(fed.isModelEffectivelyVisible(mid));
fed.setModelVisible(model_id, false);
REQUIRE_FALSE(fed.isModelEffectivelyVisible(model_id));
// Re-show the outer group; model is still hidden by its own flag.
fed.setGroupVisible(outer, true);
REQUIRE(fed.isGroupChainVisible(inner));
REQUIRE_FALSE(fed.isModelEffectivelyVisible(mid));
REQUIRE_FALSE(fed.isModelEffectivelyVisible(model_id));
fed.setModelVisible(mid, true);
REQUIRE(fed.isModelEffectivelyVisible(mid));
fed.setModelVisible(model_id, true);
REQUIRE(fed.isModelEffectivelyVisible(model_id));
}
TEST_CASE("setGroupParent rejects cycles and self-parenting",
@@ -610,9 +616,9 @@ TEST_CASE("removeGroup reparents direct children + models up one level",
QString mid_outer = fed.addGroup("MidOuter", outer);
QString inner = fed.addGroup("Inner", mid_outer);
QString m_outer = fed.addModel(writeStubFile(tmp.filePath("a.ifc")));
QString m_mid = fed.addModel(writeStubFile(tmp.filePath("b.ifc")));
QString m_inner = fed.addModel(writeStubFile(tmp.filePath("c.ifc")));
QString m_outer = addLocalModel(fed, writeStubFile(tmp.filePath("a.ifc")));
QString m_mid = addLocalModel(fed, writeStubFile(tmp.filePath("b.ifc")));
QString m_inner = addLocalModel(fed, writeStubFile(tmp.filePath("c.ifc")));
fed.setModelGroup(m_outer, outer);
fed.setModelGroup(m_mid, mid_outer);
fed.setModelGroup(m_inner, inner);
@@ -653,8 +659,8 @@ TEST_CASE("groups + model.group_id round-trip through nested JSON save/load",
Federation src;
site_id = src.addGroup("Site");
bldg_id = src.addGroup("Building 1", site_id);
m_root = src.addModel(writeStubFile(tmp.filePath("root.ifc")));
m_bldg = src.addModel(writeStubFile(tmp.filePath("bldg.ifc")));
m_root = addLocalModel(src, writeStubFile(tmp.filePath("root.ifc")));
m_bldg = addLocalModel(src, writeStubFile(tmp.filePath("bldg.ifc")));
src.setModelGroup(m_bldg, bldg_id);
src.setGroupVisible(bldg_id, false);
src/ifcviewer/tests/test_instance_compose.cpp
+1 -1
View File
@@ -323,7 +323,7 @@ TEST_CASE("findInstanceInModels fills the correct lookup for an owned id", "[ins
InstanceCompose::InstanceLookup out;
REQUIRE(InstanceCompose::findInstanceInModels(8u, models, out));
REQUIRE(out.model_id == 2u);
REQUIRE(out.session_model_id == 2u);
REQUIRE(out.mesh_id == 4u);
REQUIRE(out.placement_transformation[12] == 22.0);
REQUIRE(out.placement_transformation[0] == 1.0);
src/ifcviewer/tests/test_instanced_geometry.cpp
+2 -2
View File
@@ -247,13 +247,13 @@ TEST_CASE("quantizeVertex passes the packed color through unchanged", "[instgeom
TEST_CASE("StreamedMesh and StreamedInstance default-init to zeroed metadata", "[instgeom]") {
StreamedMesh mc;
REQUIRE(mc.model_id == 0);
REQUIRE(mc.session_model_id == 0);
REQUIRE(mc.local_mesh_id == 0);
REQUIRE(mc.vertices.empty());
REQUIRE(mc.indices.empty());
StreamedInstance ic;
REQUIRE(ic.model_id == 0);
REQUIRE(ic.session_model_id == 0);
REQUIRE(ic.local_mesh_id == 0);
REQUIRE(ic.object_id == 0);
REQUIRE(ic.color_override_rgba8 == 0);
src/ifcviewer/tests/test_sidecar_cache.cpp
+2 -2
View File
@@ -87,7 +87,7 @@ SidecarData buildFixture() {
inst.mesh_id = (i < 3) ? 0u : 1u;
inst.object_id = uint32_t(100 + i);
inst.color_override_rgba8 = uint32_t(0xAA000000u | (i * 0x010203u));
inst.model_id = 1;
inst.session_model_id = 1;
for (int k = 0; k < 16; ++k) {
inst.placement_transformation[k] = double(i) * 0.25 + double(k);
inst.transform[k] = float(i) * 0.5f + float(k);
@@ -111,7 +111,7 @@ SidecarData buildFixture() {
for (size_t i = 0; i < sd.elements.size(); ++i) {
ElementTableRecord& e = sd.elements[i];
e.object_id = uint32_t(100 + i);
e.model_id = 1;
e.session_model_id = 1;
e.ifc_id = int32_t(1000 + i);
e.guid_offset = 0; e.guid_length = 0;
e.name_offset = 1; e.name_length = 4; // "Wall"
src/ifcviewer/tests/test_sidecar_layout.cpp
+1 -1
View File
@@ -84,7 +84,7 @@ SidecarData buildFixture() {
InstanceInfo ic;
ic.mesh_id = uint32_t(i); // authoritative
ic.object_id = obj++;
ic.model_id = 1;
ic.session_model_id = 1;
const float x = float((i * 13 + k * 5) % 11);
const float y = float((i * 7 + k * 3) % 9);
const float z = float((i * 5 + k * 2) % 7);
src/ifcviewer/tests/test_streaming_loader.cpp
+9 -9
View File
@@ -70,7 +70,7 @@ SidecarData buildFixture() {
for (size_t i = 0; i < sd.instances.size(); ++i) {
sd.instances[i].mesh_id = (i < 2) ? 0u : 1u;
sd.instances[i].object_id = uint32_t(100 + i);
sd.instances[i].model_id = 1;
sd.instances[i].session_model_id = 1;
}
sd.has_coordinate_operation = 1;
@@ -82,7 +82,7 @@ SidecarData buildFixture() {
sd.elements.resize(2);
for (size_t i = 0; i < sd.elements.size(); ++i) {
sd.elements[i].object_id = uint32_t(100 + i);
sd.elements[i].model_id = 1;
sd.elements[i].session_model_id = 1;
sd.elements[i].ifc_id = int32_t(1000 + i);
}
// v16 stores geometry per-chunk (compressed); a fixture with geometry needs
@@ -93,14 +93,14 @@ SidecarData buildFixture() {
} // namespace
TEST_CASE("readSidecarMetadataOnly returns metadata, skips bulk geometry",
TEST_CASE("readSidecarMetadata returns metadata, skips bulk geometry",
"[streaming]") {
fs::path dir = makeScratchDir("metaonly");
fs::path ifc = dir / "model.ifc";
SidecarData sd = buildFixture();
REQUIRE(writeSidecar(ifc.string(), sd));
auto meta = readSidecarMetadataOnly(ifc.string());
auto meta = readSidecarMetadata(ifc.string());
REQUIRE(meta.has_value());
// Bulk geometry is skipped, not loaded.
@@ -127,9 +127,9 @@ TEST_CASE("readSidecarMetadataOnly returns metadata, skips bulk geometry",
REQUIRE(std::memcmp(&meta->meta.meshes[1], &sd.meshes[1], sizeof(MeshInfo)) == 0);
}
TEST_CASE("readSidecarMetadataOnly rejects missing / corrupt files", "[streaming]") {
TEST_CASE("readSidecarMetadata rejects missing / corrupt files", "[streaming]") {
fs::path dir = makeScratchDir("reject");
REQUIRE_FALSE(readSidecarMetadataOnly((dir / "absent.ifc").string()).has_value());
REQUIRE_FALSE(readSidecarMetadata((dir / "absent.ifc").string()).has_value());
// Truncated head (under 16 bytes).
fs::path bad = dir / "bad.ifc";
@@ -140,7 +140,7 @@ TEST_CASE("readSidecarMetadataOnly rejects missing / corrupt files", "[streaming
std::fwrite(junk, 1, sizeof(junk), f);
std::fclose(f);
}
REQUIRE_FALSE(readSidecarMetadataOnly(bad.string()).has_value());
REQUIRE_FALSE(readSidecarMetadata(bad.string()).has_value());
}
TEST_CASE("readChunkGeometryCompressed decompresses a chunk's blobs", "[streaming]") {
@@ -148,7 +148,7 @@ TEST_CASE("readChunkGeometryCompressed decompresses a chunk's blobs", "[streamin
fs::path ifc = dir / "model.ifc";
SidecarData sd = buildFixture();
REQUIRE(writeSidecar(ifc.string(), sd));
auto meta = readSidecarMetadataOnly(ifc.string());
auto meta = readSidecarMetadata(ifc.string());
REQUIRE(meta.has_value());
REQUIRE(meta->meta.chunks.size() == 2);
@@ -202,7 +202,7 @@ TEST_CASE("v16 element metadata block: fetch via locator, decompress, parse", "[
SidecarData sd = buildFixture();
REQUIRE(writeSidecar(ifc.string(), sd));
auto meta = readSidecarMetadataOnly(ifc.string());
auto meta = readSidecarMetadata(ifc.string());
REQUIRE(meta.has_value());
REQUIRE(meta->meta.meshes.size() == sd.meshes.size()); // geometry metadata
REQUIRE(meta->meta.chunks.size() == sd.chunks.size());