Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add height and width measurement tools #997

Open
wants to merge 26 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 22 commits
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
29fe260
Add MeasureHeightTool to measure-tools package
saaaaaally Jul 15, 2024
dd016be
Add MeasureWidthTool to measure-tools package
saaaaaally Jul 15, 2024
ab0fc01
Fix icons
saaaaaally Jul 16, 2024
5f7aa1a
pnpm change
saaaaaally Jul 16, 2024
16009ce
Merge branch 'master' into saally/AddHeightTool
saaaaaally Jul 16, 2024
79c0ecd
Fix error
saaaaaally Jul 17, 2024
6977efa
Merge branch 'master' into saally/AddHeightTool
saaaaaally Jul 17, 2024
15abc6b
Merge branch 'master' into saally/AddHeightTool
saaaaaally Jul 22, 2024
9ea6e76
Merge branch 'master' into saally/AddHeightTool
saaaaaally Jul 23, 2024
2cf0245
Fix pr comments
saaaaaally Jul 29, 2024
7b93d6e
Merge branch 'master' into saally/AddHeightTool
saaaaaally Jul 29, 2024
1aff6ec
Add PerpendicularDistanceMeasurementSerializer & fix pr comments
saaaaaally Aug 2, 2024
e67eb56
Merge branch 'master' into saally/AddHeightTool
saaaaaally Aug 2, 2024
5517257
Move secondary line calculation to onMouseMotion
saaaaaally Aug 21, 2024
463af55
Merge branch 'master' into saally/AddHeightTool
saaaaaally Aug 21, 2024
99901ee
Fix merge error
saaaaaally Aug 21, 2024
2ad84e2
Pr comment fix
saaaaaally Aug 26, 2024
76e5f2a
Remove secondaryLine setter
saaaaaally Aug 26, 2024
b030042
Fix pr comments
saaaaaally Aug 28, 2024
9a76d73
Merge branch 'master' into saally/AddHeightTool
saaaaaally Aug 28, 2024
8a56391
Add PerpendicularDistanceMeasurement tests
saaaaaally Aug 29, 2024
68b802b
Merge branch 'master' into saally/AddHeightTool
saaaaaally Aug 30, 2024
04354a4
Fix comments
saaaaaally Sep 4, 2024
c88347c
Merge branch 'master' into saally/AddHeightTool
saaaaaally Sep 4, 2024
3709bbe
Fix pr comments
saaaaaally Sep 10, 2024
a0724c2
Merge branch 'master' into saally/AddHeightTool
saaaaaally Sep 10, 2024
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
2 changes: 1 addition & 1 deletion apps/test-viewer/src/UiProvidersConfig.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -213,7 +213,7 @@ const configuredUiItems = new Map<string, UiItem>([
"measure-tools",
{
initialize: async () => {
await MeasureTools.startup();
await MeasureTools.startup({ featureFlags: { showWidthTool: true, showHeightTool: true } });
MeasurementActionToolbar.setDefaultActionProvider();
},
createUiItemsProviders: () => [new MeasureToolsUiItemsProvider()],
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"type": "minor",
"comment": "Add height and width measurement tools.",
"packageName": "@itwin/measure-tools-react",
"email": "88331924+saaaaaally@users.noreply.github.com",
"dependentChangeType": "patch"
}
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,20 @@
"mainInstruction": "Enter point to measure from",
"mainInstruction2": "Enter point to measure to"
},
"MeasureHeight": {
"keyin": "Height",
"flyover": "Height",
"description": "Create height measurements.",
"height": "Height:",
"toolTitle": "Height Measurement [{0}]"
},
"MeasureWidth": {
"keyin": "Width",
"flyover": "Width",
"description": "Create width measurements.",
"width": "Width:",
"toolTitle": "Width Measurement [{0}]"
},
"MeasureArea": {
"keyin": "Area",
"flyover": "Area",
Expand Down Expand Up @@ -148,4 +162,4 @@
"label": "Measurement",
"tooltip": "Measurement Tools"
}
}
}
10 changes: 10 additions & 0 deletions packages/itwin/measure-tools/src/MeasureTools.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@ import { MeasureAngleTool } from "./tools/MeasureAngleTool";
import { MeasurePerpendicularTool } from "./tools/MeasurePerpendicularTool";
import type { Localization } from "@itwin/core-common";
import { DrawingDataCache } from "./api/DrawingTypeDataCache";
import { MeasureHeightTool } from "./tools/MeasureHeightTool";
import { MeasureWidthTool } from "./tools/MeasureWidthTool";

export interface FeatureFlags {
hideDistanceTool?: boolean;
Expand All @@ -24,6 +26,8 @@ export interface FeatureFlags {
hideAngleTool?: boolean;
hidePerpendicularTool?: boolean;
hideToggleDisplayAxesTool?: boolean;
showHeightTool?: boolean;
bsy-nicholasw marked this conversation as resolved.
Show resolved Hide resolved
showWidthTool?: boolean;
}

export interface StartupOptions {
Expand Down Expand Up @@ -86,6 +90,12 @@ export class MeasureTools {
if (!featureFlags?.hideToggleDisplayAxesTool) {
toolsToRegister.push(ToggleDisplayMeasurementAxesTool);
}
if (featureFlags?.showHeightTool) {
toolsToRegister.push(MeasureHeightTool);
}
if (featureFlags?.showWidthTool) {
toolsToRegister.push(MeasureWidthTool);
}
if (toolsToRegister.length > 0) {
toolsToRegister.push(ClearMeasurementsTool);
}
Expand Down
2 changes: 2 additions & 0 deletions packages/itwin/measure-tools/src/measure-tools-react.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,8 @@ export * from "./tools/ClearMeasurementsTool";
export * from "./tools/MeasureAngleTool";
export * from "./tools/MeasureAreaTool";
export * from "./tools/MeasureDistanceTool";
export * from "./tools/MeasureHeightTool";
export * from "./tools/MeasureWidthTool";
export * from "./tools/MeasureLocationTool";
export * from "./tools/MeasurePerpendicularTool";
export * from "./tools/MeasureRadiusTool";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -232,7 +232,7 @@ export class DistanceMeasurement extends Measurement {
);
}

private getSnapId(): string | undefined {
protected getSnapId(): string | undefined {
if (!this.transientId)
this.transientId = MeasurementSelectionSet.nextTransientId;

Expand Down Expand Up @@ -334,7 +334,7 @@ export class DistanceMeasurement extends Measurement {
return Point3d.createAdd2Scaled(clampedStartPoint, 0.5, clampedEndPoint, 0.5);
}

private buildRunRiseAxes(): void {
protected buildRunRiseAxes(): void {
this._runRiseAxes = [];

if (this.isAxis) return;
Expand Down Expand Up @@ -428,7 +428,7 @@ export class DistanceMeasurement extends Measurement {
this._textMarker.applyStyle(tStyle);
}

private async createTextMarker(): Promise<void> {
protected async createTextMarker(): Promise<void> {
const lengthSpec =
await IModelApp.quantityFormatter.getFormatterSpecByQuantityType(
QuantityType.LengthEngineering
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,231 @@
/*---------------------------------------------------------------------------------------------
* Copyright (c) Bentley Systems, Incorporated. All rights reserved.
* See LICENSE.md in the project root for license terms and full copyright notice.
*--------------------------------------------------------------------------------------------*/

import { Point3d, type XYAndZ, type XYZProps } from "@itwin/core-geometry";
import { ColorDef, LinePixels } from "@itwin/core-common";
import type { DecorateContext } from "@itwin/core-frontend";
import { GraphicType, IModelApp, QuantityType } from "@itwin/core-frontend";
import { Measurement, type MeasurementEqualityOptions, type MeasurementWidgetData } from "../api/Measurement";
import { MeasureTools } from "../MeasureTools";
import { DistanceMeasurement, type DistanceMeasurementProps, DistanceMeasurementSerializer } from "./DistanceMeasurement";
import type { MeasurementProps } from "../api/MeasurementProps";

export enum PerpendicularMeasurementType {
Width = "width",
Height = "height",
}

/**
* Props for serializing a [[PerpendicularDistanceMeasurement]].
*/
export interface PerpendicularDistanceMeasurementProps extends DistanceMeasurementProps {
measurementType: PerpendicularMeasurementType;
secondaryLine?: XYZProps[];
}

/** Serializer for a [[PerpendicularDistanceMeasurement]]. */
export class PerpendicularDistanceMeasurementSerializer extends DistanceMeasurementSerializer {
public static readonly perpendicularDistanceMeasurementName = "perpendicularDistanceMeasurement";

public override get measurementName(): string {
return PerpendicularDistanceMeasurementSerializer.perpendicularDistanceMeasurementName;
}

public override isValidType(measurement: Measurement): boolean {
return measurement instanceof PerpendicularDistanceMeasurement;
}

public override isValidJSON(json: any): boolean {
if (!super.isValidJSON(json) || !json.hasOwnProperty("startPoint") || !json.hasOwnProperty("endPoint") || !json.hasOwnProperty("measurementType")) {
Copy link
Contributor

Choose a reason for hiding this comment

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

@saaaaaally Can omit the startPoint/endPoint check since that will be taken care of in the super call.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed

return false;
}

return true;
}

protected override parseSingle(data: MeasurementProps): Measurement | undefined {
if (!this.isValidJSON(data)) return undefined;

const props = data as PerpendicularDistanceMeasurementProps;
return PerpendicularDistanceMeasurement.fromJSON(props);
}
}

export class PerpendicularDistanceMeasurement extends DistanceMeasurement {
bsy-nicholasw marked this conversation as resolved.
Show resolved Hide resolved
bsy-nicholasw marked this conversation as resolved.
Show resolved Hide resolved
bsy-nicholasw marked this conversation as resolved.
Show resolved Hide resolved
public static override readonly serializer = Measurement.registerSerializer(new PerpendicularDistanceMeasurementSerializer());

private _measurementType?: PerpendicularMeasurementType;
private _secondaryLine?: Point3d[];

public get measurementType(): PerpendicularMeasurementType {
return this._measurementType ?? PerpendicularMeasurementType.Height;
}
public set measurementType(t: PerpendicularMeasurementType) {
this._measurementType = t;
}

public override setStartPoint(point: XYAndZ) {
const heightPoints = this.getHeightPoints([Point3d.createFrom(point), this.endPointRef]);
let newStartPoint = point;
if (this._measurementType === PerpendicularMeasurementType.Width) {
newStartPoint = heightPoints[1];
}
super.setStartPoint(newStartPoint);
this.updateSecondaryLine(heightPoints);
}

public override setEndPoint(point: XYAndZ, customStartPoint?: Point3d) {
const heightPoints = this.getHeightPoints([customStartPoint ?? this.startPointRef, Point3d.createFrom(point)]);
let newEndPoint = point;
if (this._measurementType === PerpendicularMeasurementType.Height) {
newEndPoint = heightPoints[1];
}
super.setEndPoint(newEndPoint);
this.updateSecondaryLine(heightPoints);
}

private updateSecondaryLine = (heightPoints: Point3d[]) => {
switch (this._measurementType) {
case PerpendicularMeasurementType.Width:
this._secondaryLine = [heightPoints[0], heightPoints[1]];
break;
case PerpendicularMeasurementType.Height:
this._secondaryLine = [heightPoints[1], heightPoints[2]];
break;
}
};

/**
* Returns the points for the base and perpendicular lines of a right triangle formed from the hypotenuse.
*/
private getHeightPoints(hypotenusePoints: Point3d[]): Point3d[] {
const heightPoints: Point3d[] = [];
heightPoints.push(hypotenusePoints[0].clone());
if (hypotenusePoints[0].z > hypotenusePoints[1].z) {
heightPoints.push(new Point3d(hypotenusePoints[0].x, hypotenusePoints[0].y, hypotenusePoints[1].z));
} else {
heightPoints.push(new Point3d(hypotenusePoints[1].x, hypotenusePoints[1].y, hypotenusePoints[0].z));
}
heightPoints.push(hypotenusePoints[1].clone());

return heightPoints;
}

public static override create(start: Point3d, end: Point3d, viewType?: string) {
// Don't ned to serialize the points, will just work as is
const measurement = new PerpendicularDistanceMeasurement({ startPoint: start, endPoint: end });
if (viewType) {
measurement.viewTarget.include(viewType);
}

return measurement;
}

public override decorate(context: DecorateContext): void {
super.decorate(context);

if (this._secondaryLine && this._secondaryLine.length > 0) {
const secondaryLine = context.createGraphicBuilder(GraphicType.WorldOverlay, undefined, this.getSnapId());
secondaryLine.setSymbology(ColorDef.white, ColorDef.black, 1, LinePixels.Code5);
secondaryLine.addLineString(this._secondaryLine);
context.addDecorationFromBuilder(secondaryLine);
}
}

protected override async getDataForMeasurementWidgetInternal(): Promise<MeasurementWidgetData> {
bsy-nicholasw marked this conversation as resolved.
Show resolved Hide resolved
const distanceData = await super.getDataForMeasurementWidgetInternal();

const lengthSpec = await IModelApp.quantityFormatter.getFormatterSpecByQuantityType(QuantityType.LengthEngineering);
const distance = this.worldScale * this.startPointRef.distance(this.endPointRef);
const fDistance = IModelApp.quantityFormatter.formatQuantity(distance, lengthSpec);

const title =
this._measurementType === PerpendicularMeasurementType.Height
? MeasureTools.localization.getLocalizedString("MeasureTools:tools.MeasureHeight.toolTitle").replace("{0}", fDistance)
Copy link
Contributor

Choose a reason for hiding this comment

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

@saaaaaally iTwin localization allows passing arguments when getting the localized string, e.g. instead of {0} you can have it as {{value}} and pass in a json object with { value: fDistance }. that's more robust than doing a string replace here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

Copy link
Contributor

Choose a reason for hiding this comment

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

@saaaaaally You don't need to do the string replace still, getLocalizedString's second argument is a TranslationOptions, and the localization library will replace {{value}} automatically when you pass in {value: fDistance}.

https://www.itwinjs.org/learning/frontend/localization/

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed

: MeasureTools.localization.getLocalizedString("MeasureTools:tools.MeasureWidth.toolTitle").replace("{0}", fDistance);

const label =
this._measurementType === PerpendicularMeasurementType.Height
? MeasureTools.localization.getLocalizedString("MeasureTools:tools.MeasureHeight.height")
: MeasureTools.localization.getLocalizedString("MeasureTools:tools.MeasureWidth.width");

distanceData.title = title;
distanceData.properties[0].label = label;
bsy-nicholasw marked this conversation as resolved.
Show resolved Hide resolved
return distanceData;
}

/**
* Tests equality with another measurement.
* @param other Measurement to test equality for.
* @param opts Options for equality testing.
* @returns true if the other measurement is equal, false if some property is not the same or if the measurement is not of the same type.
*/
public override equals(other: Measurement, opts?: MeasurementEqualityOptions): boolean {
if (!super.equals(other, opts)) return false;

// Compare data (ignore isDynamic)
const tol = opts ? opts.tolerance : undefined;
bsy-nicholasw marked this conversation as resolved.
Show resolved Hide resolved
const otherDist = other as PerpendicularDistanceMeasurement;
if (this._measurementType !== otherDist.measurementType) {
return false;
}
if (this._secondaryLine && otherDist._secondaryLine) {
if (this._secondaryLine.length !== otherDist._secondaryLine.length) {
return false;
}
for (let i = 0; i < this._secondaryLine.length; i++) {
if (!this._secondaryLine[i].isAlmostEqual(otherDist._secondaryLine[i], tol)) {
return false;
}
}
}
return true;
}

/**
* Copies data from the other measurement into this instance.
* @param other Measurement to copy property values from.
*/
protected override copyFrom(other: Measurement) {
super.copyFrom(other);

if (other instanceof PerpendicularDistanceMeasurement) {
bsy-nicholasw marked this conversation as resolved.
Show resolved Hide resolved
this.measurementType = other.measurementType;
this._secondaryLine = other._secondaryLine;
}
}

/**
* Deserializes properties (if they exist) from the JSON object.
* @param json JSON object to read data from.
*/
protected override readFromJSON(json: MeasurementProps) {
super.readFromJSON(json);

const jsonDist = json as PerpendicularDistanceMeasurementProps;
bsy-nicholasw marked this conversation as resolved.
Show resolved Hide resolved
if (jsonDist.measurementType !== undefined) {
this.measurementType = jsonDist.measurementType;
}
if (jsonDist.secondaryLine !== undefined) {
this._secondaryLine = jsonDist.secondaryLine.map((p) => Point3d.fromJSON(p));
}
}

/**
* Serializes properties to a JSON object.
* @param json JSON object to append data to.
*/
protected override writeToJSON(json: MeasurementProps) {
super.writeToJSON(json);

const jsonDist = json as PerpendicularDistanceMeasurementProps;
Copy link
Contributor

Choose a reason for hiding this comment

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

@saaaaaally Similar to the other comments, super.writeToJSON will already have written out startPoint/endPoint/showAxes to the json object.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

Copy link
Contributor

Choose a reason for hiding this comment

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

@saaaaaally Should convert this._secondaryLine to an array of XYZProps, calling Point3d's toJSON method. While technically it's valid as written (Point3d instances can mimic XYZProps), it's still good practice to make sure the returned JSON is only primitive data + separate object instances from the original object, to allow callers to mutate it without side effects.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

Copy link
Contributor

Choose a reason for hiding this comment

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

@saaaaaally Using as here doesn't do anything more than satisfy the type checker, which was already satisfied by Point3d being the same shape as XYZProps.

I was suggesting something more like

this._secondaryLine ? this._secondaryLine.map((p) => p.toJSON()) : undefined

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed

jsonDist.measurementType = this.measurementType;
jsonDist.secondaryLine = this._secondaryLine;
}

public static override fromJSON(data: PerpendicularDistanceMeasurementProps): PerpendicularDistanceMeasurement {
return new PerpendicularDistanceMeasurement(data);
}
}
Loading
Loading