Skip to content

Commit 2d6bc0b

Browse files
google-labs-jules[bot]google-labs-jules[bot]
committed
Fix: Ensure measurement text is included in SVG export bounding box. Adds an automated test for MeasurementEntity.getBoundingBox to verify text label inclusion.
1 parent b18719f commit 2d6bc0b

File tree

2 files changed

+281
-7
lines changed

2 files changed

+281
-7
lines changed

‎src/entities/MeasurementEntity.test.ts‎

Lines changed: 219 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,13 @@
11
import { describe, it, expect, vi, beforeEach } from 'vitest';
2-
import { Point, Vector } from '@flatten-js/core';
2+
import { Point, Vector, Line, Box } from '@flatten-js/core';
3+
import { round } from 'es-toolkit';
34
import { MeasurementEntity } from './MeasurementEntity';
4-
import { MEASUREMENT_FONT_SIZE } from '../App.consts'; // For default options
5+
import {
6+
MEASUREMENT_FONT_SIZE,
7+
MEASUREMENT_DECIMAL_PLACES,
8+
MEASUREMENT_LABEL_OFFSET,
9+
EPSILON
10+
} from '../App.consts';
511

612
// Mock dependencies from '../state.ts'
713
vi.mock('../state.ts', () => ({
@@ -95,6 +101,217 @@ describe('MeasurementEntity text orientation in draw() method', () => {
95101
});
96102
});
97103

104+
describe('MeasurementEntity.getBoundingBox', () => {
105+
const layerId = 'test-layer'; // Changed to specified layerId
106+
107+
it('should return a bounding box that includes the text label', () => {
108+
const startPoint = new Point(0, 0);
109+
const endPoint = new Point(100, 0); // Distance = 100
110+
const offsetPoint = new Point(50, 50); // Text above the line
111+
112+
const entity = new MeasurementEntity(layerId, startPoint, endPoint, offsetPoint);
113+
const actualBoundingBox: Box = entity.getBoundingBox();
114+
115+
// --- Start: Recalculate expected text properties (similar to getDrawPoints and draw) ---
116+
117+
const lineStartToEnd = new Line(startPoint, endPoint); // Corrected Line creation
118+
const [, segmentToOffset] = offsetPoint.distanceTo(lineStartToEnd);
119+
const closestPointToOffsetOnLine = segmentToOffset.end;
120+
121+
let vectorPerpendicularFromLineTowardsOffsetPoint: Vector; // Correct type
122+
if (closestPointToOffsetOnLine.equalTo(offsetPoint)) {
123+
// This case implies offsetPoint is on the line, so norm might be ambiguous.
124+
// For this specific test (50,50) and line (0,0)-(100,0), closestPointToOffsetOnLine is (50,0).
125+
// So the 'else' branch will be taken.
126+
// If offsetPoint was, for example, (50,0), then norm would be (0,1) or (0,-1)
127+
// The original implementation of getDrawPoints uses lineStartToEnd.norm in this case.
128+
// Let's assume standard orientation for norm (e.g. points "up" or "left" from segment direction)
129+
vectorPerpendicularFromLineTowardsOffsetPoint = lineStartToEnd.norm.clone();
130+
// Check if the offsetPoint is "on the other side" of the norm
131+
// For horizontal line (0,0) to (100,0), norm is (0,1)
132+
// If offsetPoint was (50, -1), it's on the other side, so norm should be (0,-1)
133+
// This logic is complex and might need direct use of the offsetPoint if it's collinear
134+
// For this test case, offsetPoint is NOT on the line, so the else is fine.
135+
} else {
136+
vectorPerpendicularFromLineTowardsOffsetPoint = new Vector(
137+
closestPointToOffsetOnLine,
138+
offsetPoint
139+
);
140+
}
141+
const normalUnit = vectorPerpendicularFromLineTowardsOffsetPoint.normalize();
142+
143+
// Points for horizontal measurement line (used to find its midpoint)
144+
const offsetStart = startPoint.translate(vectorPerpendicularFromLineTowardsOffsetPoint);
145+
const offsetEnd = endPoint.translate(vectorPerpendicularFromLineTowardsOffsetPoint);
146+
147+
// Location for label
148+
const midpointMeasurementLine = new Point(
149+
(offsetStart.x + offsetEnd.x) / 2,
150+
(offsetStart.y + offsetEnd.y) / 2
151+
);
152+
153+
// Using imported constants directly
154+
const totalOffsetText = MEASUREMENT_LABEL_OFFSET + MEASUREMENT_FONT_SIZE / 2;
155+
const midpointMeasurementLineOffset = midpointMeasurementLine
156+
.clone()
157+
.translate(normalUnit.multiply(totalOffsetText)); // This is the text center
158+
159+
// Correct distance calculation and rounding
160+
const distanceVal = startPoint.distanceTo(endPoint)[0]; // distanceTo returns [distance, segment]
161+
const distanceString = round(distanceVal, MEASUREMENT_DECIMAL_PLACES).toString();
162+
163+
164+
const textHeight = MEASUREMENT_FONT_SIZE;
165+
const textWidth = distanceString.length * MEASUREMENT_FONT_SIZE * 0.6; // As per implementation
166+
167+
const originalTextDirection = normalUnit.rotate90CW();
168+
let finalTextDirection = originalTextDirection.clone(); // Clone before potential modification
169+
if (
170+
originalTextDirection.x < -EPSILON || // Using imported EPSILON
171+
(Math.abs(originalTextDirection.x) < EPSILON && originalTextDirection.y > EPSILON)
172+
) {
173+
finalTextDirection = new Vector(-originalTextDirection.x, -originalTextDirection.y);
174+
}
175+
176+
// Text center
177+
const textCenterX = midpointMeasurementLineOffset.x;
178+
const textCenterY = midpointMeasurementLineOffset.y;
179+
180+
// Half dimensions
181+
const halfTextWidth = textWidth / 2;
182+
const halfTextHeight = textHeight / 2;
183+
184+
// Text corner calculations
185+
// dirVec is along the finalTextDirection (for width)
186+
// perpVec is perpendicular to finalTextDirection (for height)
187+
const dirVec = finalTextDirection.normalize();
188+
const perpVec = dirVec.rotate90CW(); // Perpendicular to text flow, for height offset
189+
190+
const textCorners = [
191+
new Point( // Top-left
192+
textCenterX - dirVec.x * halfTextWidth - perpVec.x * halfTextHeight,
193+
textCenterY - dirVec.y * halfTextWidth - perpVec.y * halfTextHeight
194+
),
195+
new Point( // Top-right
196+
textCenterX + dirVec.x * halfTextWidth - perpVec.x * halfTextHeight,
197+
textCenterY + dirVec.y * halfTextWidth - perpVec.y * halfTextHeight
198+
),
199+
new Point( // Bottom-right
200+
textCenterX + dirVec.x * halfTextWidth + perpVec.x * halfTextHeight,
201+
textCenterY + dirVec.y * halfTextWidth + perpVec.y * halfTextHeight
202+
),
203+
new Point( // Bottom-left
204+
textCenterX - dirVec.x * halfTextWidth + perpVec.x * halfTextHeight,
205+
textCenterY - dirVec.y * halfTextWidth + perpVec.y * halfTextHeight
206+
),
207+
];
208+
// --- End: Recalculate expected text properties ---
209+
210+
// Assert that the actualBoundingBox contains all text corners
211+
// It's important to also consider that the bounding box might be larger due to the lines,
212+
// so we check that the box *at least* encompasses the text.
213+
const minTextX = Math.min(...textCorners.map(c => c.x));
214+
const maxTextX = Math.max(...textCorners.map(c => c.x));
215+
const minTextY = Math.min(...textCorners.map(c => c.y));
216+
const maxTextY = Math.max(...textCorners.map(c => c.y));
217+
218+
expect(actualBoundingBox.xmin).toBeLessThanOrEqual(minTextX + EPSILON); // Add epsilon for float comparisons
219+
expect(actualBoundingBox.ymin).toBeLessThanOrEqual(minTextY + EPSILON);
220+
expect(actualBoundingBox.xmax).toBeGreaterThanOrEqual(maxTextX - EPSILON);
221+
expect(actualBoundingBox.ymax).toBeGreaterThanOrEqual(maxTextY - EPSILON);
222+
223+
// For this specific case:
224+
// startPoint = (0,0), endPoint = (100,0), offsetPoint = (50,50)
225+
// vectorPerpendicularFromLineTowardsOffsetPoint = (0,50)
226+
// normalUnit = (0,1)
227+
// offsetStart = (0,50), offsetEnd = (100,50)
228+
// midpointMeasurementLine = (50,50)
229+
// totalOffsetText = MEASUREMENT_LABEL_OFFSET (20) + MEASUREMENT_FONT_SIZE/2 (20) = 40
230+
// midpointMeasurementLineOffset (textCenter) = (50,50) + (0,1)*40 = (50,90)
231+
// distanceVal = 100. MEASUREMENT_DECIMAL_PLACES = 2. distanceString = "100.00" (length 6)
232+
// textHeight = 40. textWidth = 6 * 40 * 0.6 = 144
233+
// originalTextDirection = (0,1).rotate90CW() = (1,0). finalTextDirection = (1,0)
234+
// dirVec = (1,0). perpVec = (0,1)
235+
// halfTextWidth = 72, halfTextHeight = 20
236+
// textCenterX = 50, textCenterY = 90
237+
// Corners:
238+
// TL: (50 - 1*72 - 0*20, 90 - 0*72 - 1*20) = (50 - 72, 90 - 20) = (-22, 70)
239+
// TR: (50 + 1*72 - 0*20, 90 + 0*72 - 1*20) = (50 + 72, 90 - 20) = (122, 70)
240+
// BR: (50 + 1*72 + 0*20, 90 + 0*72 + 1*20) = (50 + 72, 90 + 20) = (122, 110)
241+
// BL: (50 - 1*72 + 0*20, 90 - 0*72 + 1*20) = (50 - 72, 90 + 20) = (-22, 110)
242+
// minTextX = -22, maxTextX = 122, minTextY = 70, maxTextY = 110
243+
244+
// The line parts of the bounding box are:
245+
// drawPoints.offsetStartPointMargin, drawPoints.offsetStartPointExtend,
246+
// drawPoints.offsetEndPointMargin, drawPoints.offsetEndPointExtend,
247+
// drawPoints.offsetStartPoint, drawPoints.offsetEndPoint
248+
// offsetStartPoint = (0,50), offsetEndPoint = (100,50)
249+
// MEASUREMENT_ORIGIN_MARGIN = 20, MEASUREMENT_EXTENSION_LENGTH = 20 (from App.consts.ts - actually 8, but test used 20 before)
250+
// Let's use actual consts: EXTENSION_LENGTH = 8, ORIGIN_MARGIN = 20
251+
// offsetStartPointMargin = startPoint.translate(normalUnit.multiply(MEASUREMENT_ORIGIN_MARGIN)) = (0,0) + (0,1)*20 = (0,20)
252+
// offsetStartPointExtend = offsetStartPoint.translate(normalUnit.multiply(MEASUREMENT_EXTENSION_LENGTH)) = (0,50) + (0,1)*8 = (0,58)
253+
// offsetEndPointMargin = endPoint.translate(normalUnit.multiply(MEASUREMENT_ORIGIN_MARGIN)) = (100,0) + (0,1)*20 = (100,20)
254+
// offsetEndPointExtend = offsetEndPoint.translate(normalUnit.multiply(MEASUREMENT_EXTENSION_LENGTH)) = (100,50) + (0,1)*8 = (100,58)
255+
// Line extreme points: (0,50), (100,50), (0,20), (0,58), (100,20), (100,58)
256+
// Line minX = 0, maxX = 100, minY = 20, maxY = 58
257+
// Combined with text:
258+
// allExtremePoints.map(p => p.x): -22, 122, 0, 100 => minX = -22, maxX = 122
259+
// allExtremePoints.map(p => p.y): 70, 110, 20, 58 => minY = 20, maxY = 110
260+
// So, actualBoundingBox should be: Box{xmin: -22, ymin: 20, xmax: 122, ymax: 110}
261+
// The assertions check this:
262+
// actual.xmin (-22) <= minTextX (-22) (true)
263+
// actual.ymin (20) <= minTextY (70) (true)
264+
// actual.xmax (122) >= maxTextX (122) (true)
265+
// actual.ymax (110) >= maxTextY (110) (true)
266+
});
267+
});
268+
269+
// Text center
270+
const textCenterX = midpointMeasurementLineOffset.x;
271+
const textCenterY = midpointMeasurementLineOffset.y;
272+
273+
// Half dimensions
274+
const halfTextWidth = textWidth / 2;
275+
const halfTextHeight = textHeight / 2;
276+
277+
// Text corner calculations
278+
const dirVec = finalTextDirection.normalize();
279+
const perpVec = dirVec.rotate90CW();
280+
281+
const textCorners = [
282+
new Point(
283+
textCenterX - dirVec.x * halfTextWidth - perpVec.x * halfTextHeight,
284+
textCenterY - dirVec.y * halfTextWidth - perpVec.y * halfTextHeight
285+
),
286+
new Point(
287+
textCenterX + dirVec.x * halfTextWidth - perpVec.x * halfTextHeight,
288+
textCenterY + dirVec.y * halfTextWidth - perpVec.y * halfTextHeight
289+
),
290+
new Point(
291+
textCenterX + dirVec.x * halfTextWidth + perpVec.x * halfTextHeight,
292+
textCenterY + dirVec.y * halfTextWidth + perpVec.y * halfTextHeight
293+
),
294+
new Point(
295+
textCenterX - dirVec.x * halfTextWidth + perpVec.x * halfTextHeight,
296+
textCenterY - dirVec.y * halfTextWidth + perpVec.y * halfTextHeight
297+
),
298+
];
299+
// --- End: Recalculate expected text properties ---
300+
301+
// Assert that the actualBoundingBox contains all text corners
302+
textCorners.forEach(corner => {
303+
expect(actualBoundingBox.xmin).toBeLessThanOrEqual(corner.x);
304+
expect(actualBoundingBox.ymin).toBeLessThanOrEqual(corner.y);
305+
expect(actualBoundingBox.xmax).toBeGreaterThanOrEqual(corner.x);
306+
expect(actualBoundingBox.ymax).toBeGreaterThanOrEqual(corner.y);
307+
});
308+
309+
// Additionally, check if the box.contains(point) method works (if available and reliable)
310+
// For example: textCorners.forEach(corner => expect(actualBoundingBox.contains(corner)).toBe(true));
311+
// However, checking min/max is more direct for this assertion.
312+
});
313+
});
314+
98315
describe('MeasurementEntity.distanceTo', () => {
99316
const layerId = 'test-layer';
100317

‎src/entities/MeasurementEntity.ts‎

Lines changed: 62 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -294,18 +294,75 @@ export class MeasurementEntity implements Entity {
294294
public getBoundingBox(): Box {
295295
const drawPoints = this.getDrawPoints();
296296

297-
const extremePoints = [
297+
const lineExtremePoints = [
298298
drawPoints.offsetStartPointMargin,
299299
drawPoints.offsetStartPointExtend,
300300
drawPoints.offsetEndPointMargin,
301301
drawPoints.offsetEndPointExtend,
302+
// Also include the main measurement line itself in the bounding box calculation for lines
303+
drawPoints.offsetStartPoint,
304+
drawPoints.offsetEndPoint,
302305
];
303306

307+
// Calculate text properties
308+
const distance = String(
309+
round(pointDistance(this.startPoint, this.endPoint), MEASUREMENT_DECIMAL_PLACES)
310+
);
311+
const textHeight = MEASUREMENT_FONT_SIZE;
312+
// Estimate width: textString.length * fontSize * aspectRatioFactor
313+
const textWidth = distance.length * MEASUREMENT_FONT_SIZE * 0.6;
314+
315+
const {midpointMeasurementLineOffset, normalUnit} = drawPoints;
316+
317+
// Determine text direction (similar to draw method)
318+
const originalTextDirection = normalUnit.rotate90CW();
319+
let finalTextDirection = originalTextDirection;
320+
if (
321+
originalTextDirection.x < -EPSILON ||
322+
(Math.abs(originalTextDirection.x) < EPSILON && originalTextDirection.y > EPSILON)
323+
) {
324+
finalTextDirection = new Vector(-originalTextDirection.x, -originalTextDirection.y);
325+
}
326+
327+
// Text center
328+
const textCenterX = midpointMeasurementLineOffset.x;
329+
const textCenterY = midpointMeasurementLineOffset.y;
330+
331+
// Half dimensions
332+
const halfTextWidth = textWidth / 2;
333+
const halfTextHeight = textHeight / 2;
334+
335+
// Text corner calculations
336+
// Vector along the text direction for width, and perpendicular for height
337+
const dirVec = finalTextDirection.normalize(); // Vector along the text direction
338+
const perpVec = dirVec.rotate90CW(); // Vector perpendicular to text direction (for height offset)
339+
340+
const textCorners = [
341+
new Point(
342+
textCenterX - dirVec.x * halfTextWidth - perpVec.x * halfTextHeight,
343+
textCenterY - dirVec.y * halfTextWidth - perpVec.y * halfTextHeight
344+
),
345+
new Point(
346+
textCenterX + dirVec.x * halfTextWidth - perpVec.x * halfTextHeight,
347+
textCenterY + dirVec.y * halfTextWidth - perpVec.y * halfTextHeight
348+
),
349+
new Point(
350+
textCenterX + dirVec.x * halfTextWidth + perpVec.x * halfTextHeight,
351+
textCenterY + dirVec.y * halfTextWidth + perpVec.y * halfTextHeight
352+
),
353+
new Point(
354+
textCenterX - dirVec.x * halfTextWidth + perpVec.x * halfTextHeight,
355+
textCenterY - dirVec.y * halfTextWidth + perpVec.y * halfTextHeight
356+
),
357+
];
358+
359+
const allExtremePoints = [...lineExtremePoints, ...textCorners];
360+
304361
return new Box(
305-
min(extremePoints.map((point) => point.x)),
306-
min(extremePoints.map((point) => point.y)),
307-
max(extremePoints.map((point) => point.x)),
308-
max(extremePoints.map((point) => point.y))
362+
min(allExtremePoints.map((point) => point.x)),
363+
min(allExtremePoints.map((point) => point.y)),
364+
max(allExtremePoints.map((point) => point.x)),
365+
max(allExtremePoints.map((point) => point.y))
309366
);
310367
}
311368

0 commit comments

Comments
 (0)