Skip to content

One IFCGRID without ObjectPlacement (schema-optional) throws in getAbsolutePlacement — the catch in GridReader.read then drops EVERY grid in the file, silently #263

Description

@stokuewe

Versions

@thatopen/fragments 3.4.7 (verified against the published dist/index.mjs; source on main: packages/fragments/src/Importers/IfcImporter/src/geometry/grid-reader.ts, packages/fragments/src/Utils/ifc-utils.ts).

What happens

ObjectPlacement is OPTIONAL on IfcProduct, so IFCGRID('…',#15,'Achsraster',$,$,$,$,(…),(…),$) is schema-valid IFC. Convert a file that contains one such grid and:

  • IfcImporter.process(...) completes normally — no thrown error, no rejected promise, no return-value signal;
  • the resulting fragments buffer contains no grid data at all — the ThatOpenGrid category is absent from getCategories(), and model.getGrids() yields an empty group;
  • every other grid in the same file is lost too, including fully placed ones;
  • element geometry is unaffected, so the file looks perfectly converted.

The only trace is a console.error inside the importer, which in a worker-based conversion nobody reads.

Why

FragmentsIfcUtils.getAbsolutePlacement dereferences the optional attribute unconditionally:

// packages/fragments/src/Utils/ifc-utils.ts
static getAbsolutePlacement(webIfc, item, unitsFactor = this.getUnitsFactor(webIfc)) {
  const placementId = item.ObjectPlacement.value;   // ← TypeError when the IFC omits it
  
}

TypeError: Cannot read properties of null (reading 'value').

GridReader.read wraps the whole grid loop in one try/catch that swallows the error and returns an empty list, so the failure of a single grid destroys the result for the entire file:

// packages/fragments/src/Importers/IfcImporter/src/geometry/grid-reader.ts
read(webIfc: WEBIFC.IfcAPI) {
  try {
    
    for (let i = 0; i < size; i++) {
      const grid = webIfc.GetLine(0, id);
      const transform = FragmentsIfcUtils.getAbsolutePlacement(webIfc, grid, units);   // throws here
      
      result.push(data);
    }
    return result;
  } catch (error) {
    console.error(error);
    return [] as GridData[];       // ← all-or-nothing
  }
}

Note that this is the same all-or-nothing failure that was already fixed one line below, for the other optional attribute on this path. The comment sitting in the current source says so:

// AxisTag is an optional IfcLabel. web-ifc returns it as null when the
// IFC omits it (IFCGRIDAXIS($,...)), so read it defensively; otherwise
// a single tagless axis threw and the outer catch dropped every grid.
tag: axisCurve.AxisTag?.value ?? "",

ObjectPlacement has exactly that shape (optional attribute, null from web-ifc, dereferenced without ?.) and exactly that blast radius, and it is still open. The defensive read fixed the symptom for one attribute; the structural cause — one catch around the whole loop — is untouched.

Reproduction sketch

Hand-authored IFC2X3, measured on fragments 3.4.7:

  1. Start from a file with a working grid (IFCGRID with an IFCLOCALPLACEMENT, two UAxes, two VAxes, all IFCPOLYLINE). Convert → getCategories() contains ThatOpenGrid, the grid data reads back with four tagged axes.
  2. Add a second IFCGRID whose ObjectPlacement is $, changing nothing else.
  3. Convert the same way:
const importer = new IfcImporter();
const bytes = await importer.process({ bytes: ifcBytes });   // resolves normally
  1. Load the result. Observed: getCategories() still lists IFCGRID (the entity is there) but not ThatOpenGrid; the grid item count is 0; the wall in the same file keeps its correct bounding box.

Suggested fix

Both halves are worth fixing, and neither alone is sufficient:

  1. Tolerate the omitted placement. An IfcProduct without ObjectPlacement is not "broken", it is a product with no placement — i.e. the identity placement. So:

    const placementId = item.ObjectPlacement?.value;
    if (placementId === undefined || placementId === null) {
      // no placement → identity, still with the IFC→three axis conversion applied
      const ifcResult = new THREE.Matrix4();
      ifcResult.premultiply(new THREE.Matrix4().makeRotationX(-Math.PI / 2));
      return ifcResult;
    }
  2. Move the try/catch inside the per-grid loop in GridReader.read, so one unreadable grid costs one grid instead of all of them — and report it with the express ID (console.warn(\Fragments: skipping grid #${id}`, error)`) rather than a bare stack for the whole file.

With (1) alone a differently-malformed grid would still wipe the file; with (2) alone a schema-valid grid would still be silently dropped.

Related: #244 (the AxisTag variant of the same failure, fixed defensively), #184 / #161 (grid reading against real exporters).

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions