Skip to content

Commit

Permalink
#6422 – Make bond overlapping detection more performant
Browse files Browse the repository at this point in the history
  • Loading branch information
svvald committed Feb 6, 2025
1 parent f81e542 commit a7d24bf
Show file tree
Hide file tree
Showing 5 changed files with 64 additions and 24 deletions.
8 changes: 3 additions & 5 deletions packages/ketcher-core/src/application/editor/Editor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -73,10 +73,6 @@ interface ICoreEditorConstructorParams {
monomersLibraryUpdate?: string | JSON;
}

function isMouseMainButtonPressed(event: MouseEvent) {
return event.button === 0;
}

let persistentMonomersLibrary: MonomerItemType[] = [];
let persistentMonomersLibraryParsedJson: IKetMacromoleculesContent | null =
null;
Expand All @@ -92,6 +88,7 @@ export class CoreEditor {
public viewModel: ViewModel;
public lastCursorPosition: Vec2 = new Vec2(0, 0);
public lastCursorPositionOfCanvas: Vec2 = new Vec2(0, 0);
public isMouseMainButtonPressed = false;
private _monomersLibraryParsedJson: IKetMacromoleculesContent | null = null;
private _monomersLibrary: MonomerItemType[] = [];
public canvas: SVGSVGElement;
Expand Down Expand Up @@ -678,10 +675,11 @@ export class CoreEditor {

subs.add((event) => {
this.updateLastCursorPosition(event);
this.isMouseMainButtonPressed = event.button === 0;

if (
['mouseup', 'mousedown', 'click', 'dbclick'].includes(event.type) &&
!isMouseMainButtonPressed(event)
!this.isMouseMainButtonPressed
) {
return true;
}
Expand Down
11 changes: 11 additions & 0 deletions packages/ketcher-core/src/application/editor/tools/Bond.ts
Original file line number Diff line number Diff line change
Expand Up @@ -320,6 +320,11 @@ class PolymerBond implements BaseTool {
}
const modelChanges = this.finishBondCreation(renderer.monomer);
this.history.update(modelChanges);
if (modelChanges.operations[0]?.polymerBond) {
this.editor.drawingEntitiesManager.detectBondsOverlappedByMonomers([
modelChanges.operations[0].polymerBond,
]);
}
this.editor.renderersContainer.update(modelChanges);
this.editor.renderersContainer.deletePolymerBond(
this.bondRenderer.polymerBond,
Expand Down Expand Up @@ -377,6 +382,7 @@ class PolymerBond implements BaseTool {
'You have connected monomers with attachment points of the same group',
);
}

return this.editor.drawingEntitiesManager.finishPolymerBondCreation(
this.bondRenderer.polymerBond,
secondMonomer,
Expand Down Expand Up @@ -437,6 +443,11 @@ class PolymerBond implements BaseTool {

// This logic so far is only for no-modal connections. Maybe then we can chain it after modal invoke
const modelChanges = this.finishBondCreation(renderer.monomer);
if (modelChanges.operations[0]?.polymerBond) {
this.editor.drawingEntitiesManager.detectBondsOverlappedByMonomers([
modelChanges.operations[0].polymerBond,
]);
}
this.editor.renderersContainer.update(modelChanges);
this.editor.renderersContainer.deletePolymerBond(
this.bondRenderer.polymerBond,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -284,6 +284,10 @@ class SelectRectangle implements BaseTool {
}

public mouseOverPolymerBond(event) {
if (this.editor.isMouseMainButtonPressed) {
return;
}

const renderer: DeprecatedFlexModeOrSnakeModePolymerBondRenderer =
event.target.__data__;

Expand Down
63 changes: 45 additions & 18 deletions packages/ketcher-core/src/domain/entities/DrawingEntitiesManager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -119,10 +119,10 @@ type RnaPresetAdditionParams = {
export class DrawingEntitiesManager {
public monomers: Map<number, BaseMonomer> = new Map();
public polymerBonds: Map<number, PolymerBond | HydrogenBond> = new Map();
private bondsMonomersOverlaps: Map<number, BaseMonomer> = new Map();
public atoms: Map<number, Atom> = new Map();
public bonds: Map<number, Bond> = new Map();
public monomerToAtomBonds: Map<number, MonomerToAtomBond> = new Map();
public cycles: Chain[] = [];

public micromoleculesHiddenEntities: Struct = new Struct();
public canvasMatrix?: CanvasMatrix;
Expand Down Expand Up @@ -809,8 +809,6 @@ export class DrawingEntitiesManager {
this.polymerBonds.set(_polymerBond.id, _polymerBond);
firstMonomer.setBond(firstMonomerAttachmentPoint, _polymerBond);
secondMonomer.setBond(secondMonomerAttachmentPoint, _polymerBond);
_polymerBond.isOverlappedByMonomer =
this.checkBondForOverlapsByMonomers(_polymerBond);
return _polymerBond;
}

Expand All @@ -826,8 +824,6 @@ export class DrawingEntitiesManager {
secondMonomerAttachmentPoint,
polymerBond,
);
polymerBond.isOverlappedByMonomer =
this.checkBondForOverlapsByMonomers(polymerBond);

polymerBond.firstMonomer.removePotentialBonds(true);
polymerBond.secondMonomer.removePotentialBonds(true);
Expand Down Expand Up @@ -2168,6 +2164,8 @@ export class DrawingEntitiesManager {
command.merge(this.redrawBonds());
}

this.detectBondsOverlappedByMonomers();

this.monomers.forEach((monomer) => {
editor.renderersContainer.deleteMonomer(monomer);
editor.renderersContainer.addMonomer(monomer);
Expand All @@ -2189,6 +2187,10 @@ export class DrawingEntitiesManager {
public rerenderBondsOverlappedByMonomers() {
const editor = CoreEditor.provideEditorInstance();

if (editor.mode instanceof SequenceMode) {
return;
}

const monomersToCheck = this.selectedEntities
.filter(([, entity]) => entity instanceof BaseMonomer)
.map(([, entity]) => entity as BaseMonomer);
Expand Down Expand Up @@ -3070,6 +3072,11 @@ export class DrawingEntitiesManager {
polymerBond: PolymerBond,
monomers?: BaseMonomer[],
) {
const editor = CoreEditor.provideEditorInstance();
if (editor.mode instanceof SequenceMode) {
return false;
}

const secondMonomer = polymerBond.secondMonomer;
if (!secondMonomer) {
return false;
Expand All @@ -3080,21 +3087,41 @@ export class DrawingEntitiesManager {
}

const monomersToUse = monomers ?? this.monomersArray;
return monomersToUse.some((monomer) => {
if (
monomer.id === polymerBond.firstMonomer.id ||
monomer.id === secondMonomer.id
) {
return false;
}
// Skip processing for large structures for now as in worst case its has O(n^2) complexity and may freeze the app
// Further optimization might be needed to allow that
if (monomersToUse.length > 500) {
return false;
}

const distanceFromMonomerToLine = monomer.center.calculateDistanceToLine([
polymerBond.firstMonomer.center,
secondMonomer.center,
]);
const previousOverlap = this.bondsMonomersOverlaps.get(polymerBond.id);
const monomersToUseWithPreviousOverlap = previousOverlap
? [previousOverlap, ...monomersToUse]
: monomersToUse;

return distanceFromMonomerToLine < 0.375;
});
const overlappingMonomer = monomersToUseWithPreviousOverlap.find(
(monomer) => {
if (
monomer.id === polymerBond.firstMonomer.id ||
monomer.id === secondMonomer.id
) {
return false;
}

const distanceFromMonomerToLine =
monomer.center.calculateDistanceToLine([
polymerBond.firstMonomer.center,
secondMonomer.center,
]);

return distanceFromMonomerToLine < 0.375;
},
);

if (overlappingMonomer) {
this.bondsMonomersOverlaps.set(polymerBond.id, overlappingMonomer);
}

return Boolean(overlappingMonomer);
}

public detectBondsOverlappedByMonomers(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ describe('Save modal', () => {
mergeInto: jest.fn(),
},
setMicromoleculesHiddenEntities: jest.fn(),
detectCycles: jest.fn(),
detectBondsOverlappedByMonomers: jest.fn(),
monomers: [],
polymerBonds: [],
bonds: [],
Expand Down

0 comments on commit a7d24bf

Please sign in to comment.