Merge pull request #326 from molstar/marker-fixes

Marker improvements

src/mol-canvas3d/canvas3d.ts

@@ -223,7 +223,7 @@ interface Canvas3D {
     clear(): void
     syncVisibility(): void
 
-    requestDraw(force?: boolean): void
+    requestDraw(): void
 
     /** Reset the timers, used by "animate" */
     resetTime(t: number): void
@@ -355,17 +355,24 @@ namespace Canvas3D {
                 changed = helper.camera.mark(loci, action) || changed;
                 reprRenderObjects.forEach((_, _repr) => { changed = _repr.mark(loci, action) || changed; });
             }
-            if (changed && !noDraw) {
-                scene.update(void 0, true);
-                helper.handle.scene.update(void 0, true);
-                helper.camera.scene.update(void 0, true);
-                const prevPickDirty = pickHelper.dirty;
-                draw(true);
-                pickHelper.dirty = prevPickDirty; // marking does not change picking buffers
+            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
+                }
             }
         }
 
-        function render(force: boolean) {
+        function render(force: boolean, allowMulti: boolean) {
             if (webgl.isContextLost) return false;
 
             let resized = false;
@@ -395,14 +402,12 @@ namespace Canvas3D {
                     cam = stereoCamera;
                 }
 
+                const ctx = { renderer, camera: cam, scene, helper };
                 if (MultiSamplePass.isEnabled(p.multiSample)) {
-                    if (!cameraChanged) {
-                        while (!multiSampleHelper.render(renderer, cam, scene, helper, true, p.transparentBackground, p));
-                    } else {
-                        multiSampleHelper.render(renderer, cam, scene, helper, true, p.transparentBackground, p);
-                    }
+                    const forceOn = !cameraChanged && allowMulti && !controls.props.spin;
+                    multiSampleHelper.render(ctx, p, true, forceOn);
                 } else {
-                    passes.draw.render(renderer, cam, scene, helper, true, p.transparentBackground, p.postprocessing, p.marking);
+                    passes.draw.render(ctx, p, true);
                 }
                 pickHelper.dirty = true;
                 didRender = true;
@@ -416,15 +421,15 @@ namespace Canvas3D {
         let currentTime = 0;
         let drawPaused = false;
 
-        function draw(force?: boolean) {
+        function draw(options?: { force?: boolean, allowMulti?: boolean }) {
             if (drawPaused) return;
-            if (render(!!force) && notifyDidDraw) {
+            if (render(!!options?.force, !!options?.allowMulti) && notifyDidDraw) {
                 didDraw.next(now() - startTime as now.Timestamp);
             }
         }
 
-        function requestDraw(force?: boolean) {
-            forceNextRender = forceNextRender || !!force;
+        function requestDraw() {
+            forceNextRender = true;
         }
 
         let animationFrameHandle = 0;
@@ -438,8 +443,8 @@ namespace Canvas3D {
                 return;
             }
 
-            draw(false);
-            if (!camera.transition.inTransition && !webgl.isContextLost) {
+            draw();
+            if (!camera.transition.inTransition && !controls.props.spin && !webgl.isContextLost) {
                 interactionHelper.tick(currentTime);
             }
         }
@@ -478,7 +483,7 @@ namespace Canvas3D {
                 resolveCameraReset();
                 if (forceDrawAfterAllCommited) {
                     if (helper.debug.isEnabled) helper.debug.update();
-                    draw(true);
+                    draw({ force: true });
                     forceDrawAfterAllCommited = false;
                 }
                 commited.next(now());
@@ -661,11 +666,11 @@ namespace Canvas3D {
 
         const contextRestoredSub = contextRestored.subscribe(() => {
             pickHelper.dirty = true;
-            draw(true);
+            draw({ force: true });
             // Unclear why, but in Chrome with wboit enabled the first `draw` only clears
             // the drawingBuffer. Note that in Firefox the drawingBuffer is preserved after
             // context loss so it is unclear if it behaves the same.
-            draw(true);
+            draw({ force: true });
         });
 
         const resized = new BehaviorSubject<any>(0);
@@ -674,7 +679,7 @@ namespace Canvas3D {
             passes.updateSize();
             updateViewport();
             syncViewport();
-            if (draw) requestDraw(true);
+            if (draw) requestDraw();
             resized.next(+new Date());
         }
 
@@ -699,7 +704,7 @@ namespace Canvas3D {
                 reprRenderObjects.clear();
                 scene.clear();
                 helper.debug.clear();
-                requestDraw(true);
+                requestDraw();
                 reprCount.next(reprRenderObjects.size);
             },
             syncVisibility: () => {
@@ -711,7 +716,7 @@ namespace Canvas3D {
                 if (scene.syncVisibility()) {
                     if (helper.debug.isEnabled) helper.debug.update();
                 }
-                requestDraw(true);
+                requestDraw();
             },
 
             requestDraw,
@@ -799,7 +804,7 @@ namespace Canvas3D {
                 }
 
                 if (!doNotRequestDraw) {
-                    requestDraw(true);
+                    requestDraw();
                 }
             },
             getImagePass: (props: Partial<ImageProps> = {}) => {

src/mol-canvas3d/passes/draw.ts

@@ -53,6 +53,19 @@ function getDepthMergeRenderable(ctx: WebGLContext, depthTexturePrimitives: Text
     return createComputeRenderable(renderItem, values);
 }
 
+type Props = {
+    postprocessing: PostprocessingProps
+    marking: MarkingProps
+    transparentBackground: boolean;
+}
+
+type RenderContext = {
+    renderer: Renderer;
+    camera: Camera | StereoCamera;
+    scene: Scene;
+    helper: Helper;
+}
+
 export class DrawPass {
     private readonly drawTarget: RenderTarget;
 
@@ -264,25 +277,25 @@ export class DrawPass {
         renderer.renderBlendedTransparent(scene.primitives, camera, null);
     }
 
-    private _render(renderer: Renderer, camera: ICamera, scene: Scene, helper: Helper, toDrawingBuffer: boolean, transparentBackground: boolean, postprocessingProps: PostprocessingProps, markingProps: MarkingProps) {
+    private _render(renderer: Renderer, camera: ICamera, scene: Scene, helper: Helper, toDrawingBuffer: boolean, props: Props) {
         const volumeRendering = scene.volumes.renderables.length > 0;
-        const postprocessingEnabled = PostprocessingPass.isEnabled(postprocessingProps);
-        const antialiasingEnabled = AntialiasingPass.isEnabled(postprocessingProps);
-        const markingEnabled = MarkingPass.isEnabled(markingProps);
+        const postprocessingEnabled = PostprocessingPass.isEnabled(props.postprocessing);
+        const antialiasingEnabled = AntialiasingPass.isEnabled(props.postprocessing);
+        const markingEnabled = MarkingPass.isEnabled(props.marking);
 
         const { x, y, width, height } = camera.viewport;
         renderer.setViewport(x, y, width, height);
         renderer.update(camera);
 
-        if (transparentBackground && !antialiasingEnabled && toDrawingBuffer) {
+        if (props.transparentBackground && !antialiasingEnabled && toDrawingBuffer) {
             this.drawTarget.bind();
             renderer.clear(false);
         }
 
         if (this.wboitEnabled) {
-            this._renderWboit(renderer, camera, scene, transparentBackground, postprocessingProps);
+            this._renderWboit(renderer, camera, scene, props.transparentBackground, props.postprocessing);
         } else {
-            this._renderBlended(renderer, camera, scene, !volumeRendering && !postprocessingEnabled && !antialiasingEnabled && toDrawingBuffer, transparentBackground, postprocessingProps);
+            this._renderBlended(renderer, camera, scene, !volumeRendering && !postprocessingEnabled && !antialiasingEnabled && toDrawingBuffer, props.transparentBackground, props.postprocessing);
         }
 
         if (postprocessingEnabled) {
@@ -294,7 +307,7 @@ export class DrawPass {
         }
 
         if (markingEnabled) {
-            const markingDepthTest = markingProps.ghostEdgeStrength < 1;
+            const markingDepthTest = props.marking.ghostEdgeStrength < 1;
             if (markingDepthTest) {
                 this.marking.depthTarget.bind();
                 renderer.clear(false);
@@ -305,7 +318,7 @@ export class DrawPass {
             renderer.clear(false);
             renderer.renderMarkingMask(scene.primitives, camera, markingDepthTest ? this.marking.depthTarget.texture : null);
 
-            this.marking.update(markingProps);
+            this.marking.update(props.marking);
             this.marking.render(camera.viewport, postprocessingEnabled ? this.postprocessing.target : this.colorTarget);
         }
 
@@ -323,7 +336,7 @@ export class DrawPass {
         }
 
         if (antialiasingEnabled) {
-            this.antialiasing.render(camera, toDrawingBuffer, postprocessingProps);
+            this.antialiasing.render(camera, toDrawingBuffer, props.postprocessing);
         } else if (toDrawingBuffer) {
             this.drawTarget.bind();
 
@@ -338,16 +351,17 @@ export class DrawPass {
         this.webgl.gl.flush();
     }
 
-    render(renderer: Renderer, camera: Camera | StereoCamera, scene: Scene, helper: Helper, toDrawingBuffer: boolean, transparentBackground: boolean, postprocessingProps: PostprocessingProps, markingProps: MarkingProps) {
-        renderer.setTransparentBackground(transparentBackground);
+    render(ctx: RenderContext, props: Props, toDrawingBuffer: boolean) {
+        const { renderer, camera, scene, helper } = ctx;
+        renderer.setTransparentBackground(props.transparentBackground);
         renderer.setDrawingBufferSize(this.colorTarget.getWidth(), this.colorTarget.getHeight());
         renderer.setPixelRatio(this.webgl.pixelRatio);
 
         if (StereoCamera.is(camera)) {
-            this._render(renderer, camera.left, scene, helper, toDrawingBuffer, transparentBackground, postprocessingProps, markingProps);
-            this._render(renderer, camera.right, scene, helper, toDrawingBuffer, transparentBackground, postprocessingProps, markingProps);
+            this._render(renderer, camera.left, scene, helper, toDrawingBuffer, props);
+            this._render(renderer, camera.right, scene, helper, toDrawingBuffer, props);
         } else {
-            this._render(renderer, camera, scene, helper, toDrawingBuffer, transparentBackground, postprocessingProps, markingProps);
+            this._render(renderer, camera, scene, helper, toDrawingBuffer, props);
         }
     }
 

src/mol-canvas3d/passes/image.ts

@@ -83,11 +83,12 @@ export class ImagePass {
         Viewport.set(this._camera.viewport, 0, 0, this._width, this._height);
         this._camera.update();
 
+        const ctx = { renderer: this.renderer, camera: this._camera, scene: this.scene, helper: this.helper };
         if (MultiSamplePass.isEnabled(this.props.multiSample)) {
-            this.multiSampleHelper.render(this.renderer, this._camera, this.scene, this.helper, false, this.props.transparentBackground, this.props);
+            this.multiSampleHelper.render(ctx, this.props, false);
             this._colorTarget = this.multiSamplePass.colorTarget;
         } else {
-            this.drawPass.render(this.renderer, this._camera, this.scene, this.helper, false, this.props.transparentBackground, this.props.postprocessing, this.props.marking);
+            this.drawPass.render(ctx, this.props, false);
             this._colorTarget = this.drawPass.getColorTarget(this.props.postprocessing);
         }
     }

src/mol-canvas3d/passes/marking.ts

@@ -22,7 +22,7 @@ import { Color } from '../../mol-util/color';
 import { edge_frag } from '../../mol-gl/shader/marking/edge.frag';
 
 export const MarkingParams = {
-    enabled: PD.Boolean(false),
+    enabled: PD.Boolean(true),
     highlightEdgeColor: PD.Color(Color.darken(Color.fromNormalizedRgb(1.0, 0.4, 0.6), 1.0)),
     selectEdgeColor: PD.Color(Color.darken(Color.fromNormalizedRgb(0.2, 1.0, 0.1), 1.0)),
     edgeScale: PD.Numeric(1, { min: 1, max: 3, step: 1 }, { description: 'Thickness of the edge.' }),

src/mol-canvas3d/passes/multi-sample.ts

@@ -59,6 +59,14 @@ type Props = {
     multiSample: MultiSampleProps
     postprocessing: PostprocessingProps
     marking: MarkingProps
+    transparentBackground: boolean;
+}
+
+type RenderContext = {
+    renderer: Renderer;
+    camera: Camera | StereoCamera;
+    scene: Scene;
+    helper: Helper;
 }
 
 export class MultiSamplePass {
@@ -97,12 +105,12 @@ export class MultiSamplePass {
         }
     }
 
-    render(sampleIndex: number, renderer: Renderer, camera: Camera | StereoCamera, scene: Scene, helper: Helper, toDrawingBuffer: boolean, transparentBackground: boolean, props: Props) {
-        if (props.multiSample.mode === 'temporal') {
-            return this.renderTemporalMultiSample(sampleIndex, renderer, camera, scene, helper, toDrawingBuffer, transparentBackground, props);
+    render(sampleIndex: number, ctx: RenderContext, props: Props, toDrawingBuffer: boolean, forceOn: boolean) {
+        if (props.multiSample.mode === 'temporal' && !forceOn) {
+            return this.renderTemporalMultiSample(sampleIndex, ctx, props, toDrawingBuffer);
         } else {
-            this.renderMultiSample(renderer, camera, scene, helper, toDrawingBuffer, transparentBackground, props);
-            return sampleIndex;
+            this.renderMultiSample(ctx, toDrawingBuffer, props);
+            return -2;
         }
     }
 
@@ -114,7 +122,8 @@ export class MultiSamplePass {
         }
     }
 
-    private renderMultiSample(renderer: Renderer, camera: Camera | StereoCamera, scene: Scene, helper: Helper, toDrawingBuffer: boolean, transparentBackground: boolean, props: Props) {
+    private renderMultiSample(ctx: RenderContext, toDrawingBuffer: boolean, props: Props) {
+        const { camera } = ctx;
         const { compose, composeTarget, drawPass, webgl } = this;
         const { gl, state } = webgl;
 
@@ -148,7 +157,7 @@ export class MultiSamplePass {
             ValueCell.update(compose.values.uWeight, sampleWeight);
 
             // render scene
-            drawPass.render(renderer, camera, scene, helper, false, transparentBackground, props.postprocessing, props.marking);
+            drawPass.render(ctx, props, false);
 
             // compose rendered scene with compose target
             composeTarget.bind();
@@ -181,7 +190,8 @@ export class MultiSamplePass {
         camera.update();
     }
 
-    private renderTemporalMultiSample(sampleIndex: number, renderer: Renderer, camera: Camera | StereoCamera, scene: Scene, helper: Helper, toDrawingBuffer: boolean, transparentBackground: boolean, props: Props) {
+    private renderTemporalMultiSample(sampleIndex: number, ctx: RenderContext, props: Props, toDrawingBuffer: boolean) {
+        const { camera } = ctx;
         const { compose, composeTarget, holdTarget, drawPass, webgl } = this;
         const { gl, state } = webgl;
 
@@ -198,7 +208,7 @@ export class MultiSamplePass {
         const sampleWeight = 1.0 / offsetList.length;
 
         if (sampleIndex === -1) {
-            drawPass.render(renderer, camera, scene, helper, false, transparentBackground, props.postprocessing, props.marking);
+            drawPass.render(ctx, props, false);
             ValueCell.update(compose.values.uWeight, 1.0);
             ValueCell.update(compose.values.tColor, drawPass.getColorTarget(props.postprocessing).texture);
             compose.update();
@@ -226,7 +236,7 @@ export class MultiSamplePass {
                 camera.update();
 
                 // render scene
-                drawPass.render(renderer, camera, scene, helper, false, transparentBackground, props.postprocessing, props.marking);
+                drawPass.render(ctx, props, false);
 
                 // compose rendered scene with compose target
                 composeTarget.bind();
@@ -325,8 +335,8 @@ export class MultiSampleHelper {
     }
 
     /** Return `true` while more samples are needed */
-    render(renderer: Renderer, camera: Camera | StereoCamera, scene: Scene, helper: Helper, toDrawingBuffer: boolean, transparentBackground: boolean, props: Props) {
-        this.sampleIndex = this.multiSamplePass.render(this.sampleIndex, renderer, camera, scene, helper, toDrawingBuffer, transparentBackground, props);
+    render(ctx: RenderContext, props: Props, toDrawingBuffer: boolean, forceOn?: boolean) {
+        this.sampleIndex = this.multiSamplePass.render(this.sampleIndex, ctx, props, toDrawingBuffer, !!forceOn);
         return this.sampleIndex < 0;
     }
 

src/mol-geo/geometry/_spec/marker.spec.ts

@@ -0,0 +1,16 @@
+/**
+ * Copyright (c) 2021 mol* contributors, licensed under MIT, See LICENSE file for more info.
+ *
+ * @author Alexander Rose <alexander.rose@weirdbyte.de>
+ */
+
+import { getMarkersAverage } from '../marker-data';
+
+describe('marker-data', () => {
+    it('getMarkersAverage', () => {
+        expect(getMarkersAverage(new Uint8Array([0, 0, 0, 0]), 3)).toBe(0);
+        expect(getMarkersAverage(new Uint8Array([0, 0, 1, 0]), 3)).toBe(1 / 3);
+        expect(getMarkersAverage(new Uint8Array([0, 0, 0, 0]), 4)).toBe(0);
+        expect(getMarkersAverage(new Uint8Array([0, 0, 1, 0]), 4)).toBe(1 / 4);
+    });
+});

src/mol-geo/geometry/marker-data.ts

@@ -47,12 +47,19 @@ export function getMarkersAverage(array: Uint8Array, count: number): number {
     const backStart = 4 * viewEnd;
 
     let sum = 0;
-    for (let i = 0; i < viewEnd; ++i) {
-        const v = view[i];
-        sum += MarkerCountLut[v & 0xFFFF] + MarkerCountLut[v >> 16];
-    }
-    for (let i = backStart; i < count; ++i) {
-        sum += array[i] && 1;
+    if (viewEnd < 0) {
+        // avoid edge cases with small arrays
+        for (let i = 0; i < count; ++i) {
+            sum += array[i] && 1;
+        }
+    } else {
+        for (let i = 0; i < viewEnd; ++i) {
+            const v = view[i];
+            sum += MarkerCountLut[v & 0xFFFF] + MarkerCountLut[v >> 16];
+        }
+        for (let i = backStart; i < count; ++i) {
+            sum += array[i] && 1;
+        }
     }
     return sum / count;
 }

src/mol-gl/renderer.ts

@@ -89,8 +89,8 @@ export const RendererParams = {
 
     highlightColor: PD.Color(Color.fromNormalizedRgb(1.0, 0.4, 0.6)),
     selectColor: PD.Color(Color.fromNormalizedRgb(0.2, 1.0, 0.1)),
-    highlightStrength: PD.Numeric(0.7, { min: 0.0, max: 1.0, step: 0.1 }),
-    selectStrength: PD.Numeric(0.7, { min: 0.0, max: 1.0, step: 0.1 }),
+    highlightStrength: PD.Numeric(0.3, { min: 0.0, max: 1.0, step: 0.1 }),
+    selectStrength: PD.Numeric(0.3, { min: 0.0, max: 1.0, step: 0.1 }),
     markerPriority: PD.Select(1, [[1, 'Highlight'], [2, 'Select']]),
 
     xrayEdgeFalloff: PD.Numeric(1, { min: 0.0, max: 3.0, step: 0.1 }),

src/mol-plugin-state/manager/structure/selection.ts

@@ -245,7 +245,6 @@ export class StructureSelectionManager extends StatefulPluginComponent<Structure
     }
 
     private onUpdate(ref: string, oldObj: PSO.Molecule.Structure | undefined, obj: PSO.Molecule.Structure) {
-
         // no change to structure
         if (oldObj === obj || oldObj?.data === obj.data) return;
 
@@ -253,7 +252,9 @@ export class StructureSelectionManager extends StatefulPluginComponent<Structure
         const cell = this.plugin.helpers.substructureParent.get(obj.data, true);
         if (!cell) return;
 
-        ref = cell.transform.ref;
+        // only need to update the root
+        if (ref !== cell.transform.ref) return;
+
         if (!this.entries.has(ref)) return;
 
         // use structure from last decorator as reference

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

@@ -57,7 +57,7 @@ export const HighlightLoci = PluginBehavior.create({
                 if (!this.ctx.canvas3d || this.ctx.isBusy) return;
 
                 const loci = this.getLoci(current.loci);
-                if (this.params.ignore?.indexOf(loci.kind) >= 0) {
+                if (this.params.ignore.includes(loci.kind)) {
                     this.ctx.managers.interactivity.lociHighlights.highlightOnly({ repr: current.repr, loci: EmptyLoci });
                     return;
                 }
@@ -161,7 +161,7 @@ export const SelectLoci = PluginBehavior.create({
                 if (!this.ctx.canvas3d || this.ctx.isBusy || !this.ctx.selectionMode) return;
 
                 const loci = this.getLoci(current.loci);
-                if (this.params.ignore?.indexOf(loci.kind) >= 0) return;
+                if (this.params.ignore.includes(loci.kind)) return;
 
                 // only trigger the 1st action that matches
                 for (const [binding, action, condition] of actions) {
@@ -186,7 +186,7 @@ export const SelectLoci = PluginBehavior.create({
                         Structure.areEquivalent(structure, oldStructure) &&
                         Structure.areHierarchiesEqual(structure, oldStructure)) return;
 
-                    const reprs = this.ctx.state.data.select(StateSelection.Generators.ofType(SO.Molecule.Structure.Representation3D, ref));
+                    const reprs = this.ctx.state.data.select(StateSelection.children(ref).ofType(SO.Molecule.Structure.Representation3D));
                     for (const repr of reprs) this.applySelectMark(repr.transform.ref, true);
                 }
             });

src/mol-repr/structure/complex-representation.ts

@@ -84,7 +84,7 @@ export function ComplexRepresentation<P extends StructureParams>(label: string,
             if (!Structure.areRootsEquivalent(loci.structure, _structure)) return false;
             // Remap `loci` from equivalent structure to the current `_structure`
             loci = Loci.remap(loci, _structure);
-            if (StructureElement.Loci.is(loci) && StructureElement.Loci.isWholeStructure(loci)) {
+            if (Structure.isLoci(loci) || (StructureElement.Loci.is(loci) && StructureElement.Loci.isWholeStructure(loci))) {
                 // Change to `EveryLoci` to allow for downstream optimizations
                 loci = EveryLoci;
             }

src/mol-repr/structure/units-representation.ts

@@ -202,7 +202,7 @@ export function UnitsRepresentation<P extends StructureParams>(label: string, ct
             if (!Structure.areRootsEquivalent(loci.structure, _structure)) return false;
             // Remap `loci` from equivalent structure to the current `_structure`
             loci = Loci.remap(loci, _structure);
-            if (StructureElement.Loci.is(loci) && StructureElement.Loci.isWholeStructure(loci)) {
+            if (Structure.isLoci(loci) || (StructureElement.Loci.is(loci) && StructureElement.Loci.isWholeStructure(loci))) {
                 // Change to `EveryLoci` to allow for downstream optimizations
                 loci = EveryLoci;
             }