diff --git a/src/ifcviewer-web/main_web.cpp b/src/ifcviewer-web/main_web.cpp index 876e8a0e59..181d2c23d8 100644 --- a/src/ifcviewer-web/main_web.cpp +++ b/src/ifcviewer-web/main_web.cpp @@ -359,7 +359,7 @@ EM_BOOL onMouseUp(int, const EmscriptenMouseEvent* e, void* user) { if (id != 0) { app->core.logSelectedObjectGuidWeb(id); } else if (!add && !remove) { - EM_ASM({ if (Module.__ifcvOnSelect) Module.__ifcvOnSelect(0, '', -1); }); + EM_ASM({ if (Module.__ifcvOnSelect) Module.__ifcvOnSelect(0, '', -1, -1); }); } app->host.requestFrame(); }); @@ -835,6 +835,7 @@ extern "C" EMSCRIPTEN_KEEPALIVE void ifcv_request_objects_c(int token) { first = false; json += "{\"objectId\":" + std::to_string(e.object_id) + ",\"model\":" + std::to_string(e.model_index) + + ",\"sourceId\":" + std::to_string(e.source_id) + ",\"guid\":" + jsonString(e.guid) + ",\"name\":" + jsonString(e.name) + ",\"type\":" + jsonString(e.type) + '}'; diff --git a/src/ifcviewer-web/tests/model-order.spec.mjs b/src/ifcviewer-web/tests/model-order.spec.mjs new file mode 100644 index 0000000000..d7fc86bc22 --- /dev/null +++ b/src/ifcviewer-web/tests/model-order.spec.mjs @@ -0,0 +1,84 @@ +import { test, expect } from '@playwright/test'; + +// Which file did this object come from? Every host page answers that by taking +// the `model` index the viewer reports and looking it up in its own list of +// models, in the order it added them — the mapping the API documents. The +// index is only worth anything if it survives federated models finishing their +// loads out of order, which is exactly what happens over a real network. +// +// The two georef fixtures carry fixed GUIDs, so an object can be attributed to +// its file here without trusting the very index under test. +const GUIDS = { + 'georef-a': ['13r0IXtWf5pf18Q1EGzHXl', '22CLYZYiz8ZhbpLaYDVIu6'], + 'georef-b': ['3DkP2KRu5AIRxhhAz$DcQH', '2ueyz_jIr2QgMKs4v0fWl2'], +}; + +test('model index follows add order when the first model loads last', async ({ page }) => { + const errors = []; + page.on('pageerror', (e) => errors.push('pageerror: ' + e.message)); + await page.goto('/scripting.html'); + await page.waitForFunction(() => !!(window.viewer && window.viewer.isLive()), null, + { timeout: 30_000 }); + + // georef-a is added first but served slowly, so every one of its range reads + // lands after georef-b's. Without a stable ordering the core hands out its + // load-order slots in completion order and the two models come back swapped. + const sourceIds = await page.evaluate(async () => { + const a = await window.viewer.addUrl('/georef-a.ifcview?delay=120', { replace: true }); + const b = await window.viewer.addUrl('/georef-b.ifcview'); + return [a, b]; + }); + expect(sourceIds[0]).toBeLessThan(sourceIds[1]); + + await page.waitForFunction(() => window.viewer.modelCount() === 2, null, { timeout: 30_000 }); + + const objects = await page.evaluate(() => window.viewer.getObjects()); + const rowFor = (guid) => objects.find((o) => o.guid === guid) || {}; + + for (const guid of GUIDS['georef-a']) { + expect(rowFor(guid).model, `${guid} belongs to georef-a, added first`).toBe(0); + expect(rowFor(guid).sourceId, `${guid} came from georef-a's source`).toBe(sourceIds[0]); + } + for (const guid of GUIDS['georef-b']) { + expect(rowFor(guid).model, `${guid} belongs to georef-b, added second`).toBe(1); + expect(rowFor(guid).sourceId, `${guid} came from georef-b's source`).toBe(sourceIds[1]); + } + expect(errors, errors.join('\n')).toEqual([]); +}); + +test('a pick reports the source the model was added from', async ({ page }) => { + const errors = []; + page.on('pageerror', (e) => errors.push('pageerror: ' + e.message)); + await page.goto('/scripting.html'); + await page.waitForFunction(() => !!(window.viewer && window.viewer.isLive()), null, + { timeout: 30_000 }); + + await page.evaluate(async () => { + await window.viewer.addUrl('/georef-a.ifcview?delay=120', { replace: true }); + await window.viewer.addUrl('/georef-b.ifcview'); + }); + await page.waitForFunction(() => window.viewer.modelCount() === 2, null, { timeout: 30_000 }); + // The pick payload is built from the element table, so make sure it is + // resident and take the same table to check the answer against. + const objects = await page.evaluate(() => window.viewer.getObjects()); + await page.evaluate(() => window.viewer.viewAll()); + await page.waitForTimeout(800); + + // Whichever box the click lands on is fine — what is under test is that the + // pick and the object table agree about which file the object came from. + await page.evaluate(() => { + window.__pick = new Promise((resolve) => window.viewer.onSelect(resolve)); + }); + const box = await page.locator('#viewer-canvas').boundingBox(); + // Web preset: RMB selects (LMB orbits). + await page.mouse.click(box.x + box.width / 2, box.y + box.height / 2, { button: 'right' }); + const detail = await page.evaluate(() => window.__pick); + + expect(detail.guid, 'click hit empty space').toBeTruthy(); + const row = objects.find((o) => o.guid === detail.guid); + expect(row, 'picked a GUID that is not in the object table').toBeTruthy(); + expect(detail.sourceId, 'pick and object table disagree on the source').toBe(row.sourceId); + expect(detail.modelIndex).toBe(row.model); + expect(detail.sourceId).not.toBeNull(); + expect(errors, errors.join('\n')).toEqual([]); +}); diff --git a/src/ifcviewer-web/tests/serve.mjs b/src/ifcviewer-web/tests/serve.mjs index 3bb00217a9..124868dd77 100644 --- a/src/ifcviewer-web/tests/serve.mjs +++ b/src/ifcviewer-web/tests/serve.mjs @@ -31,6 +31,12 @@ http.createServer(async (req, res) => { try { const url = new URL(req.url, `http://localhost:${PORT}`); let p = decodeURIComponent(url.pathname); + // ?delay= stalls every response for this URL, HEAD and Range alike. + // Load order across federated models is decided by whichever model's + // async read chain finishes first, so a test that wants a specific + // interleaving has to be able to make one source slower than another. + const delay = Number(url.searchParams.get('delay') || 0); + if (delay > 0) await new Promise((r) => setTimeout(r, delay)); if (p === '/') p = '/IfcViewerWeb.html'; const inRoot = path.join(ROOT, p); const inSrc = path.join(SRC, p); diff --git a/src/ifcviewer-web/web/ifcviewer.js b/src/ifcviewer-web/web/ifcviewer.js index 4753cac38f..7fe800899d 100644 --- a/src/ifcviewer-web/web/ifcviewer.js +++ b/src/ifcviewer-web/web/ifcviewer.js @@ -11,7 +11,7 @@ // await viewer.addFile(file, { replace: true }); // await viewer.addUrl('/model.ifcview'); // appends (federation) // -// const objects = await viewer.getObjects(); // [{objectId, guid, name, type, model}] +// const objects = await viewer.getObjects(); // [{objectId, guid, name, type, model, sourceId}] // viewer.setSelection(['3vB2YO$MX4xv5uCqZZG05x']); // viewer.setColor(objects.filter(o => o.type === 'IfcWall'), '#ff8800'); // viewer.setCamera({ yaw: 45, pitch: 30 }); @@ -21,6 +21,14 @@ // The canvas element MUST have id="viewer-canvas" — the wasm side hard-codes // that selector for its WebGPU surface and input handlers. // +// Model identity. addFile/addUrl return a source id: the handle for that model, +// minted the moment it is registered and stable for the session. Objects come +// back tagged with both their `sourceId` and a `model` index (the model's slot +// in load order). Map an object to the file it came from through the source id +// — the index is a POSITION, so it shifts down if an earlier model fails to +// load, and a host keying its own list off it then attributes objects to the +// wrong file. +// // Object identity. Everything the scripting API takes or returns is keyed by // `objectId`: a u32 the renderer assigns, unique across the federation but only // meaningful for this session. IFC GlobalIds are the stable identity, and every @@ -197,15 +205,17 @@ // a remote URL (HTTP Range). load_sidecar_from_source_c(sid) streams one. Module.__ifcvSources = Module.__ifcvSources || []; - // The wasm calls this on every single-object pick; (0, '', -1) means the + // The wasm calls this on every single-object pick; (0, '', -1, -1) means the // selection was cleared. modelIndex is the picked object's model in load - // order (matches the modelProgress index), or -1. A marquee box-select does - // NOT fire this (it has no single object) — use onSelectionChange for that. - Module.__ifcvOnSelect = function (objectId, guid, modelIndex) { + // order (matches the modelProgress index) and sourceId the source it was + // added from, either null when unknown. A marquee box-select does NOT fire + // this (it has no single object) — use onSelectionChange for that. + Module.__ifcvOnSelect = function (objectId, guid, modelIndex, sourceId) { const detail = { objectId: objectId >>> 0, guid: guid || null, modelIndex: (typeof modelIndex === 'number' && modelIndex >= 0) ? modelIndex : null, + sourceId: (typeof sourceId === 'number' && sourceId >= 0) ? sourceId : null, }; selectListeners.forEach(function (cb) { try { cb(detail); } catch (e) { console.error(e); } @@ -279,8 +289,8 @@ // ---- Events ---------------------------------------------------------- - // Single-object picks (click). Fires with {objectId, guid, modelIndex}. - // Returns an unsubscribe function. + // Single-object picks (click). Fires with + // {objectId, guid, modelIndex, sourceId}. Returns an unsubscribe function. onSelect: function (cb) { selectListeners.push(cb); return function () { @@ -421,11 +431,14 @@ // ---- Objects --------------------------------------------------------- - // Every object in the scene: [{objectId, guid, name, type, model}], where - // `model` is the index into the load-ordered model list (same index as - // modelProgress). Asynchronous — the element tables are fetched lazily per - // model so first paint never waits on them. Resolving this is also what - // lets every other call accept GlobalIds; the result is cached for that. + // Every object in the scene: + // [{objectId, guid, name, type, model, sourceId}], where `model` is the + // index into the load-ordered model list (same index as modelProgress) + // and `sourceId` the source the model was added from — see the model + // identity note at the top of the file. Asynchronous — the element tables + // are fetched lazily per model so first paint never waits on them. + // Resolving this is also what lets every other call accept GlobalIds; the + // result is cached for that. getObjects: function () { const token = ++objectsToken; return new Promise(function (resolve) { diff --git a/src/ifcviewer/ModelGpuData.h b/src/ifcviewer/ModelGpuData.h index 781afac955..14d8fac13a 100644 --- a/src/ifcviewer/ModelGpuData.h +++ b/src/ifcviewer/ModelGpuData.h @@ -319,7 +319,9 @@ struct ModelGpuData { // (Module.__ifcvSources[id] = a picked File or a remote URL) this model's // chunk + element metadata reads pull from. Lets several federated models stream // from different files at once, mirroring the desktop per-model path. - int web_source_id = 0; + // -1 when the model came from somewhere else (a path read on desktop, the + // embedded sample) — source id 0 is a real source, so it can't mean "none". + int web_source_id = -1; // v15 element metadata (web, on-demand). The IFC element metadata // (elements + string_table — names/GUIDs, for UI/picking, never diff --git a/src/ifcviewer/ViewportCore.cpp b/src/ifcviewer/ViewportCore.cpp index b8e734391b..f7c01f3f7a 100644 --- a/src/ifcviewer/ViewportCore.cpp +++ b/src/ifcviewer/ViewportCore.cpp @@ -3898,10 +3898,24 @@ void ViewportCore::loadSidecarMetadataWeb(int source_id, std::string source_labe return; } + // Mint the session model id HERE, synchronously, rather than at the end of + // the read chain below. Session ids are what orders the scene's models — + // modelIdsInLoadOrder sorts by them, and every per-model slot a host sees + // (modelProgress's index, ElementRef::model_index) is a rank in that order. + // Minting on completion made that rank the order the models' network reads + // happened to finish in, so with several federated models in flight the + // slots came out shuffled against the order the host added them and a pick + // was attributed to the wrong file. Requesting order is the order the host + // asked for, which is the order it can reason about. A load that fails + // partway simply abandons its id — the ranks compact over whatever models + // made it into the scene, exactly as before. + const std::uint32_t session_model_id = next_session_model_id_++; + // Head (v16): [header 12][geom_bytes 8]. The two compressed metadata blocks // follow the compressed geometry at SIDECAR_HEAD_BYTES + geom_bytes. webReadRangesAsync(source_id, 0, {{0, SIDECAR_HEAD_BYTES}}, - [this, fsize, source_id, source_label, on_loaded = std::move(on_loaded)] + [this, fsize, source_id, source_label, session_model_id, + on_loaded = std::move(on_loaded)] (bool ok, std::vector&& head) mutable { std::uint64_t geom_bytes = 0; if (!ok || !parseSidecarHead(head.data(), head.size(), geom_bytes)) { @@ -3915,7 +3929,7 @@ void ViewportCore::loadSidecarMetadataWeb(int source_id, std::string source_labe } // Geometry metadata block on disk: [comp u64][raw u64][zstd frame]. webReadRangesAsync(source_id, 0, {{meta_off, 16}}, - [this, fsize, meta_off, source_id, source_label, + [this, fsize, meta_off, source_id, source_label, session_model_id, on_loaded = std::move(on_loaded)] (bool ok2, std::vector&& h) { if (!ok2 || h.size() < 16) { @@ -3934,7 +3948,7 @@ void ViewportCore::loadSidecarMetadataWeb(int source_id, std::string source_labe {{geometry_metadata_off, geometry_metadata_comp}}, [this, geometry_metadata_off, geometry_metadata_comp, geometry_metadata_raw, source_id, source_label, - on_loaded = std::move(on_loaded)] + session_model_id, on_loaded = std::move(on_loaded)] (bool ok3, std::vector&& cz) { if (!ok3) { Log::warn() << "loadSidecarMetadataWeb: geometry metadata read failed"; @@ -3971,7 +3985,7 @@ void ViewportCore::loadSidecarMetadataWeb(int source_id, std::string source_labe geometry_metadata_off + geometry_metadata_comp; webReadRangesAsync(source_id, 0, {{element_metadata_hdr_off, 16}}, [this, sc = std::move(sc), element_metadata_hdr_off, - source_id, source_label, + source_id, source_label, session_model_id, on_loaded = std::move(on_loaded)] (bool ok4, std::vector&& dh) mutable { if (ok4 && dh.size() >= 16) { @@ -3989,7 +4003,6 @@ void ViewportCore::loadSidecarMetadataWeb(int source_id, std::string source_labe const std::size_t n_meshes = sc.meta.meshes.size(); const std::size_t n_instances = sc.meta.instances.size(); - const std::uint32_t session_model_id = next_session_model_id_++; applyCachedModel(session_model_id, std::move(sc)); // Mark web-streamed + set the source IMMEDIATELY — the // model now has non-resident chunks and the RAF loop's @@ -4091,11 +4104,14 @@ void ViewportCore::logSelectedObjectGuidWeb(std::uint32_t object_id) { } Log::info() << "pick: object " << object_id << " GUID " << e.guid; // Surface the selection to JS so host pages can react (e.g. show the - // GUID + model). Fires Module.__ifcvOnSelect(object_id, guid, modelIndex); - // model_index is the load-order slot, matching the JS model list. + // GUID + model). Fires + // Module.__ifcvOnSelect(object_id, guid, modelIndex, sourceId). + // modelIndex is the load-order slot; sourceId is the byte-source the + // host added the model from, which is the one that cannot shift. EM_ASM({ - if (Module.__ifcvOnSelect) Module.__ifcvOnSelect($0, UTF8ToString($1), $2); - }, object_id, e.guid.c_str(), e.model_index); + if (Module.__ifcvOnSelect) + Module.__ifcvOnSelect($0, UTF8ToString($1), $2, $3); + }, object_id, e.guid.c_str(), e.model_index, e.source_id); }); } #endif // __EMSCRIPTEN__ @@ -4157,6 +4173,7 @@ ViewportCore::ElementRef makeElementRef(const ModelGpuData& m, int model_index, ViewportCore::ElementRef ref; ref.object_id = e.object_id; ref.model_index = model_index; + ref.source_id = m.web_source_id; ref.guid = str(e.guid_offset, e.guid_length); ref.name = str(e.name_offset, e.name_length); ref.type = str(e.type_offset, e.type_length); diff --git a/src/ifcviewer/ViewportCore.h b/src/ifcviewer/ViewportCore.h index 51ce2ff85b..72ff28ce16 100644 --- a/src/ifcviewer/ViewportCore.h +++ b/src/ifcviewer/ViewportCore.h @@ -544,8 +544,10 @@ public: // Per-model progress for a federation loading UI. count() is how many // models have metadata (are in the scene); progress(idx,…) gives the - // idx-th model's resident/total chunks, ordered by session_model_id (= load order) - // so each model keeps a stable UI slot as it streams. + // idx-th model's resident/total chunks, ordered by session_model_id — which + // is minted when a load is REQUESTED, so this is the order the host asked + // for its models, not the order their reads finished. Each model keeps a + // stable UI slot as it streams. int streamingModelCount() const; void streamingModelProgress(int idx, int& resident_chunks, int& total_chunks) const; @@ -556,11 +558,17 @@ public: int modelLoadIndex(std::uint32_t session_model_id) const; // One row of the element table: the IFC identity behind a rendered - // object_id. `model_index` is the load-order slot (modelLoadIndex), so a - // host UI can attribute an object to the file it came from. + // object_id, plus which model it came from, said two ways. + // + // `model_index` is the load-order slot (modelLoadIndex) — a POSITION, so it + // shifts if an earlier model fails to load. `source_id` is the JS byte-source + // the model was added from (-1 when it came from somewhere else), which the + // host minted itself and which never moves. Prefer the latter for + // attributing an object to a file; the index is for UI slots. struct ElementRef { std::uint32_t object_id = 0; int model_index = -1; + int source_id = -1; std::string guid; std::string name; std::string type; @@ -1044,9 +1052,10 @@ public: private: bool createPool(); - // The scene's models in load order (ascending session_model_id). Every - // per-model API indexes against this, so a model keeps a stable UI slot - // instead of hopping with unordered_map iteration order. + // The scene's models in load order (ascending session_model_id, minted at + // request time — see loadSidecarMetadataWeb). Every per-model API indexes + // against this, so a model keeps a stable UI slot instead of hopping with + // unordered_map iteration order. std::vector modelIdsInLoadOrder() const; public: