Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion apps/viewer/src/components/viewer/GLBExportDialog.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -56,14 +56,15 @@ type ColorSource = 'rendering' | 'shading';
* matches what the user sees in the viewport.
*/
function buildHiddenIfcTypes(
typeVisibility: { spaces: boolean; spatialZones: boolean; openings: boolean; virtualElements: boolean; site: boolean },
typeVisibility: { spaces: boolean; spatialZones: boolean; openings: boolean; virtualElements: boolean; site: boolean; ifcAnnotations: boolean },
): Set<string> {
const out = new Set<string>();
if (!typeVisibility.spaces) out.add('IfcSpace');
if (!typeVisibility.spatialZones) out.add('IfcSpatialZone');
if (!typeVisibility.openings) out.add('IfcOpeningElement');
if (!typeVisibility.virtualElements) out.add('IfcVirtualElement');
if (!typeVisibility.site) out.add('IfcSite');
if (!typeVisibility.ifcAnnotations) out.add('IfcAnnotation');
return out;
}

Expand Down
9 changes: 8 additions & 1 deletion apps/viewer/src/components/viewer/ViewportContainer.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -699,6 +699,7 @@ export function ViewportContainer() {
prevVis.openings !== typeVisibility.openings ||
prevVis.virtualElements !== typeVisibility.virtualElements ||
prevVis.site !== typeVisibility.site ||
prevVis.ifcAnnotations !== typeVisibility.ifcAnnotations ||
filteredTypeModeRef.current !== effectiveViewMode;
const sourceChanged = filteredSourceRef.current !== allMeshes;
if (typeVisChanged || sourceChanged || allMeshes.length < filteredSourceLenRef.current) {
Expand All @@ -709,7 +710,7 @@ export function ViewportContainer() {
filteredTypeModeRef.current = effectiveViewMode;
}

const needsFilter = !typeVisibility.spaces || !typeVisibility.spatialZones || !typeVisibility.openings || !typeVisibility.virtualElements || !typeVisibility.site;
const needsFilter = !typeVisibility.spaces || !typeVisibility.spatialZones || !typeVisibility.openings || !typeVisibility.virtualElements || !typeVisibility.site || !typeVisibility.ifcAnnotations;
const prevCacheLen = cache.length;

// Only process NEW meshes since last run — O(batch_size) not O(total)
Expand Down Expand Up @@ -739,6 +740,12 @@ export function ViewportContainer() {
if (ifcType === 'IfcOpeningElement' && !typeVisibility.openings) continue;
if (ifcType === 'IfcVirtualElement' && !typeVisibility.virtualElements) continue;
if (ifcType === 'IfcSite' && !typeVisibility.site) continue;
// IfcAnnotation can carry real 3D solid geometry (e.g. Bonsai
// plan-view "DRAWING" boxes) on top of the 2D symbolic curve overlay.
// The `ifcAnnotations` toggle drives the curve overlay (Viewport.tsx);
// honour it here too so the toggle also hides those 3D meshes instead
// of leaving them rendered as stray cubes (issue #1354).
if (ifcType === 'IfcAnnotation' && !typeVisibility.ifcAnnotations) continue;
}

// Mesh alpha flows through unchanged. The previous code re-multiplied
Expand Down
38 changes: 38 additions & 0 deletions apps/viewer/src/store/basketVisibleSet.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -255,6 +255,44 @@ describe('basketVisibleSet', () => {
});
});

describe('type visibility: IfcAnnotation (issue #1354)', () => {
const meshes = [
{ expressId: 1, ifcType: 'IfcWall' },
{ expressId: 2, ifcType: 'IfcAnnotation' },
];

it('includes IfcAnnotation 3D meshes when the toggle is on', () => {
useViewerStore.setState({
selectedEntitiesSet: new Set(),
selectedEntity: null,
selectedEntityIds: new Set(),
hierarchyBasketSelection: new Set(),
geometryResult: { meshes } as any,
typeVisibility: { ...useViewerStore.getState().typeVisibility, ifcAnnotations: true },
});
invalidateVisibleBasketCache();

const refs = getVisibleBasketEntityRefsFromStore();
assert.ok(refs.some((r) => entityRefToString(r) === 'legacy:2'));
});

it('drops IfcAnnotation 3D meshes when the toggle is off', () => {
useViewerStore.setState({
selectedEntitiesSet: new Set(),
selectedEntity: null,
selectedEntityIds: new Set(),
hierarchyBasketSelection: new Set(),
geometryResult: { meshes } as any,
Comment on lines +270 to +285

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📐 Maintainability & Code Quality | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== GeometryResult declarations =="
rg -n -C2 "export (type|interface) GeometryResult|type GeometryResult|interface GeometryResult" apps/viewer/src -g '**/*.ts' -g '**/*.tsx'

echo
echo "== New casts in basketVisibleSet.test.ts =="
rg -n -C2 "geometryResult: \{ meshes \} as any|as any" apps/viewer/src/store/basketVisibleSet.test.ts

echo
echo "== Existing typed geometry fixtures in tests =="
rg -n -C2 "GeometryResult|geometryResult:" apps/viewer/src -g '**/*.test.ts' -g '**/*.test.tsx'

Repository: LTplus-AG/ifc-lite

Length of output: 15800


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== basketVisibleSet.test.ts around the casts =="
sed -n '1,360p' apps/viewer/src/store/basketVisibleSet.test.ts

echo
echo "== GeometryResult / MeshData declarations =="
fd -a 'index.ts' apps -t f | rg '`@ifc-lite/geometry`|geometry' || true
rg -n -C3 "export (type|interface) GeometryResult|export (type|interface) MeshData|type GeometryResult|interface GeometryResult|type MeshData|interface MeshData" . -g '**/*.ts' -g '**/*.tsx'

echo
echo "== Where geometryResult is typed in viewer store types =="
rg -n -C3 "geometryResult" apps/viewer/src/store -g '**/*.ts' -g '**/*.tsx'

Repository: LTplus-AG/ifc-lite

Length of output: 50374


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== basketVisibleSet.test.ts around the casts =="
sed -n '1,360p' apps/viewer/src/store/basketVisibleSet.test.ts

echo
echo "== GeometryResult / MeshData declarations =="
rg -n -C3 "export (type|interface) GeometryResult|export (type|interface) MeshData|type GeometryResult|interface GeometryResult|type MeshData|interface MeshData" . -g '**/*.ts' -g '**/*.tsx'

echo
echo "== Where geometryResult is typed in viewer store types =="
rg -n -C3 "geometryResult" apps/viewer/src/store -g '**/*.ts' -g '**/*.tsx'

Repository: LTplus-AG/ifc-lite

Length of output: 50374


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== basketVisibleSet.test.ts around the casts =="
sed -n '1,360p' apps/viewer/src/store/basketVisibleSet.test.ts

echo
echo "== GeometryResult / MeshData declarations =="
rg -n -C3 "export (type|interface) GeometryResult|export (type|interface) MeshData|type GeometryResult|interface GeometryResult|type MeshData|interface MeshData" . -g '**/*.ts' -g '**/*.tsx'

echo
echo "== Where geometryResult is typed in viewer store types =="
rg -n -C3 "geometryResult" apps/viewer/src/store -g '**/*.ts' -g '**/*.tsx'

Repository: LTplus-AG/ifc-lite

Length of output: 50374


Replace the geometryResult casts with a typed fixture/helper

apps/viewer/src/store/basketVisibleSet.test.ts uses as any for geometryResult in several places. That bypasses the GeometryResult shape and makes the test brittle if the fixture drifts. Use a small typed helper/fixture instead.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@apps/viewer/src/store/basketVisibleSet.test.ts` around lines 270 - 285, The
tests are bypassing the GeometryResult type by casting geometryResult as any in
basketVisibleSet.test.ts, which makes the fixture brittle. Replace those ad hoc
casts with a small typed helper or shared fixture that returns a correctly
shaped geometryResult for the relevant test cases, and use it wherever setState
is populating geometryResult in this spec.

Source: Coding guidelines

typeVisibility: { ...useViewerStore.getState().typeVisibility, ifcAnnotations: false },
});
invalidateVisibleBasketCache();

const refs = getVisibleBasketEntityRefsFromStore();
assert.ok(refs.some((r) => entityRefToString(r) === 'legacy:1'));
assert.ok(!refs.some((r) => entityRefToString(r) === 'legacy:2'));
});
});

describe('federation: unresolved globalId in multi-model', () => {
it('getBasketSelectionRefsFromStore returns array when models exist', () => {
useViewerStore.setState({
Expand Down
3 changes: 3 additions & 0 deletions apps/viewer/src/store/basketVisibleSet.ts
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,9 @@ function matchesTypeVisibility(ifcType: string | undefined, typeVisibility: View
if (ifcType === 'IfcOpeningElement' && !typeVisibility.openings) return false;
if (ifcType === 'IfcVirtualElement' && !typeVisibility.virtualElements) return false;
if (ifcType === 'IfcSite' && !typeVisibility.site) return false;
// IfcAnnotation 3D mesh geometry (e.g. Bonsai plan-view boxes) tracks the
// same toggle that hides the 2D symbolic curve overlay (issue #1354).
if (ifcType === 'IfcAnnotation' && !typeVisibility.ifcAnnotations) return false;
return true;
}

Expand Down
Loading