iTwin / itwinjs-core

Monorepo for iTwin.js Library
https://www.itwinjs.org
MIT License
600 stars 210 forks source link

Asserts thrown for pickable polyface decorations #3682

Closed ImVeryLost closed 2 years ago

ImVeryLost commented 2 years ago

Describe the bug In the latest versions of iTwin.js, asserts are being thrown for pickable polyface decorations, specifically when zooming far enough that they become "invisible". Issue is present only for pickable decorations. Managed to reproduce the issue with one of the iTwin.js test apps, with a pretty minimal setup, but it is present elsewhere as well.

To Reproduce

  1. Go to "presentation-test-app" in itwinjs-core test apps.
  2. Add the following decorator that reproduces the issue, add it to the app via IModelApp.viewManager.addDecorator:
    const decorator = new TestDecorator();
    IModelApp.viewManager.addDecorator(decorator);
  3. After starting the app and load a iModel, so a viewport is visible. There will be a small white "dot" at the center of the viewport.
  4. Zoom out. As soon as the decoration becomes small enough, the whole app freezes due to a thrown assertion: image

Decorator code


import { Id64String } from "@itwin/core-bentley";
import { ColorDef, GeometryStreamProps } from "@itwin/core-common";
import {
  DecorateContext, Decorator, GraphicBranch,
  GraphicType, HitDetail
} from "@itwin/core-frontend";
import { IModelJson, IndexedPolyface, Point3d, PolyfaceBuilder, Sphere, Transform } from "@itwin/core-geometry";

export class TestDecorator implements Decorator {
  private _elementId = "0x20000000074"; //Dummy id

  public decorate(context: DecorateContext): void {
    const branch = new GraphicBranch(true); // If i dont use a branch, issue is still present.

    let builder = context.createGraphic({
      type: GraphicType.WorldDecoration,
      pickable: {
        id: this._elementId, // If its not a pickable decoration, everything is fine.
      },
    });

    builder.setSymbology(ColorDef.white, ColorDef.white, 1);
    builder.addPolyface(this.getGraphics(), false); // If decoration is a PointString or similar, everything is fine.
    branch.add(builder.finish());
    const graphic = context.createBranch(branch, Transform.identity);
    context.addDecoration(GraphicType.WorldDecoration, graphic);
  }

  /** Creates very simple polyface. */
  private getGraphics(): IndexedPolyface {
    const polyfaceBuilder = PolyfaceBuilder.create();
    // Issue is present no matter the radius, but its easier to notice with small items.
    const sphere = Sphere.createCenterRadius(Point3d.create(0, 0, 0), 0.02);
    polyfaceBuilder.addSphere(sphere);
    return polyfaceBuilder.claimPolyface();
  }

  public getDecorationGeometry(): GeometryStreamProps | undefined {
    const geometryJson = IModelJson.Writer.toIModelJson(this.getGraphics());
    return [geometryJson];
  }

  public testDecorationHit(id: Id64String): boolean {
    return id === this._elementId;
  }

  public overrideElementHit(hit: HitDetail): boolean {
    return hit.sourceId === this._elementId;
  }
}

Expected behavior Asserts should not be thrown, or should describe the cause of the issue explicitly.

Desktop (please complete the applicable information):

Additional context Since its an assert that's being thrown, rather than a exception, it is implied that something is going wrong on my side of the code. If so, i would appreciate any pointers. Note that the issue was not present with iTwin.js 3.2.0-dev.62 and previous versions. I realize that dev versions are not stable at all, but it might help narrow down the cause.

Also, is there a way to temporarily disable asserts in dev environment? After they are thrown, the whole app freezes without any way to dismiss it and continue...

pmconne commented 2 years ago

Thanks for the detailed issue. This would have been introduced by #3598 in 3.2.0-dev.70. We'll fix it.

pmconne commented 2 years ago

The fix will become available in 3.3.0-dev.13 tonight, and in 3.2.1 which is expected to release before the end of this week.

ImVeryLost commented 2 years ago

Verified that it works with 3.3.0-dev.13, thank you for resolving this so quickly!