Merge branch 'master' into ar-bonds

CHANGELOG.md

@@ -13,6 +13,8 @@ Note that since we don't clearly distinguish between a public and private interf
 - Fix visual for delocalized bonds (parsed from mmcif and mol2)
 - Fix ring computation algorithm
 - Add ``UnitResonance`` property with info about delocalized triplets
+- Resolve marking in main renderer loop to improve overall performance
+- Use ``throttleTime`` instead of ``debounceTime`` in sequence viewer for better responsiveness
 
 ## [v3.2.0] - 2022-02-17
 

package.json

@@ -154,7 +154,7 @@
     "xhr2": "^0.2.1"
   },
   "peerDependencies": {
-    "react": "^17.0.2",
-    "react-dom": "^17.0.2"
+    "react": "^17.0.2 || ^16.14.0",
+    "react-dom": "^17.0.2 || ^ 16.14.0"
   }
 }

src/mol-canvas3d/canvas3d.ts

@@ -236,7 +236,7 @@ interface Canvas3D {
     /** Sets drawPaused = false without starting the built in animation loop */
     resume(): void
     identify(x: number, y: number): PickData | undefined
-    mark(loci: Representation.Loci, action: MarkerAction, noDraw?: boolean): void
+    mark(loci: Representation.Loci, action: MarkerAction): void
     getLoci(pickingId: PickingId | undefined): Representation.Loci
 
     notifyDidDraw: boolean,
@@ -345,7 +345,30 @@ namespace Canvas3D {
             return { loci, repr };
         }
 
-        function mark(reprLoci: Representation.Loci, action: MarkerAction, noDraw = false) {
+        let markBuffer: [reprLoci: Representation.Loci, action: MarkerAction][] = [];
+
+        function mark(reprLoci: Representation.Loci, action: MarkerAction) {
+            // NOTE: might try to optimize a case with opposite actions for the
+            //       same loci. Tho this might end up being more expensive (and error prone)
+            //       then just applying everything "naively".
+            markBuffer.push([reprLoci, action]);
+        }
+
+        function resolveMarking() {
+            let changed = false;
+            for (const [r, l] of markBuffer) {
+                changed = applyMark(r, l) || changed;
+            }
+            markBuffer = [];
+            if (changed) {
+                scene.update(void 0, true);
+                helper.handle.scene.update(void 0, true);
+                helper.camera.scene.update(void 0, true);
+            }
+            return changed;
+        }
+
+        function applyMark(reprLoci: Representation.Loci, action: MarkerAction) {
             const { repr, loci } = reprLoci;
             let changed = false;
             if (repr) {
@@ -355,24 +378,10 @@ namespace Canvas3D {
                 changed = helper.camera.mark(loci, action) || changed;
                 reprRenderObjects.forEach((_, _repr) => { changed = _repr.mark(loci, action) || changed; });
             }
-            if (changed) {
-                if (noDraw) {
-                    // Even with `noDraw` make sure changes will be rendered.
-                    // Note that with this calling mark (with or without `noDraw`) multiple times
-                    // during a JS event loop iteration will only result in a single render call.
-                    forceNextRender = true;
-                } else {
-                    scene.update(void 0, true);
-                    helper.handle.scene.update(void 0, true);
-                    helper.camera.scene.update(void 0, true);
-                    const prevPickDirty = pickHelper.dirty;
-                    draw({ force: true, allowMulti: true });
-                    pickHelper.dirty = prevPickDirty; // marking does not change picking buffers
-                }
-            }
+            return changed;
         }
 
-        function render(force: boolean, allowMulti: boolean) {
+        function render(force: boolean) {
             if (webgl.isContextLost) return false;
 
             let resized = false;
@@ -386,6 +395,8 @@ namespace Canvas3D {
                 y > gl.drawingBufferHeight || y + height < 0
             ) return false;
 
+            const markingUpdated = resolveMarking();
+
             let didRender = false;
             controls.update(currentTime);
             const cameraChanged = camera.update();
@@ -393,9 +404,9 @@ namespace Canvas3D {
             const shouldRender = force || cameraChanged || resized || forceNextRender;
             forceNextRender = false;
 
-            const multiSampleChanged = multiSampleHelper.update(shouldRender, p.multiSample);
+            const multiSampleChanged = multiSampleHelper.update(markingUpdated || shouldRender, p.multiSample);
 
-            if (shouldRender || multiSampleChanged) {
+            if (shouldRender || multiSampleChanged || markingUpdated) {
                 let cam: Camera | StereoCamera = camera;
                 if (p.camera.stereo.name === 'on') {
                     stereoCamera.update();
@@ -404,12 +415,13 @@ namespace Canvas3D {
 
                 const ctx = { renderer, camera: cam, scene, helper };
                 if (MultiSamplePass.isEnabled(p.multiSample)) {
-                    const forceOn = !cameraChanged && allowMulti && !controls.isAnimating;
+                    const forceOn = !cameraChanged && markingUpdated && !controls.isAnimating;
                     multiSampleHelper.render(ctx, p, true, forceOn);
                 } else {
                     passes.draw.render(ctx, p, true);
                 }
-                pickHelper.dirty = true;
+                // if only marking has updated, do not set the flag to dirty
+                pickHelper.dirty = pickHelper.dirty || shouldRender;
                 didRender = true;
             }
 
@@ -421,9 +433,9 @@ namespace Canvas3D {
         let currentTime = 0;
         let drawPaused = false;
 
-        function draw(options?: { force?: boolean, allowMulti?: boolean }) {
+        function draw(options?: { force?: boolean }) {
             if (drawPaused) return;
-            if (render(!!options?.force, !!options?.allowMulti) && notifyDidDraw) {
+            if (render(!!options?.force) && notifyDidDraw) {
                 didDraw.next(now() - startTime as now.Timestamp);
             }
         }
@@ -831,6 +843,8 @@ namespace Canvas3D {
             dispose: () => {
                 contextRestoredSub.unsubscribe();
 
+                markBuffer = [];
+
                 scene.clear();
                 helper.debug.clear();
                 controls.dispose();

src/mol-plugin-state/manager/interactivity.ts

@@ -76,9 +76,11 @@ namespace InteractivityManager {
 
     /**
      * The `noRender` argument indicates that the action should only update the internal
-     * data structure but not render anything user visible. For example 1) no drawing of
-     * the canvas3d scene or 2) no ui update of loci labels. This is useful because some
-     * actions require clearing any markings before they can be applied.
+     * data structure but not render anything user visible. For example, no ui update of
+     * loci labels.
+     *
+     * This is useful because some actions require clearing any markings before
+     * they can be applied.
      */
     export type LociMarkProvider = (loci: Representation.Loci, action: MarkerAction, /* test */ noRender?: boolean) => void
 

src/mol-plugin-ui/sequence/sequence.tsx

@@ -6,15 +6,15 @@
  */
 
 import * as React from 'react';
-import { PluginUIComponent } from '../base';
-import { MarkerAction } from '../../mol-util/marker-action';
-import { ButtonsType, ModifiersKeys, getButtons, getModifiers, getButton } from '../../mol-util/input/input-observer';
-import { SequenceWrapper } from './wrapper';
-import { StructureElement, StructureProperties, Unit } from '../../mol-model/structure';
 import { Subject } from 'rxjs';
-import { debounceTime } from 'rxjs/operators';
+import { throttleTime } from 'rxjs/operators';
 import { OrderedSet } from '../../mol-data/int';
+import { StructureElement, StructureProperties, Unit } from '../../mol-model/structure';
 import { Representation } from '../../mol-repr/representation';
+import { ButtonsType, getButton, getButtons, getModifiers, ModifiersKeys } from '../../mol-util/input/input-observer';
+import { MarkerAction } from '../../mol-util/marker-action';
+import { PluginUIComponent } from '../base';
+import { SequenceWrapper } from './wrapper';
 
 type SequenceProps = {
     sequenceWrapper: SequenceWrapper.Any,
@@ -55,12 +55,10 @@ export class Sequence<P extends SequenceProps> extends PluginUIComponent<P> {
         this.plugin.managers.interactivity.lociHighlights.addProvider(this.lociHighlightProvider);
         this.plugin.managers.interactivity.lociSelects.addProvider(this.lociSelectionProvider);
 
-        this.subscribe(debounceTime<{ seqIdx: number, buttons: number, button: number, modifiers: ModifiersKeys }>(15)(this.highlightQueue), (e) => {
+        this.subscribe(this.highlightQueue.pipe(throttleTime(3 * 16.666, void 0, { leading: true, trailing: true })), (e) => {
             const loci = this.getLoci(e.seqIdx < 0 ? void 0 : e.seqIdx);
             this.hover(loci, e.buttons, e.button, e.modifiers);
         });
-
-        // this.updateMarker()
     }
 
     componentWillUnmount() {

src/mol-plugin/behavior/dynamic/representation.ts

@@ -43,9 +43,9 @@ export const HighlightLoci = PluginBehavior.create({
     name: 'representation-highlight-loci',
     category: 'interaction',
     ctor: class extends PluginBehavior.Handler<HighlightLociProps> {
-        private lociMarkProvider = (interactionLoci: Representation.Loci, action: MarkerAction, noRender?: boolean) => {
+        private lociMarkProvider = (interactionLoci: Representation.Loci, action: MarkerAction) => {
             if (!this.ctx.canvas3d || !this.params.mark) return;
-            this.ctx.canvas3d.mark(interactionLoci, action, noRender);
+            this.ctx.canvas3d.mark(interactionLoci, action);
         };
         private getLoci(loci: Loci) {
             return this.params.preferAtoms && Bond.isLoci(loci) && loci.bonds.length === 2
@@ -113,9 +113,9 @@ export const SelectLoci = PluginBehavior.create({
     category: 'interaction',
     ctor: class extends PluginBehavior.Handler<SelectLociProps> {
         private spine: StateTreeSpine.Impl;
-        private lociMarkProvider = (reprLoci: Representation.Loci, action: MarkerAction, noRender?: boolean) => {
+        private lociMarkProvider = (reprLoci: Representation.Loci, action: MarkerAction) => {
             if (!this.ctx.canvas3d || !this.params.mark) return;
-            this.ctx.canvas3d.mark({ loci: reprLoci.loci }, action, noRender);
+            this.ctx.canvas3d.mark({ loci: reprLoci.loci }, action);
         };
         private getLoci(loci: Loci) {
             return this.params.preferAtoms && Bond.isLoci(loci) && loci.bonds.length === 2

tsconfig.commonjs.json

@@ -5,7 +5,7 @@
         "alwaysStrict": true,
         "noImplicitAny": true,
         "noImplicitThis": true,
-        "sourceMap": true,
+        "sourceMap": false,
         "noUnusedLocals": true,
         "strictNullChecks": true,
         "strictFunctionTypes": true,

tsconfig.json

@@ -5,7 +5,7 @@
         "alwaysStrict": true,
         "noImplicitAny": true,
         "noImplicitThis": true,
-        "sourceMap": true,
+        "sourceMap": false,
         "noUnusedLocals": true,
         "strictNullChecks": true,
         "strictFunctionTypes": true,