Skip to content

Commit e327cde

Browse files
authored
fix: correctly ignore activity and task label bounds (#3434)
The positions (x and y coordinates) were not ignored. Only the global positioning of the label, relative to the shape, was updated. Also rename some visual tests prior adding new ones, including test for ignore label styles as well.
1 parent e9dfc68 commit e327cde

15 files changed

Lines changed: 402 additions & 183 deletions

src/component/mxgraph/BpmnRenderer.ts

Lines changed: 22 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -34,11 +34,18 @@ import StyleComputer from './renderer/StyleComputer';
3434
* @internal
3535
*/
3636
export class BpmnRenderer {
37+
private readonly ignoreBpmnActivityLabelBounds: boolean;
38+
private readonly ignoreBpmnTaskLabelBounds: boolean;
39+
3740
constructor(
3841
readonly graph: BpmnGraph,
3942
readonly coordinatesTranslator: CoordinatesTranslator,
4043
readonly styleComputer: StyleComputer,
41-
) {}
44+
rendererOptions: RendererOptions,
45+
) {
46+
this.ignoreBpmnActivityLabelBounds = rendererOptions?.ignoreBpmnActivityLabelBounds ?? false;
47+
this.ignoreBpmnTaskLabelBounds = rendererOptions?.ignoreBpmnTaskLabelBounds ?? false;
48+
}
4249

4350
render(renderedModel: RenderedModel): void {
4451
this.insertShapesAndEdges(renderedModel);
@@ -71,9 +78,7 @@ export class BpmnRenderer {
7178
const bpmnElement = shape.bpmnElement;
7279
const parent = this.getParent(bpmnElement);
7380
const bounds = shape.bounds;
74-
let labelBounds = shape.label?.bounds;
75-
// pool/lane label bounds are not managed for now (use hard coded values)
76-
labelBounds = ShapeUtil.isPoolOrLane(bpmnElement.kind) ? undefined : labelBounds;
81+
const labelBounds = isLabelBoundsIgnored(shape, this.ignoreBpmnActivityLabelBounds, this.ignoreBpmnTaskLabelBounds) ? undefined : shape.label?.bounds;
7782
const style = this.styleComputer.computeStyle(shape, labelBounds);
7883

7984
this.insertVertex(parent, bpmnElement.id, bpmnElement.name, bounds, labelBounds, style);
@@ -139,11 +144,23 @@ export class BpmnRenderer {
139144
}
140145
}
141146

147+
/**
148+
* @internal
149+
*/
150+
export function isLabelBoundsIgnored(shape: Shape, ignoreBpmnActivityLabelBounds: boolean, ignoreBpmnTaskLabelBounds: boolean): boolean {
151+
const kind = shape.bpmnElement.kind;
152+
return (
153+
ShapeUtil.isPoolOrLane(kind) || // pool/lane label bounds are not managed for now (use hard coded values)
154+
(ignoreBpmnActivityLabelBounds && ShapeUtil.isActivity(kind)) ||
155+
(ignoreBpmnTaskLabelBounds && ShapeUtil.isTask(kind))
156+
);
157+
}
158+
142159
/**
143160
* @internal
144161
*/
145162
export function newBpmnRenderer(graph: BpmnGraph, options: RendererOptions): BpmnRenderer {
146-
return new BpmnRenderer(graph, new CoordinatesTranslator(graph), new StyleComputer(options));
163+
return new BpmnRenderer(graph, new CoordinatesTranslator(graph), new StyleComputer(options), options);
147164
}
148165

149166
/**

src/component/mxgraph/renderer/StyleComputer.ts

Lines changed: 3 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -40,14 +40,10 @@ import { BpmnStyleIdentifier } from '../style';
4040
export default class StyleComputer {
4141
private readonly ignoreBpmnColors: boolean;
4242
private readonly ignoreBpmnLabelStyles: boolean;
43-
private readonly ignoreBpmnActivityLabelBounds: boolean;
44-
private readonly ignoreBpmnTaskLabelBounds: boolean;
4543

4644
constructor(options?: RendererOptions) {
4745
this.ignoreBpmnColors = options?.ignoreBpmnColors ?? true;
4846
this.ignoreBpmnLabelStyles = options?.ignoreBpmnLabelStyles ?? false;
49-
this.ignoreBpmnActivityLabelBounds = options?.ignoreBpmnActivityLabelBounds ?? false;
50-
this.ignoreBpmnTaskLabelBounds = options?.ignoreBpmnTaskLabelBounds ?? false;
5147
}
5248

5349
computeStyle(bpmnCell: Shape | Edge, labelBounds: Bounds): string {
@@ -62,16 +58,12 @@ export default class StyleComputer {
6258
}
6359

6460
const fontStyleValues = this.computeFontStyleValues(bpmnCell);
65-
const labelStyleValues = this.computeLabelStyleValues(bpmnCell, labelBounds);
61+
const labelStyleValues = computeLabelStyleValues(bpmnCell, labelBounds);
6662

6763
styles.push(...toArrayOfMxGraphStyleEntries([...mainStyleValues, ...fontStyleValues, ...labelStyleValues]));
6864
return styles.join(';');
6965
}
7066

71-
private computeLabelStyleValues(bpmnCell: Shape | Edge, labelBounds: Bounds): Map<string, string | number> {
72-
return computeLabelStyleValues(bpmnCell, labelBounds, this.ignoreBpmnActivityLabelBounds, this.ignoreBpmnTaskLabelBounds);
73-
}
74-
7567
private computeShapeStyleValues(shape: Shape): Map<string, string | number> {
7668
const styleValues = new Map<string, string | number>();
7769
const bpmnElement = shape.bpmnElement;
@@ -182,20 +174,11 @@ function computeEdgeBaseStyles(edge: Edge): string[] {
182174
return styles;
183175
}
184176

185-
function computeLabelStyleValues(
186-
bpmnCell: Shape | Edge,
187-
labelBounds: Bounds,
188-
ignoreBpmnActivityLabelBounds: boolean,
189-
ignoreBpmnTaskLabelBounds: boolean,
190-
): Map<string, string | number> {
177+
function computeLabelStyleValues(bpmnCell: Shape | Edge, labelBounds: Bounds): Map<string, string | number> {
191178
const styleValues = new Map<string, string | number>();
192179

193180
const bpmnElement = bpmnCell.bpmnElement;
194-
195-
// Check if we should ignore label bounds for this element
196-
const shouldIgnoreLabelBounds = shouldIgnoreBpmnLabelBounds(bpmnCell, ignoreBpmnActivityLabelBounds, ignoreBpmnTaskLabelBounds);
197-
198-
if (labelBounds && !shouldIgnoreLabelBounds) {
181+
if (labelBounds) {
199182
styleValues.set(mxConstants.STYLE_VERTICAL_ALIGN, mxConstants.ALIGN_TOP);
200183
if (bpmnCell.bpmnElement.kind != ShapeBpmnElementKind.TEXT_ANNOTATION) {
201184
styleValues.set(mxConstants.STYLE_ALIGN, mxConstants.ALIGN_CENTER);
@@ -224,32 +207,6 @@ function computeLabelStyleValues(
224207
return styleValues;
225208
}
226209

227-
/**
228-
* Determines if label bounds should be ignored based on the element type and options.
229-
*/
230-
function shouldIgnoreBpmnLabelBounds(bpmnCell: Shape | Edge, ignoreBpmnActivityLabelBounds: boolean, ignoreBpmnTaskLabelBounds: boolean): boolean {
231-
// Only apply to shapes, not edges
232-
if (!(bpmnCell instanceof Shape)) {
233-
return false;
234-
}
235-
236-
const bpmnElement = bpmnCell.bpmnElement;
237-
238-
// If ignoring all activity label bounds
239-
if (ignoreBpmnActivityLabelBounds && bpmnElement instanceof ShapeBpmnActivity) {
240-
return true;
241-
}
242-
243-
// If ignoring task label bounds only, check if it's a task (but not subprocess or call activity)
244-
if (ignoreBpmnTaskLabelBounds && bpmnElement instanceof ShapeBpmnActivity) {
245-
// Activities include tasks, sub-processes, and call activities
246-
// We only want to ignore bounds for tasks, not sub-processes or call activities
247-
return !(bpmnElement instanceof ShapeBpmnSubProcess) && !(bpmnElement instanceof ShapeBpmnCallActivity);
248-
}
249-
250-
return false;
251-
}
252-
253210
/**
254211
* @internal
255212
* @private
27.8 KB
Loading
29.5 KB
Loading
14.8 KB
Loading
14.7 KB
Loading
Lines changed: 194 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,194 @@
1+
/*
2+
Copyright 2025 Bonitasoft S.A.
3+
4+
Licensed under the Apache License, Version 2.0 (the "License");
5+
you may not use this file except in compliance with the License.
6+
You may obtain a copy of the License at
7+
8+
http://www.apache.org/licenses/LICENSE-2.0
9+
10+
Unless required by applicable law or agreed to in writing, software
11+
distributed under the License is distributed on an "AS IS" BASIS,
12+
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
See the License for the specific language governing permissions and
14+
limitations under the License.
15+
*/
16+
17+
import type { ImageSnapshotThresholdConfig } from './helpers/visu/image-snapshot-config';
18+
19+
import { ImageSnapshotConfigurator, MultiBrowserImageSnapshotThresholds } from './helpers/visu/image-snapshot-config';
20+
21+
import { AvailableTestPages, PageTester } from '@test/shared/visu/bpmn-page-utils';
22+
23+
class ImageSnapshotThresholdsActivityLabelBounds extends MultiBrowserImageSnapshotThresholds {
24+
constructor() {
25+
super({ chromium: 0 / 100, firefox: 0 / 100, webkit: 0 / 100 });
26+
}
27+
28+
protected override getChromiumThresholds(): Map<string, ImageSnapshotThresholdConfig> {
29+
return new Map<string, ImageSnapshotThresholdConfig>([
30+
[
31+
'activities.with.wrongly.positioned.labels.not-ignored',
32+
{
33+
macos: 0.19 / 100, // 0.18413140767182812%
34+
windows: 0.22 / 100, // 0.21177195104159496%
35+
},
36+
],
37+
[
38+
'activities.with.wrongly.positioned.labels.ignored',
39+
{
40+
macos: 0.29 / 100, // 0.28115982788768923%
41+
windows: 0.28 / 100, // 0.27545483944123594%
42+
},
43+
],
44+
]);
45+
}
46+
47+
protected override getFirefoxThresholds(): Map<string, ImageSnapshotThresholdConfig> {
48+
return new Map<string, ImageSnapshotThresholdConfig>([
49+
[
50+
'activities.with.wrongly.positioned.labels.not-ignored',
51+
{
52+
linux: 0.23 / 100, // 0.2236111574782207%
53+
macos: 0.36 / 100, // 0.35012765743468455%
54+
windows: 1.32 / 100, // 1.3103779470739596%
55+
},
56+
],
57+
[
58+
'activities.with.wrongly.positioned.labels.ignored',
59+
{
60+
linux: 0.85 / 100, // 0.8457215876566115%
61+
macos: 0.64 / 100, // 0.634897664634726%
62+
windows: 1.9 / 100, // 1.8900990794732508%
63+
},
64+
],
65+
]);
66+
}
67+
68+
protected override getWebkitThresholds(): Map<string, ImageSnapshotThresholdConfig> {
69+
return new Map<string, ImageSnapshotThresholdConfig>([
70+
[
71+
'activities.with.wrongly.positioned.labels.not-ignored',
72+
{
73+
macos: 0.44 / 100, // 0.4382175377357411%
74+
},
75+
],
76+
[
77+
'activities.with.wrongly.positioned.labels.ignored',
78+
{
79+
macos: 1.5 / 100, // 1.4951298719464878%
80+
},
81+
],
82+
]);
83+
}
84+
}
85+
86+
class ImageSnapshotThresholdsLabelStyles extends MultiBrowserImageSnapshotThresholds {
87+
constructor() {
88+
super({ chromium: 0 / 100, firefox: 0 / 100, webkit: 0 / 100 });
89+
}
90+
91+
protected override getChromiumThresholds(): Map<string, ImageSnapshotThresholdConfig> {
92+
return new Map<string, ImageSnapshotThresholdConfig>([
93+
[
94+
'labels.with.font.styles.not-ignored',
95+
{
96+
macos: 0.17 / 100, // 0.16518659366272503%
97+
windows: 0.16 / 100, // 0.1578221549419112%
98+
},
99+
],
100+
[
101+
'labels.with.font.styles.ignored',
102+
{
103+
macos: 0.21 / 100, // 0.20961855005547925%
104+
windows: 0.29 / 100, // 0.2864761242524328%
105+
},
106+
],
107+
]);
108+
}
109+
110+
protected override getFirefoxThresholds(): Map<string, ImageSnapshotThresholdConfig> {
111+
return new Map<string, ImageSnapshotThresholdConfig>([
112+
[
113+
'labels.with.font.styles.not-ignored',
114+
{
115+
// very large number because of firefox rendering differences compared to chrome (font)
116+
// this test requires to use a dedicated reference screenshot, see https://github.com/process-analytics/bpmn-visualization-js/issues/2838
117+
linux: 4.53 / 100, // 4.529968414713514%
118+
macos: 0.5 / 100, // 0.49100385220669507%
119+
windows: 2.1 / 100, // 2.096414153407533%
120+
},
121+
],
122+
[
123+
'labels.with.font.styles.ignored',
124+
{
125+
linux: 0.06 / 100, // 0.05351232277721607%
126+
macos: 0.26 / 100, // 0.2513269855389466%
127+
windows: 1.64 / 100, // 1.637503468193735%
128+
},
129+
],
130+
]);
131+
}
132+
133+
protected override getWebkitThresholds(): Map<string, ImageSnapshotThresholdConfig> {
134+
return new Map<string, ImageSnapshotThresholdConfig>([
135+
[
136+
'labels.with.font.styles.not-ignored',
137+
{
138+
macos: 0.31 / 100, // 0.30621380597637415%
139+
},
140+
],
141+
[
142+
'labels.with.font.styles.ignored',
143+
{
144+
macos: 0.38 / 100, // 0.37988509633168904%
145+
},
146+
],
147+
]);
148+
}
149+
}
150+
151+
function getConfigName(bpmnDiagramName: string, ignoredOption: boolean): string {
152+
return bpmnDiagramName + '.' + (ignoredOption ? 'ignored' : 'not-ignored');
153+
}
154+
155+
describe('BPMN rendering - ignore options', () => {
156+
const diagramSubfolder = 'bpmn-rendering-ignore-options';
157+
const pageTester = new PageTester({ targetedPage: AvailableTestPages.BPMN_RENDERING, diagramSubfolder }, page);
158+
159+
describe('Ignore activity label bounds', () => {
160+
const bpmnDiagramName = 'activities.with.wrongly.positioned.labels';
161+
162+
describe.each([false, true])('ignoreBpmnActivityLabelBounds: %s', (ignoreBpmnActivityLabelBounds: boolean) => {
163+
const imageSnapshotConfigurator = new ImageSnapshotConfigurator(new ImageSnapshotThresholdsActivityLabelBounds(), 'bpmn-rendering-ignore-options');
164+
165+
it(`${bpmnDiagramName}`, async () => {
166+
await pageTester.gotoPageAndLoadBpmnDiagram(bpmnDiagramName, {
167+
rendererIgnoreBpmnActivityLabelBounds: ignoreBpmnActivityLabelBounds,
168+
});
169+
170+
const image = await page.screenshot({ fullPage: true });
171+
const config = imageSnapshotConfigurator.getConfig(getConfigName(bpmnDiagramName, ignoreBpmnActivityLabelBounds));
172+
expect(image).toMatchImageSnapshot(config);
173+
});
174+
});
175+
});
176+
177+
describe('Ignore label styles', () => {
178+
const bpmnDiagramName = 'labels.with.font.styles';
179+
180+
describe.each([false, true])('ignoreBpmnLabelStyles: %s', (ignoreBpmnLabelStyles: boolean) => {
181+
const imageSnapshotConfigurator = new ImageSnapshotConfigurator(new ImageSnapshotThresholdsLabelStyles(), 'bpmn-rendering-ignore-options');
182+
183+
it(`${bpmnDiagramName}`, async () => {
184+
await pageTester.gotoPageAndLoadBpmnDiagram(bpmnDiagramName, {
185+
rendererIgnoreBpmnLabelStyles: ignoreBpmnLabelStyles,
186+
});
187+
188+
const image = await page.screenshot({ fullPage: true });
189+
const config = imageSnapshotConfigurator.getConfig(getConfigName(bpmnDiagramName, ignoreBpmnLabelStyles));
190+
expect(image).toMatchImageSnapshot(config);
191+
});
192+
});
193+
});
194+
});
Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
1+
<?xml version="1.0" encoding="UTF-8"?>
2+
<bpmn:definitions xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xmlns:bpmn="http://www.omg.org/spec/BPMN/20100524/MODEL" xmlns:bpmndi="http://www.omg.org/spec/BPMN/20100524/DI" xmlns:dc="http://www.omg.org/spec/DD/20100524/DC" id="Definitions_wrongly_positioned_labels" targetNamespace="http://example.bpmn.com/schema/bpmn">
3+
<bpmn:process id="Process_1" isExecutable="false">
4+
<bpmn:task id="Task_1" name="Task with label too high" />
5+
<bpmn:userTask id="UserTask_1" name="User Task partly outside" />
6+
<bpmn:serviceTask id="ServiceTask_1" name="Service Task label out of bounds" />
7+
<bpmn:callActivity id="CallActivity_1" name="Call Activity top right" calledElement="global_task" />
8+
<bpmn:subProcess id="SubProcess_1" name="SubProcess bottom left">
9+
<bpmn:task id="InnerTask_1" />
10+
</bpmn:subProcess>
11+
</bpmn:process>
12+
<bpmndi:BPMNDiagram id="BPMNDiagram_1">
13+
<bpmndi:BPMNPlane id="BPMNPlane_1" bpmnElement="Process_1">
14+
<bpmndi:BPMNShape id="Task_1_di" bpmnElement="Task_1">
15+
<dc:Bounds x="160" y="80" width="100" height="80" />
16+
<bpmndi:BPMNLabel>
17+
<dc:Bounds x="160" y="80" width="100" height="30" />
18+
</bpmndi:BPMNLabel>
19+
</bpmndi:BPMNShape>
20+
<bpmndi:BPMNShape id="UserTask_1_di" bpmnElement="UserTask_1">
21+
<dc:Bounds x="320" y="80" width="100" height="80" />
22+
<bpmndi:BPMNLabel>
23+
<dc:Bounds x="320" y="65" width="100" height="40" />
24+
</bpmndi:BPMNLabel>
25+
</bpmndi:BPMNShape>
26+
<bpmndi:BPMNShape id="ServiceTask_1_di" bpmnElement="ServiceTask_1">
27+
<dc:Bounds x="470" y="80" width="100" height="80" />
28+
<bpmndi:BPMNLabel>
29+
<dc:Bounds x="510" y="140" width="100" height="50" />
30+
</bpmndi:BPMNLabel>
31+
</bpmndi:BPMNShape>
32+
<bpmndi:BPMNShape id="CallActivity_1_di" bpmnElement="CallActivity_1" isExpanded="true">
33+
<dc:Bounds x="160" y="220" width="140" height="100" />
34+
<bpmndi:BPMNLabel>
35+
<dc:Bounds x="220" y="225" width="80" height="25" />
36+
</bpmndi:BPMNLabel>
37+
</bpmndi:BPMNShape>
38+
<bpmndi:BPMNShape id="SubProcess_1_di" bpmnElement="SubProcess_1" isExpanded="true">
39+
<dc:Bounds x="360" y="200" width="250" height="150" />
40+
<bpmndi:BPMNLabel>
41+
<dc:Bounds x="365" y="320" width="120" height="25" />
42+
</bpmndi:BPMNLabel>
43+
</bpmndi:BPMNShape>
44+
<bpmndi:BPMNShape id="InnerTask_1_di" bpmnElement="InnerTask_1">
45+
<dc:Bounds x="430" y="250" width="100" height="60" />
46+
</bpmndi:BPMNShape>
47+
</bpmndi:BPMNPlane>
48+
</bpmndi:BPMNDiagram>
49+
</bpmn:definitions>

0 commit comments

Comments
 (0)