Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
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
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"type": "patch",
"comment": "Fix Measure Distance Tool: Incorrect distance, rise, and slope calculations in sheets with vertical exaggeration",
"packageName": "@itwin/measure-tools-react",
"email": "[email protected]",
"dependentChangeType": "patch"
}
Original file line number Diff line number Diff line change
Expand Up @@ -433,9 +433,9 @@ export class DistanceMeasurement extends Measurement {
await IModelApp.quantityFormatter.getFormatterSpecByQuantityType(
QuantityType.LengthEngineering
);
const distance = this._startPoint.distance(this._endPoint);
const { distance } = this.getDistances();
const fDistance = IModelApp.quantityFormatter.formatQuantity(
distance * this.worldScale,
distance,
lengthSpec
);

Expand Down Expand Up @@ -468,15 +468,45 @@ export class DistanceMeasurement extends Measurement {
return true;
}

private getDistances(): { distance: number, run: number, rise: number } {
if (this.drawingMetadata?.sheetToWorldTransform) {
// calculate distances in sheet coordinates
const adjustedStartPoint = this.adjustPointWithSheetToWorldTransform(this.adjustPointForGlobalOrigin(this.startPointRef.clone()));
Copy link
Contributor

Choose a reason for hiding this comment

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

Assuming startPointRef is in 'drawing' coordinates, shouldn't we first adjust the point with the sheet to world, THEN adjust the point for global origin?

Copy link
Author

Choose a reason for hiding this comment

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

The transformation order follows the approach used in the LocationMeasurement tool, which has been stable and reliable (Ref: LocationMeasurement.ts#L290).

Since this method was originally implemented by Maxime, I believe he would be the best person to clarify the reasoning behind choosing this specific order.

@Maxime-Brassard, could you provide some insights on why adjustPointForGlobalOrigin is applied before sheetToWorldTransform in this tool?

Thanks!

Copy link
Contributor

Choose a reason for hiding this comment

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

I believe Alex is right, I'd test it and see if results make sense.
I think the adjustment for global coordinate should be made on the 3d points.
If it's not that way in LocationMeasurement, it's probably an oversight and an coincidence that it worked so far.
I'd test it and change it across measurements if everything looks fine.

const adjustedEndPointFinal = this.adjustPointWithSheetToWorldTransform(this.adjustPointForGlobalOrigin(this.endPointRef.clone()));
const directDistanceFinal = adjustedStartPoint.distance(adjustedEndPointFinal);

// Project End Point Vertically (for Horizontal Distance Calculation)
const projectedEndPointX = Point3d.create(this.endPointRef.x, this.startPointRef.y, this.startPointRef.z);
const adjustedEndPointX = this.adjustPointWithSheetToWorldTransform(this.adjustPointForGlobalOrigin(projectedEndPointX));
const horizontalDistance = adjustedStartPoint.distance(adjustedEndPointX);


// Project End Point Horizontally (for Vertical Distance Calculation)
const projectedEndPointY = Point3d.create(this.startPointRef.x, this.endPointRef.y, this.startPointRef.z);
const adjustedEndPointY = this.adjustPointWithSheetToWorldTransform(this.adjustPointForGlobalOrigin(projectedEndPointY));
const verticalDistance = adjustedEndPointY.minus(adjustedStartPoint).z;
return {
distance: directDistanceFinal,
run: horizontalDistance,
rise: verticalDistance,
};
}

// calculate distances in world coordinates
return {
distance: this._startPoint.distance(this._endPoint),
run: this._startPoint.distanceXY(this._endPoint),
rise: this._endPoint.z - this._startPoint.z,
};
}

protected override async getDataForMeasurementWidgetInternal(): Promise<MeasurementWidgetData> {
const lengthSpec =
await IModelApp.quantityFormatter.getFormatterSpecByQuantityType(
QuantityType.LengthEngineering
);

const distance = this.worldScale * this._startPoint.distance(this._endPoint);
const run = this.drawingMetadata?.worldScale !== undefined ? this.worldScale * Math.abs(this._endPoint.x - this._startPoint.x): this._startPoint.distanceXY(this._endPoint);
const rise = this.drawingMetadata?.worldScale !== undefined ? this.worldScale * (this._endPoint.y - this._startPoint.y): this._endPoint.z - this._startPoint.z;
const { distance, run, rise } = this.getDistances();
const slope = 0.0 < run ? (100 * rise) / run : 0.0;
const dx = Math.abs(this._endPoint.x - this._startPoint.x);
const dy = Math.abs(this._endPoint.y - this._startPoint.y);
Expand Down Expand Up @@ -547,7 +577,7 @@ export class DistanceMeasurement extends Measurement {
},
);

if (this.drawingMetadata?.worldScale === undefined) {
if (this.drawingMetadata?.sheetToWorldTransform === undefined) {
data.properties.push(
{
label: MeasureTools.localization.getLocalizedString(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,7 @@ MeasureDistanceToolModel
this.toolModel.sheetViewId = ev.viewport.view.id;

if (drawingInfo?.drawingId !== undefined && drawingInfo.origin !== undefined && drawingInfo.worldScale !== undefined) {
const data: DrawingMetadata = { origin: drawingInfo.origin, drawingId: drawingInfo.drawingId, worldScale: drawingInfo.worldScale, extents: drawingInfo.extents};
const data: DrawingMetadata = { origin: drawingInfo.origin, drawingId: drawingInfo.drawingId, worldScale: drawingInfo.worldScale, extents: drawingInfo.extents, sheetToWorldTransform: drawingInfo.sheetToWorldTransform, sheetToProfileTransform: drawingInfo.sheetToProfileTransform};
this.toolModel.drawingMetadata = data;
}
}
Expand Down
Loading