mol-canvas/gl: refactored scene add/remove object sync

src/mol-canvas3d/canvas3d.ts

@@ -2,6 +2,7 @@
  * Copyright (c) 2018-2020 mol* contributors, licensed under MIT, See LICENSE file for more info.
  *
  * @author Alexander Rose <alexander.rose@weirdbyte.de>
+ * @author David Sehnal <david.sehnal@gmail.com>
  */
 
 import { BehaviorSubject, Subscription } from 'rxjs';
@@ -56,30 +57,33 @@ export { Canvas3D }
 interface Canvas3D {
     readonly webgl: WebGLContext,
 
-    add: (repr: Representation.Any) => Promise<void>
-    remove: (repr: Representation.Any) => Promise<void>
-    update: (repr?: Representation.Any, keepBoundingSphere?: boolean) => void
-    clear: () => void
-
-    // draw: (force?: boolean) => void
-    requestDraw: (force?: boolean) => void
-    animate: () => void
-    identify: (x: number, y: number) => PickingId | undefined
-    mark: (loci: Representation.Loci, action: MarkerAction) => void
-    getLoci: (pickingId: PickingId) => Representation.Loci
+    add(repr: Representation.Any): void
+    remove(repr: Representation.Any): void
+    /**
+     * This function must be called if animate() is not set up so that add/remove actions take place.
+     */
+    commit(): void
+    update(repr?: Representation.Any, keepBoundingSphere?: boolean): void
+    clear(): void
+
+    requestDraw(force?: boolean): void
+    animate(): void
+    identify(x: number, y: number): PickingId | undefined
+    mark(loci: Representation.Loci, action: MarkerAction): void
+    getLoci(pickingId: PickingId): Representation.Loci
 
     readonly didDraw: BehaviorSubject<now.Timestamp>
     readonly reprCount: BehaviorSubject<number>
 
-    handleResize: () => void
+    handleResize(): void
     /** Focuses camera on scene's bounding sphere, centered and zoomed. */
-    resetCamera: () => void
+    requestCameraReset(): void
     readonly camera: Camera
     readonly boundingSphere: Readonly<Sphere3D>
-    downloadScreenshot: () => void
-    getPixelData: (variant: GraphicsRenderVariant) => PixelData
-    setProps: (props: Partial<Canvas3DProps>) => void
-    getImagePass: () => ImagePass
+    downloadScreenshot(): void
+    getPixelData(variant: GraphicsRenderVariant): PixelData
+    setProps(props: Partial<Canvas3DProps>): void
+    getImagePass(): ImagePass
 
     /** Returns a copy of the current Canvas3D instance props */
     readonly props: Readonly<Canvas3DProps>
@@ -87,7 +91,7 @@ interface Canvas3D {
     readonly stats: RendererStats
     readonly interaction: Canvas3dInteractionHelper['events']
 
-    dispose: () => void
+    dispose(): void
 }
 
 const requestAnimationFrame = typeof window !== 'undefined' ? window.requestAnimationFrame : (f: (time: number) => void) => setImmediate(()=>f(Date.now()))
@@ -220,7 +224,7 @@ namespace Canvas3D {
         }
 
         function render(force: boolean) {
-            if (scene.isCommiting || webgl.isContextLost) return false
+            if (webgl.isContextLost) return false
 
             let didRender = false
             controls.update(currentTime)
@@ -262,7 +266,11 @@ namespace Canvas3D {
 
         function animate() {
             currentTime = now();
+            
+            commit();
+
             camera.transition.tick(currentTime);
+
             draw(false);
             if (!camera.transition.inTransition && !webgl.isContextLost) {
                 interactionHelper.tick(currentTime);
@@ -274,22 +282,31 @@ namespace Canvas3D {
             return webgl.isContextLost ? undefined : pickPass.identify(x, y)
         }
 
-        async function commit(renderObjects?: readonly GraphicsRenderObject[]) {
-            scene.update(renderObjects, false)
+        function commit() {
+            commitScene();
+            resolveCameraReset();
+        }
 
-            return runTask(scene.commit()).then(() => {
-                if (cameraResetRequested && !scene.isCommiting) {
-                    const { center, radius } = scene.boundingSphere
-                    camera.focus(center, radius, radius)
-                    cameraResetRequested = false
-                }
-                if (debugHelper.isEnabled) debugHelper.update()
-                requestDraw(true)
-                reprCount.next(reprRenderObjects.size)
-            })
+        function resolveCameraReset() {
+            if (!cameraResetRequested) return;
+            const { center, radius } = scene.boundingSphere;
+            camera.focus(center, radius, radius, p.cameraResetDurationMs);
+            cameraResetRequested = false;
+        }
+
+        let isDirty = false;
+        function commitScene() {
+            if (!isDirty) return;
+
+            scene.syncCommit();
+            if (debugHelper.isEnabled) debugHelper.update();
+            reprCount.next(reprRenderObjects.size);
+            isDirty = false;
         }
 
         function add(repr: Representation.Any) {
+            registerAutoUpdate(repr);
+
             const oldRO = reprRenderObjects.get(repr)
             const newRO = new Set<GraphicsRenderObject>()
             repr.renderObjects.forEach(o => newRO.add(o))
@@ -303,7 +320,37 @@ namespace Canvas3D {
                 repr.renderObjects.forEach(o => scene.add(o))
             }
             reprRenderObjects.set(repr, newRO)
-            return commit(repr.renderObjects)
+
+            scene.update(repr.renderObjects, false)
+            isDirty = true;
+        }
+
+        function remove(repr: Representation.Any) {
+            unregisterAutoUpdate(repr);
+
+            const renderObjects = reprRenderObjects.get(repr)
+            if (renderObjects) {
+                renderObjects.forEach(o => scene.remove(o))
+                reprRenderObjects.delete(repr)
+                scene.update(repr.renderObjects, false, true)
+                isDirty = true;
+            }
+        }
+
+        function registerAutoUpdate(repr: Representation.Any) {
+            if (reprUpdatedSubscriptions.has(repr)) return;
+
+            reprUpdatedSubscriptions.set(repr, repr.updated.subscribe(_ => {
+                if (!repr.state.syncManually) add(repr);
+            }))
+        }
+
+        function unregisterAutoUpdate(repr: Representation.Any) {
+            const updatedSubscription = reprUpdatedSubscriptions.get(repr);
+            if (updatedSubscription) {
+                updatedSubscription.unsubscribe();
+                reprUpdatedSubscriptions.delete(repr);
+            }
         }
 
         handleResize()
@@ -311,25 +358,9 @@ namespace Canvas3D {
         return {
             webgl,
 
-            add: (repr: Representation.Any) => {
-                reprUpdatedSubscriptions.set(repr, repr.updated.subscribe(_ => {
-                    if (!repr.state.syncManually) add(repr)
-                }))
-                return add(repr)
-            },
-            remove: (repr: Representation.Any) => {
-                const updatedSubscription = reprUpdatedSubscriptions.get(repr)
-                if (updatedSubscription) {
-                    updatedSubscription.unsubscribe()
-                }
-                const renderObjects = reprRenderObjects.get(repr)
-                if (renderObjects) {
-                    renderObjects.forEach(o => scene.remove(o))
-                    reprRenderObjects.delete(repr)
-                    return commit()
-                }
-                return Promise.resolve()
-            },
+            add,
+            remove,
+            commit,
             update: (repr, keepSphere) => {
                 if (repr) {
                     if (!reprRenderObjects.has(repr)) return;
@@ -356,14 +387,8 @@ namespace Canvas3D {
             getLoci,
 
             handleResize,
-            resetCamera: () => {
-                if (scene.isCommiting) {
-                    cameraResetRequested = true
-                } else {
-                    const { center, radius } = scene.boundingSphere
-                    camera.focus(center, radius, radius, p.cameraResetDurationMs)
-                    requestDraw(true);
-                }
+            requestCameraReset: () => {
+                cameraResetRequested = true;
             },
             camera,
             boundingSphere: scene.boundingSphere,

src/mol-gl/_spec/renderer.spec.ts

@@ -121,7 +121,7 @@ describe('renderer', () => {
         const points = createPoints()
 
         scene.add(points)
-        await scene.commit().run()
+        scene.syncCommit()
         expect(ctx.stats.resourceCounts.attribute).toBe(4);
         expect(ctx.stats.resourceCounts.texture).toBe(5);
         expect(ctx.stats.resourceCounts.vertexArray).toBe(5);
@@ -129,7 +129,7 @@ describe('renderer', () => {
         expect(ctx.stats.resourceCounts.shader).toBe(10);
 
         scene.remove(points)
-        await scene.commit().run()
+        scene.syncCommit()
         expect(ctx.stats.resourceCounts.attribute).toBe(0);
         expect(ctx.stats.resourceCounts.texture).toBe(0);
         expect(ctx.stats.resourceCounts.vertexArray).toBe(0);

src/mol-gl/scene.ts

@@ -1,7 +1,8 @@
 /**
- * Copyright (c) 2018-2019 mol* contributors, licensed under MIT, See LICENSE file for more info.
+ * Copyright (c) 2018-2020 mol* contributors, licensed under MIT, See LICENSE file for more info.
  *
  * @author Alexander Rose <alexander.rose@weirdbyte.de>
+ * @author David Sehnal <david.sehnal@gmail.com>
  */
 
 import { Renderable } from './renderable'
@@ -12,8 +13,7 @@ import { Object3D } from './object3d';
 import { Sphere3D } from '../mol-math/geometry';
 import { Vec3 } from '../mol-math/linear-algebra';
 import { BoundaryHelper } from '../mol-math/geometry/boundary-helper';
-import { RuntimeContext, Task } from '../mol-task';
-import { AsyncQueue } from '../mol-util/async-queue';
+import { arraySetAdd, arraySetRemove } from '../mol-util/array';
 
 const boundaryHelper = new BoundaryHelper();
 function calculateBoundingSphere(renderables: Renderable<RenderableValues & BaseValues>[], boundingSphere: Sphere3D): Sphere3D {
@@ -56,13 +56,13 @@ interface Scene extends Object3D {
     readonly count: number
     readonly renderables: ReadonlyArray<Renderable<RenderableValues & BaseValues>>
     readonly boundingSphere: Sphere3D
-    readonly isCommiting: boolean
+    // readonly isCommiting: boolean
 
-    update: (objects: ArrayLike<GraphicsRenderObject> | undefined, keepBoundingSphere: boolean) => void
+    update: (objects: ArrayLike<GraphicsRenderObject> | undefined, keepBoundingSphere: boolean, isRemoving?: boolean) => void
     add: (o: GraphicsRenderObject) => void // Renderable<any>
     remove: (o: GraphicsRenderObject) => void
     syncCommit: () => void
-    commit: () => Task<void>
+    // commit: () => Task<void>
     has: (o: GraphicsRenderObject) => boolean
     clear: () => void
     forEach: (callbackFn: (value: Renderable<RenderableValues & BaseValues>, key: GraphicsRenderObject) => void) => void
@@ -101,37 +101,17 @@ namespace Scene {
             }
         }
 
-        const commitQueue = new AsyncQueue<any>();
         const toAdd: GraphicsRenderObject[] = []
         const toRemove: GraphicsRenderObject[] = []
 
-        type CommitParams = { toAdd: GraphicsRenderObject[], toRemove: GraphicsRenderObject[] }
-
-        const step = 100
-        const handle = async (ctx: RuntimeContext, arr: GraphicsRenderObject[], fn: (o: GraphicsRenderObject) => void, message: string) => {
-            for (let i = 0, il = arr.length; i < il; i += step) {
-                if (ctx.shouldUpdate) await ctx.update({ message, current: i, max: il })
-                for (let j = i, jl = Math.min(i + step, il); j < jl; ++j) {
-                    fn(arr[j])
-                }
-            }
-        }
-
-        const commit = async (ctx: RuntimeContext, p: CommitParams) => {
-            await handle(ctx, p.toRemove, remove, 'Removing GraphicsRenderObjects')
-            await handle(ctx, p.toAdd, add, 'Adding GraphicsRenderObjects')
-            if (ctx.shouldUpdate) await ctx.update({ message: 'Sorting GraphicsRenderObjects' })
-            renderables.sort(renderableSort)
-        }
-
         return {
             get view () { return object3d.view },
             get position () { return object3d.position },
             get direction () { return object3d.direction },
             get up () { return object3d.up },
-            get isCommiting () { return commitQueue.length > 0 },
+            // get isCommiting () { return commitQueue.length > 0 },
 
-            update(objects, keepBoundingSphere) {
+            update(objects, keepBoundingSphere, isRemoving) {
                 Object3D.update(object3d)
                 if (objects) {
                     for (let i = 0, il = objects.length; i < il; ++i) {
@@ -139,7 +119,7 @@ namespace Scene {
                         if (!o) continue;
                         o.update();
                     }
-                } else {
+                } else if (!isRemoving) {
                     for (let i = 0, il = renderables.length; i < il; ++i) {
                         renderables[i].update()
                     }
@@ -147,10 +127,12 @@ namespace Scene {
                 if (!keepBoundingSphere) boundingSphereDirty = true
             },
             add: (o: GraphicsRenderObject) => {
-                toAdd.push(o)
+                arraySetAdd(toAdd, o);
+                arraySetRemove(toRemove, o);
             },
             remove: (o: GraphicsRenderObject) => {
-                toRemove.push(o)
+                arraySetAdd(toRemove, o);
+                arraySetRemove(toAdd, o);
             },
             syncCommit: () => {
                 for (let i = 0, il = toRemove.length; i < il; ++i) remove(toRemove[i])
@@ -159,24 +141,6 @@ namespace Scene {
                 toAdd.length = 0
                 renderables.sort(renderableSort)
             },
-            commit: () => {
-                const params = { toAdd: [ ...toAdd ], toRemove: [ ...toRemove ] }
-                toAdd.length = 0
-                toRemove.length = 0
-
-                return Task.create('Commiting GraphicsRenderObjects', async ctx => {
-                    const removed = await commitQueue.enqueue(params);
-                    if (!removed) return;
-
-                    try {
-                        await commit(ctx, params);
-                    } finally {
-                        commitQueue.handled(params);
-                    }
-                }, () => {
-                    commitQueue.remove(params);
-                })
-            },
             has: (o: GraphicsRenderObject) => {
                 return renderableMap.has(o)
             },

src/mol-plugin/behavior/static/camera.ts

@@ -16,7 +16,7 @@ export function registerDefault(ctx: PluginContext) {
 
 export function Reset(ctx: PluginContext) {
     PluginCommands.Camera.Reset.subscribe(ctx, () => {
-        ctx.canvas3d?.resetCamera();
+        ctx.canvas3d?.requestCameraReset();
     })
 }
 

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

@@ -20,7 +20,7 @@ export function SyncRepresentationToCanvas(ctx: PluginContext) {
 
     ctx.events.canvas3d.initialized.subscribe(() => {
         ctx.canvas3d?.reprCount.subscribe(v => {
-            if (reprCount === 0) ctx.canvas3d?.resetCamera();
+            if (reprCount === 0) ctx.canvas3d?.requestCameraReset();
             reprCount = v;
         });
     })

src/tests/browser/marching-cubes.ts

@@ -114,7 +114,7 @@ async function init() {
     const mcIsoSurfaceRepr = Representation.fromRenderObject('texture-mesh', mcIsoSurfaceRenderObject)
 
     canvas3d.add(mcIsoSurfaceRepr)
-    canvas3d.resetCamera()
+    canvas3d.requestCameraReset()
 
     //
 
@@ -141,7 +141,7 @@ async function init() {
     const meshRepr = Representation.fromRenderObject('mesh', meshRenderObject)
 
     canvas3d.add(meshRepr)
-    canvas3d.resetCamera()
+    canvas3d.requestCameraReset()
 }
 
 init()

src/tests/browser/render-lines.ts

@@ -41,4 +41,4 @@ function linesRepr() {
 }
 
 canvas3d.add(linesRepr())
-canvas3d.resetCamera()
+canvas3d.requestCameraReset()

src/tests/browser/render-mesh.ts

@@ -47,4 +47,4 @@ function meshRepr() {
 }
 
 canvas3d.add(meshRepr())
-canvas3d.resetCamera()
+canvas3d.requestCameraReset()

src/tests/browser/render-shape.ts

@@ -112,7 +112,7 @@ export async function init() {
     // Create shape from myData and add to canvas3d
     await repr.createOrUpdate({}, myData).run((p: Progress) => console.log(Progress.format(p)))
     canvas3d.add(repr)
-    canvas3d.resetCamera()
+    canvas3d.requestCameraReset()
 
     // Change color after 1s
     setTimeout(async () => {

src/tests/browser/render-spheres.ts

@@ -40,4 +40,4 @@ function spheresRepr() {
 }
 
 canvas3d.add(spheresRepr())
-canvas3d.resetCamera()
+canvas3d.requestCameraReset()

src/tests/browser/render-structure.ts

@@ -195,7 +195,7 @@ async function init() {
     if (show.ballAndStick) canvas3d.add(ballAndStickRepr)
     if (show.molecularSurface) canvas3d.add(molecularSurfaceRepr)
     if (show.gaussianSurface) canvas3d.add(gaussianSurfaceRepr)
-    canvas3d.resetCamera()
+    canvas3d.requestCameraReset()
     // canvas3d.setProps({ trackball: { ...canvas3d.props.trackball, spin: true } })
 }
 

src/tests/browser/render-text.ts

@@ -74,4 +74,4 @@ function spheresRepr() {
 
 canvas3d.add(textRepr())
 canvas3d.add(spheresRepr())
-canvas3d.resetCamera()
+canvas3d.requestCameraReset()