chore: [POC] Markers rendering improvements

[tbouffard]

DISCLAIMER: this is a POC, so it is not intended to be merged

Summary

Initially, this PR was created to experiment with marker enhancements in order to implement the rendering of the Ad-Hoc Subprocess.
Finally, this POC provides a solution to implement at least:

• [FEAT] Render the Compensation Marker of an activity  #355
• [FEAT] Render the Ad-Hoc Marker of a Sub-Process #356
• [FEAT] Fix the size of the marker icon #465
• [FEAT] Consider using the originally planned Loop marker #475
• [FEAT] Review activity markers margin and spacing #992

The experiments already leads to the creation of the following Pull Requests:

• docs: improve JSDoc of PaintParameter and StyleDefault #3049
• refactor!: remove BpmnCanvas.setIconOriginForIconOnBottomLeft #3050
• chore: enforce no unused parameters and locals with TypeScript #3051
• fix: remove extra spacing between 2 activity markers #3052
• refactor!: simplify code for shapes without icon #3054

Conclusions and decisions

Decisions taken with @csouchet on 2024-04-04

Implement the following

• enforce marker width + review width value: value 12, see chore: [POC] Markers rendering improvements #2724 (comment)
• marker stroke width: value 1, see chore: [POC] Markers rendering improvements #2724 (comment)
• center maker vertically
• markers spacing: : value 4, see chore: [POC] Markers rendering improvements #2724 (comment)
• marker bottom margin: remain unchanged, to not overlap the lines of the Transaction Sub-Process
• loop marker: reduce width and keep the existing design
• adhoc subprocess icon: use the icon of this POC
• rename StyleDefault elements: including the margin bottom, no need to include ICON in the name if we use MARKER. All names should be review (for example, there is no need to prefix them by DEFAULT, these are properties of an object name StyleDefault, so the "default" purpose is already included).
• update values in ShapeBpmnMarkerKind
• enrich the BPMN diagram used for visual tests (more usage of compensation) to prepare next developments

Details of topics covered

This PR experiment fixes and improvements for the following topics:

• improve the name of parameters of getMarkerOriginFunction:
  • allMarkers --> numberOfMarkers
  • markerOrder —> markerPosition (past proposals: markerIdx or position or index)
  • implement this in the master branch: fix: remove extra spacing between 2 activity markers #3052
• support more than 2 markers and try to provide a generalized implementation (at least for 3 and 4 markers). This will prepare [FEAT] Render the Ad-Hoc Marker of a Sub-Process #356 and [FEAT] Render the Compensation Marker of an activity  #355:
  • support 3 markers (using an arbitrary marker for the adhoc-subprocess marker)
  • generalize implementation. See chore: [POC] Markers rendering improvements #2724 (comment)
• allow to provide a shape for the marker that does not have the "16x16" dimensions. See [FEAT] Fix the size of the marker icon #465 (comment)
  • add screenshots of the current situation when using existing icon that are very large (width greater than 16 or 32 ) or the cross icon (1x1, very small)
  • POC how we could set a fixed width. See chore: [POC] Markers rendering improvements #2724 (comment) for a test with the compensation marker
  • Enrich the markers diagram with more use cases for the compensation markers to prepare [FEAT] Render the Compensation Marker of an activity  #355 --> chore: [POC] Markers rendering improvements #2724 (comment)
  • Allow to center the marker vertically in the marker line
  • Experiment an implementation of the adhoc marker [FEAT] Render the Ad-Hoc Marker of a Sub-Process #356 --> chore: [POC] Markers rendering improvements #2724 (comment)
  • To be investigated for marker whose height is larger than the width. We could decide to not handle this case as the adhoc and compensation marker are going to have a larger width than its height --> no use case, so skip investigation
  • ⚠️ be aware of the current value of SHAPE_ACTIVITY_MARKER_ICON_SIZE (see 👇🏿)
• Review SHAPE_ACTIVITY_MARKER_ICON_SIZE name and value --> SHAPE_ACTIVITY_MARKER_ICON_WIDTH: 16
  • the size of the marker configured in the StyleDefault is 20 whereas the actual width of markers is 16 (except for the loop marker). Provide a screenshot of the rendering with spacing between the markers is set to 0 to show the issue,
  • enforce marker width set to 16 (including the loop marker) --> chore: [POC] Markers rendering improvements #2724 (comment)
  • show that the extra spacing is fixed when it is set to 16 --> see screenshot showing the fix when the spacing is set to 0 chore: [POC] Markers rendering improvements #2724 (comment)
  • the configuration entry is name "size" but it may be named "width" as we need to ensure the width of the marker.
• the spacing/margin is too large
  • Issue: the spacing is applied twice when there are 2 markers
    • Create a schema to show visually what is happening
    • Fix: see screenshots below
    • Implement a fix in the master branch: fix: remove extra spacing between 2 activity markers #3052
  • After applying the fix for 2 markers, experiment new spacing values to prepare [FEAT] Review activity markers margin and spacing #992 --> chore: [POC] Markers rendering improvements #2724 (comment)
• Clarify what are spacing and margin as mentioned [FEAT] Review activity markers margin and spacing #992.
  • At least provide better JSDoc in the master branch: docs: improve JSDoc of PaintParameter and StyleDefault #3049
  • In this POC, rename StyleDefault properties
• The identifiers of the "multi instance" markers contain spaces. They are very long. Check if we can use values without spaces. The value are only used through the enum, so internally, such a change wouldn't have any effect. As the enum is part of the public API, changing the value would be a breaking change with a very limited impact as users are supposed to use the enum elements and not the string values. --> no impact on tests

  MULTI_INSTANCE_PARALLEL = 'parallel multi instance',
  MULTI_INSTANCE_SEQUENTIAL = 'sequential multi instance',

Identified issues that this POC tries to cover

Too large spacing between 2 markers

This POC provides a fix for this. The implementation to generalize markers positioning for 3 markers and more also uses the same logic.
Previously, the translation includes SHAPE_ACTIVITY_MARKER_ICON_SIZE instead of SHAPE_ACTIVITY_MARKER_ICON_SIZE/2. The following schema explains why the value has to be divided by 2 👇🏿

⚠️ In the following schema

• MARKER_WIDTH = SHAPE_ACTIVITY_MARKER_ICON_SIZE
• MARKER_SPACING = SHAPE_ACTIVITY_MARKER_ICON_MARGIN

Prior fix	With the fix

The current value of StyleDefault.SHAPE_ACTIVITY_MARKER_ICON_SIZE is not accurate

When using a spacing of 0, the 2 markers should be stacked together but this is not the case.
The "width" is configured to 20 whereas the actual icon size is currently 16. So it is larger by 4.

spacing 0	spacing -2

Unable to use markers shape not defined with 16x16 dimensions

The following screenshots demonstrate the problem by replacing the "loop marker" by some icons with specific dimensions

• paintDoubleLeftArrowheadsIcon (compensation): width 105 / height 53.5
• paintCircleIcon (icon of the inclusive gateway and terminate event) dynamic size (base on the dimensions in paintParameter.shapeConfig)
• paintAsteriskIcon (icon of the complex gatweay): dimensions 1x1

Notice that the dimensions of the "loop marker" are not 16x16 but "width: 22.49, height: 21.62". They are closed to 16x16 so the problem is not very visible

paintDoubleLeftArrowheadsIcon	paintCircleIcon	paintAsteriskIcon

This POC proposes an experimental implementation to use a fixed width for markers.
See #2724 (comment)

[tbouffard]

Markers positioning - generalization

Here, the compensation marker is arbitrary added to the list of markers defined in the BPMN diagram. The adhoc marker is displayed using an pentagon.
Screenshots generated with http://localhost:10001/dev/public/index.html?url=/test/fixtures/bpmn/bpmn-rendering/markers.01.positioning.bpmn

3 markers	4 markers

[tbouffard]

Experiment usage of fixed size markers (reuse compensation marker)

Reuse the existing "compensation" paint function and enforce that its width is "16" (absolute width of the other existing markers).

Here, the compensation marker is arbitrary added to the list of markers defined in the BPMN diagram. The adhoc marker is displayed using an pentagon.
Screenshots generated with http://localhost:10001/dev/public/index.html?url=/test/fixtures/bpmn/bpmn-rendering/markers.01.positioning.bpmn

[tbouffard]

Update markers.01.positioning BPMN diagram to prepare the support of the compensation marker

Rendering with bpmn-js in https://cdn.statically.io/gh/process-analytics/bpmn-visualization-examples/v0.43.0/examples/misc/compare-with-bpmn-js/index.html

[tbouffard]

Markers positioning - allow to center the marker vertically

[tbouffard]

Test an adhoc marker icon proposal

Test the rendering with https://thenounproject.com/icon/tilde-651893/ which is proposed in #356 (comment)

[tbouffard]

Using fixed width and experimenting smaller spacing

This is an experiment for #465, #475, #992.

Enforce width of 16 and vertical align is centered.
The code that paint the loop marker has been adapted: the declared dimensions were wrong and the painted loop was not centered

all markers with width set to 16, centered vertically and original spacing (set to 5)

This includes the changes of the "painted loop"

spacing set to 4

spacing set to 3

spacing set to 2

spacing set to 0

This demonstrates that there is no longer extra spacing added because of markers with non fixed sized.

[tbouffard]

Experiment smaller marker width to give more place to the activity label

By reducing the width, this reduces the height.

width 16	width 12

The following shows the impact on B.1.0 from miwg-test-suite
The spacing is set to 4.

width 16 (as shown in #992)	width 12

[tbouffard]

Experiment alternative stroke width

spacing: 4
width: 12

stroke width 1.5	stroke width 1

[tbouffard]

Closing, decisions taken and listed here are now tracked in #3146.