Skip to content
Open
Show file tree
Hide file tree
Changes from 8 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
2 changes: 2 additions & 0 deletions .memory-bank/adr/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,8 @@ recorded in a ticket that the code links to.
bonds and not by the renderer
- [sgroup-parent-links.md](./sgroup-parent-links.md) — an S-group parent is resolved by one rule;
KET sets the pool position only; a copy or a removal never leaves a stale link
- [sgroup-renderers-per-type.md](./sgroup-renderers-per-type.md) — the renderer draws S-groups through one
class per type over a shared default, and places no brackets of its own for bracketed groups

Still worth reconstructing from the analyses in `indigo-common/Task/`: the valence-model
configuration contract (BIOVIA 2009 vs 2017), and the decision to route non-local formats through
Expand Down
45 changes: 45 additions & 0 deletions .memory-bank/adr/sgroup-renderers-per-type.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
# S-groups are drawn by one renderer class per type over a shared default

**Date:** 2026-11-10 · **Status:** Accepted · **Ticket:** #3976

## Context

`MoleculeRenderInternal::_initSGroups` was one function with a branch per S-group type. Each branch computed
brackets, hooks and the label position with its own arithmetic, the label went to the bracket written last,
and the copolymer, component, mixture and formulation types had no branch at all. The ticket adds the drawing
of those four types and changes the rules for the rest: hooks of a fixed length towards the atoms, the label
at the lower end of the closing bracket found by position, the repeat pattern at its upper end. The team lead
asked for a structure in which a parent class draws by default and a type with a custom drawing overrides it.

## Decision

A `SGroupRenderer` base class draws an S-group by the rules of the ticket. One subclass per drawn type
overrides only what differs: the label text, the repeat pattern, the source of the brackets, the hook length,
the label offset and placement, or the whole drawing for data S-groups. A factory maps the type to its class
and returns nothing for a type that is not drawn. The drawers see the molecule drawing through `SGroupCanvas`
and never through `MoleculeRenderInternal`; the rules that read a bracket on the page (closing bracket, ends,
hook side) live in `SGroupBrackets`, in page coordinates.

The renderer places no brackets of its own for bracketed groups: a group that arrives without brackets gets
them from the layout's `SGroupLayout`, run on the renderer's copy of the molecule.

## Alternatives considered

- Keep the single function and add four branches. Rejected: the label and hook arithmetic would exist in nine
copies, and the lead asked for the class structure.
- One class with a label table for the types that differ only by their label text (component, mixture,
formulation). Rejected for uniformity: a subclass per type is a few lines, and a custom drawing for any type
stays a one-step change.
- Brackets for groups without them by the renderer's former rule, a pair three line widths outside the atom
labels. Rejected: the ticket requires the layout's rules, so that a structure looks the same whether its
brackets were placed on load or at drawing time.

## Consequences

- A new drawn type is a subclass and a line in the factory; a change to the common rules is one place.
- The order and direction of brackets in a file no longer matter to the picture; invariant A12 is now a
writing convention.
- The former drawing of generic groups and superatoms is preserved through overrides, and a change to the base
must keep them as they are.
- Rendering a molecule with a bracketed group without brackets copies the molecule, as contracted superatoms
already did.
4 changes: 3 additions & 1 deletion .memory-bank/features/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,9 @@ feature file must stay true across a refactor.
- [molfile-data-sgroups.md](./molfile-data-sgroups.md) — data S-groups in molfiles: what the reader rejects and
the writer refuses, when `auto` writes V3000, the V3000 field type, quoting and coordinate precision
- [sgroup-brackets.md](./sgroup-brackets.md) — brackets of polymer S-groups: when they are computed, where
they stand, what is kept from a file, and what the renderer does not yet draw
they stand, what is kept from a file
- [sgroup-rendering.md](./sgroup-rendering.md) — brackets, hooks, labels and repeat patterns of S-groups in a
picture: which types are drawn, where the label stands, what stayed as it was
- [sgroup-data-values.md](./sgroup-data-values.md) — values of data S-groups: how they follow their atoms in
layout and clean-up, and what Ketcher does not yet send

Expand Down
9 changes: 4 additions & 5 deletions .memory-bank/features/sgroup-brackets.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,8 @@
> **Read when:** a ticket is about where the brackets of a repeating unit, copolymer, component, mixture or
> formulation stand, or when they appear or move.
> **Skip when:** the question is about superatoms, multiple groups or data S-groups (those are
> [sgroup-data-values.md](./sgroup-data-values.md)).
> [sgroup-data-values.md](./sgroup-data-values.md)), or about how the brackets and labels look in a picture:
> that is [sgroup-rendering.md](./sgroup-rendering.md).

Verified on 2026-10-28.

Expand Down Expand Up @@ -47,8 +48,8 @@ scaled gets brackets anew by these rules, also with `layout-preserve-existing`;
types move and scale with their molecules.

**When** a structure is saved to MOL, **then** zero-length brackets are not written, for any S-group type; a real
bracket next to one is written, and a group left with fewer than two real brackets is drawn with the renderer's
own pair.
bracket next to one is written, and a group read back with that one bracket keeps it: a generic or multiple group
is then drawn with the renderer's own pair, a bracketed group with its one bracket.

## Guarantees

Expand All @@ -57,8 +58,6 @@ mock-ups are pinned to four decimals.

## Limitations

- The renderer draws brackets only for repeating units, multiple groups, superatoms and generic groups; those
of copolymers, components, mixtures and formulations are in the file but not in the picture.
- Distances are in the default bond length, 1.0, whatever the bond length of the drawing: a structure drawn
at another scale gets margins out of proportion.
- Nothing outside the group is considered: a bracket can run through a neighbouring atom, the `*` at a chain
Expand Down
70 changes: 70 additions & 0 deletions .memory-bank/features/sgroup-rendering.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,70 @@
# Brackets and labels of S-groups in a picture

> **Read when:** a ticket is about how an S-group looks in PNG, SVG, PDF or EMF: its brackets, their hooks, its
> label or its repeat pattern.
> **Skip when:** the question is where the brackets stand in the structure: that is
> [sgroup-brackets.md](./sgroup-brackets.md).

Verified on 2026-11-10. Ticket #3976, under the polymer epic #3971.
Comment thread
konstantin-mordovskih marked this conversation as resolved.
Resolved

## Problem

A picture showed the brackets and the label of repeating units, multiple groups, superatoms and generic groups
only. A copolymer, a component, a mixture or a formulation had its brackets in the file and nothing in the
picture: a block copolymer of two repeating units showed the two inner pairs with `n` and nothing around them.
A structure without bracket coordinates got a pair placed by rules of the renderer's own, not those of the
layout, and the hooks of a bracket grew with its length.

## Interface

Nothing to call: every rendering of a molecule or a reaction, in every output format, with no option.

## Expected behaviour

**When** a bracketed S-group (repeating unit, copolymer, component, mixture, formulation) has brackets, **then**
they are drawn where they are, in any direction; a zero-length bracket is not drawn.

**When** such a group has no brackets, or only zero-length ones (CDX, CDXML, CXSMILES, a structure built through
the API), **then** it is drawn with brackets placed by the rules of [sgroup-brackets.md](./sgroup-brackets.md),
at the default font and in the label mode of the rendering; the structure itself is not changed.

**When** a bracket of a bracketed group or of a multiple group is drawn, **then** its hooks are 0.15 bond lengths
long, perpendicular to the bracket and turned towards the centre of the group's atoms. The hooks of generic
groups and superatoms stay 0.15 of the bracket's length, as before.

**When** a group has a label, **then** it stands at the lower end of the closing bracket: its top edge level
with that end, its left edge 0.05 bond lengths right of the bracket, horizontal whatever the bracket's direction.
The closing bracket is the rightmost one by its middle, of brackets one above the other the lower one; the lower
end is the one lower on the page, of a horizontal bracket its left end. This holds for brackets stored in any
order, swapped in the file or turned with the molecule. The labels: a repeating unit its label from the file or
`n`; a copolymer `co`, or `alt`, `ran`, `blk` by its subtype; a component `c`, or `cN` by its order; a mixture
`mix`; a formulation `f`; a multiple group its multiplier. A superatom's label stays where it was: its bottom
edge level with the lower end, a line width right of the bracket. A generic group has no label.

**When** a repeating unit or a copolymer has a repeat pattern other than head-to-tail, **then** `hh`
(head-to-head) or `eu` (either, unknown, or not set) stands at the upper end of the closing bracket: its bottom
edge level with that end, the same offset to the right.

**When** groups are nested, **then** all of them are drawn. Labels and repeat patterns do not move to make room:
they may overlap the bracket of a parent or of a neighbouring group.

**When** a data S-group has a value, **then** it is drawn where its position says, as before. A relative value
without atoms of its own, whose parents give it no frame, is not drawn; before this change it had no position
and was not visible either.

## Guarantees

Deterministic for given coordinates; the same drawing in PNG, SVG, PDF and EMF. The forty structures of the
ticket are pinned as SVG in the integration harness, one reference for the CI platforms, since the glyphs come from
the font built into the renderer; an ARM Mac differs in the sixth decimal of the coordinates.

## Limitations

- A repeating unit of one atom without a label has brackets 0.2 bond lengths apart, so their hooks overlap.
- Nothing keeps a label clear of the bracket of a neighbouring group, or a bracket clear of a label standing
within its margin.
- Brackets placed by the renderer for a group without them use the default font, not `render-font-size`.
- A generic or multiple group whose file has fewer than two real brackets gets the renderer's own pair, three
line widths outside its atom labels, as before.
- The picture of an unnamed superatom no longer reserves room for its empty label, so it is a little smaller
than before.
23 changes: 13 additions & 10 deletions .memory-bank/invariants.md
Original file line number Diff line number Diff line change
Expand Up @@ -113,12 +113,13 @@ of these, should read the line first.

## S-group brackets

**A12 — The two brackets of a bracketed S-group keep their order and direction: the opening one first, running
bottom to top, the closing one second, running top to bottom.**
The renderer turns the hooks of a bracket to the right of its direction and puts the label and the repeat pattern
at the last bracket (`core/render2d/src/render_internal.cpp#_positionIndex`). A pair written the other way round
draws its hooks outwards and its label at the left bracket; nothing fails. The one place that
writes a pair is `core/indigo-core/layout/src/sgroup_layout.cpp#_setBrackets`.
**A12 — The two brackets of a bracketed S-group are written in one order and direction: the opening one first,
running bottom to top, the closing one second, running top to bottom.**
Since #3976 nothing in Indigo reads that order: the renderer takes the closing bracket by position, the rightmost
one, and turns the hooks of every bracket towards the atoms of the group
(`core/render2d/src/render_sgroups.cpp#SGroupBrackets::closing`), so a pair written the other way round draws
the same. The order is the convention of the files Indigo writes, for the readers of those files; the one place
that writes a pair is `core/indigo-core/layout/src/sgroup_layout.cpp#_setBrackets`.

**A13 — Whatever moves the atoms of a molecule moves its S-group brackets and data values with them.**
Brackets and the stored position of a data value are coordinates on the group; nothing ties them to the atoms.
Expand All @@ -141,10 +142,12 @@ and the CDXML and CXSMILES loaders leave such pairs too. `SGroup::hasBrackets()`
(`core/indigo-core/molecule/molecule_sgroups.h#hasBrackets`): one zero-length bracket means the group has none,
and the layout replaces them. The MOL savers skip each zero-length bracket and keep the real ones next to it
(`core/indigo-core/molecule/src/molfile_saver.cpp#isZeroLengthBracket`); `transformPositions` moves the real
brackets of a group and leaves its placeholders; the renderer takes the file's brackets only when the first two
are real and draws its own pair otherwise (`core/render2d/src/render_internal.cpp#isBracketsCoordinates`). Code
that tests `brackets.size() > 0` instead takes a bracket at the origin for a real one: the CML saver writes
placeholders out that way.
brackets of a group and leaves its placeholders; the renderer draws the real brackets of a generic or multiple
group when it has at least two and a pair of its own otherwise
(`core/render2d/src/render_sgroups.cpp#_storedOrPairAroundAtoms`), and gives a bracketed group without brackets
a pair by the layout's rules on its own copy of the molecule before drawing
(`core/render2d/src/render_internal.cpp#_placeMissingBrackets`). Code that tests `brackets.size() > 0` instead
takes a bracket at the origin for a real one: the CML saver writes placeholders out that way.

## S-group data fields

Expand Down
5 changes: 4 additions & 1 deletion .memory-bank/modules/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,8 @@ document that restates the header file saves nothing and should not be written.
fingerprint pre-filter + postprocess pattern, its own test suite
- [layout-sgroup-brackets.md](./layout-sgroup-brackets.md) — brackets of polymer S-groups: who places them
and when, the fixed margins, nesting, the constant-unit and font traps
- [render-sgroups.md](./render-sgroups.md) — `render2d/render_sgroups.*`: one drawer class per S-group type,
the canvas they draw on, the bracket geometry in page coordinates, how to add a type
- [layout-sgroup-data.md](./layout-sgroup-data.md) — values of data S-groups: when they move, the fit of their atoms
and its line and single-atom cases, what a whole-molecule move leaves to it

Expand All @@ -37,7 +39,8 @@ when a task forces you to reconstruct the knowledge anyway.
- `layout` — the 2D layout pipeline and its cross-platform reproducibility limits (invariant A10); the
S-group bracket part is covered by [layout-sgroup-brackets.md](./layout-sgroup-brackets.md), the data values by
[layout-sgroup-data.md](./layout-sgroup-data.md)
- `render2d` — rendering pipeline, cairo backend, font and platform dependencies
- `render2d` — rendering pipeline, cairo backend, font and platform dependencies; the S-group part is
covered by [render-sgroups.md](./render-sgroups.md)
- `c-api` — the FFI surface, session/object registry, `INDIGO_BEGIN`/`INDIGO_END` machinery
- `wrappers` — how the Python/Java/.NET/WASM bindings are generated and kept in step
- `indigo-service` — the FastAPI REST service and its Docker packaging
Expand Down
5 changes: 3 additions & 2 deletions .memory-bank/modules/layout-sgroup-brackets.md
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ Callers:
| `MoleculeCleaner2d::do_clean`: `clean2d()`, both passes | after the trivial-structure returns | after the coordinates are written back | default |
| `MolfileLoader::_postLoad` | at once | at once: only groups without `SDI` / `BRKXYZ` change | default |
| `ReactionLayout::make`, `PathwayLayout::make`: the metalayout's move and scale of whole molecules | before `Metalayout::process()` / `buildLayoutTree()` | after `process()` / `applyLayout()` | the same options |
| `MoleculeRenderInternal::_placeMissingBrackets`: a drawing of a bracketed group without brackets | at once, on the renderer's copy of the molecule | at once | default size, `render-label-mode` of the drawing |

`Metalayout::adjustMol`, `BaseMolecule::scale` and `BaseMolecule::offsetCoordinates` carry every group's real
brackets along through `core/indigo-core/molecule/molecule_sgroups.h#transformPositions` (invariant A13), with the
Expand Down Expand Up @@ -89,5 +90,5 @@ be a child, never a parent.
the unselected fragments the layout packs anew.
- **References with layout coordinates are generated on Linux** (invariant A10). On an ARM Mac two `clean2d`
references of `sgroup_brackets.py` differ in the fourth decimal.
- The order of the two brackets is a contract with the renderer (A12); zero-length brackets are placeholders
(A14).
- The order of the two brackets is the convention of the files Indigo writes; nothing in Indigo reads it, the
renderer finds the closing bracket by position (A12). Zero-length brackets are placeholders (A14).
4 changes: 2 additions & 2 deletions .memory-bank/modules/layout-sgroup-data.md
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ absolute values; `ReactionLayout` and `PathwayLayout` move whole molecules, whic
done for an absolute value; the loaders move nothing.

What a stored position means lives on the group, not here: `core/indigo-core/molecule/molecule_sgroups.h#originAtom`,
which the renderer reads too (`core/render2d/src/render_internal.cpp#originAtom`),
which the renderer reads too (`core/render2d/src/render_sgroups.cpp#originAtom`),
`core/indigo-core/molecule/molecule_sgroups.h#displayPoint`,
`core/indigo-core/molecule/molecule_sgroups.h#setDisplayPoint` and
`core/indigo-core/molecule/molecule_sgroups.h#transformPosition`.
Expand Down Expand Up @@ -70,7 +70,7 @@ which the renderer reads too (`core/render2d/src/render_internal.cpp#originAtom`
`data-distorted-copolymer.mol`.
- **A relative position is an offset from the first atom of the group**, as the renderer resolves it. For a group
without atoms the offset is from the right top corner of a box the renderer hands down its tree of groups
(`core/render2d/src/render_internal.cpp#rightTop`): a repeating unit passes the box of its atoms, a component,
(`core/render2d/src/render_sgroups.cpp#rightTop`): a repeating unit passes the box of its atoms, a component,
mixture, formulation or copolymer passes on the box it got, starting from the whole molecule, and a data, generic,
multiple or superatom group passes none. That box does not turn and is computed anew for every drawing, so such a
value is left as it is.
Expand Down
Loading
Loading