From 8b7333b470eed9e8408c61ecbdca14f2aa24861a Mon Sep 17 00:00:00 2001 From: Alexander Rose <alexander.rose@weirdbyte.de> Date: Sat, 4 Sep 2021 14:48:09 -0700 Subject: [PATCH] avoid unnecessary draw calls/ui updates when marking - fix wrong type narrowing of Loci.isEmpty --- src/mol-canvas3d/canvas3d.ts | 6 +-- src/mol-model/loci.ts | 2 +- .../helpers/structure-clipping.ts | 4 +- .../helpers/structure-overpaint.ts | 4 +- .../helpers/structure-transparency.ts | 4 +- src/mol-plugin-state/manager/interactivity.ts | 40 +++++++++++++------ src/mol-plugin-state/manager/loci-label.ts | 6 +-- src/mol-plugin-ui/structure/measurements.tsx | 2 +- src/mol-plugin-ui/structure/superposition.tsx | 4 +- .../behavior/dynamic/representation.ts | 10 ++--- .../dynamic/volume-streaming/behavior.ts | 2 +- 11 files changed, 49 insertions(+), 35 deletions(-) diff --git a/src/mol-canvas3d/canvas3d.ts b/src/mol-canvas3d/canvas3d.ts index bae279c26..02366cb75 100644 --- a/src/mol-canvas3d/canvas3d.ts +++ b/src/mol-canvas3d/canvas3d.ts @@ -230,7 +230,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): void + mark(loci: Representation.Loci, action: MarkerAction, noDraw?: boolean): void getLoci(pickingId: PickingId | undefined): Representation.Loci notifyDidDraw: boolean, @@ -339,7 +339,7 @@ namespace Canvas3D { return { loci, repr }; } - function mark(reprLoci: Representation.Loci, action: MarkerAction) { + function mark(reprLoci: Representation.Loci, action: MarkerAction, noDraw = false) { const { repr, loci } = reprLoci; let changed = false; if (repr) { @@ -349,7 +349,7 @@ namespace Canvas3D { changed = helper.camera.mark(loci, action) || changed; reprRenderObjects.forEach((_, _repr) => { changed = _repr.mark(loci, action) || changed; }); } - if (changed) { + if (changed && !noDraw) { scene.update(void 0, true); helper.handle.scene.update(void 0, true); helper.camera.scene.update(void 0, true); diff --git a/src/mol-model/loci.ts b/src/mol-model/loci.ts index 43d52130e..6b90c0221 100644 --- a/src/mol-model/loci.ts +++ b/src/mol-model/loci.ts @@ -116,7 +116,7 @@ namespace Loci { return !!loci && loci.kind === 'every-loci'; } - export function isEmpty(loci: Loci): loci is EmptyLoci { + export function isEmpty(loci: Loci) { if (isEveryLoci(loci)) return false; if (isEmptyLoci(loci)) return true; if (isDataLoci(loci)) return isDataLociEmpty(loci); diff --git a/src/mol-plugin-state/helpers/structure-clipping.ts b/src/mol-plugin-state/helpers/structure-clipping.ts index 5b0a07ac4..5b51e52e9 100644 --- a/src/mol-plugin-state/helpers/structure-clipping.ts +++ b/src/mol-plugin-state/helpers/structure-clipping.ts @@ -11,7 +11,7 @@ import { StateTransforms } from '../../mol-plugin-state/transforms'; import { PluginContext } from '../../mol-plugin/context'; import { StateBuilder, StateObjectCell, StateSelection, StateTransform } from '../../mol-state'; import { StructureComponentRef } from '../manager/structure/hierarchy-state'; -import { EmptyLoci, Loci } from '../../mol-model/loci'; +import { EmptyLoci, isEmptyLoci, Loci } from '../../mol-model/loci'; import { Clipping } from '../../mol-theme/clipping'; type ClippingEachReprCallback = (update: StateBuilder.Root, repr: StateObjectCell<PluginStateObject.Molecule.Structure.Representation3D, StateTransform<typeof StateTransforms.Representation.StructureRepresentation3D>>, clipping?: StateObjectCell<any, StateTransform<typeof StateTransforms.Representation.ClippingStructureRepresentation3DFromBundle>>) => Promise<void> @@ -25,7 +25,7 @@ export async function setStructureClipping(plugin: PluginContext, components: St // always use the root structure to get the loci so the clipping // stays applicable as long as the root structure does not change const loci = await lociGetter(structure.root); - if (Loci.isEmpty(loci)) return; + if (Loci.isEmpty(loci) || isEmptyLoci(loci)) return; const layer = { bundle: StructureElement.Bundle.fromLoci(loci), diff --git a/src/mol-plugin-state/helpers/structure-overpaint.ts b/src/mol-plugin-state/helpers/structure-overpaint.ts index 2b4bfafa1..0cfbf3fd7 100644 --- a/src/mol-plugin-state/helpers/structure-overpaint.ts +++ b/src/mol-plugin-state/helpers/structure-overpaint.ts @@ -13,7 +13,7 @@ import { StateBuilder, StateObjectCell, StateSelection, StateTransform } from '. import { Overpaint } from '../../mol-theme/overpaint'; import { Color } from '../../mol-util/color'; import { StructureComponentRef } from '../manager/structure/hierarchy-state'; -import { EmptyLoci, Loci } from '../../mol-model/loci'; +import { EmptyLoci, isEmptyLoci, Loci } from '../../mol-model/loci'; type OverpaintEachReprCallback = (update: StateBuilder.Root, repr: StateObjectCell<PluginStateObject.Molecule.Structure.Representation3D, StateTransform<typeof StateTransforms.Representation.StructureRepresentation3D>>, overpaint?: StateObjectCell<any, StateTransform<typeof StateTransforms.Representation.OverpaintStructureRepresentation3DFromBundle>>) => Promise<void> const OverpaintManagerTag = 'overpaint-controls'; @@ -26,7 +26,7 @@ export async function setStructureOverpaint(plugin: PluginContext, components: S // always use the root structure to get the loci so the overpaint // stays applicable as long as the root structure does not change const loci = await lociGetter(structure.root); - if (Loci.isEmpty(loci)) return; + if (Loci.isEmpty(loci) || isEmptyLoci(loci)) return; const layer = { bundle: StructureElement.Bundle.fromLoci(loci), diff --git a/src/mol-plugin-state/helpers/structure-transparency.ts b/src/mol-plugin-state/helpers/structure-transparency.ts index 4bbaf793f..adf196072 100644 --- a/src/mol-plugin-state/helpers/structure-transparency.ts +++ b/src/mol-plugin-state/helpers/structure-transparency.ts @@ -11,7 +11,7 @@ import { StateTransforms } from '../../mol-plugin-state/transforms'; import { PluginContext } from '../../mol-plugin/context'; import { StateBuilder, StateObjectCell, StateSelection, StateTransform } from '../../mol-state'; import { StructureComponentRef } from '../manager/structure/hierarchy-state'; -import { EmptyLoci, Loci } from '../../mol-model/loci'; +import { EmptyLoci, isEmptyLoci, Loci } from '../../mol-model/loci'; import { Transparency } from '../../mol-theme/transparency'; type TransparencyEachReprCallback = (update: StateBuilder.Root, repr: StateObjectCell<PluginStateObject.Molecule.Structure.Representation3D, StateTransform<typeof StateTransforms.Representation.StructureRepresentation3D>>, transparency?: StateObjectCell<any, StateTransform<typeof StateTransforms.Representation.TransparencyStructureRepresentation3DFromBundle>>) => Promise<void> @@ -25,7 +25,7 @@ export async function setStructureTransparency(plugin: PluginContext, components // always use the root structure to get the loci so the transparency // stays applicable as long as the root structure does not change const loci = await lociGetter(structure.root); - if (Loci.isEmpty(loci)) return; + if (Loci.isEmpty(loci) || isEmptyLoci(loci)) return; const layer = { bundle: StructureElement.Bundle.fromLoci(loci), diff --git a/src/mol-plugin-state/manager/interactivity.ts b/src/mol-plugin-state/manager/interactivity.ts index bb2578011..5a7d5d144 100644 --- a/src/mol-plugin-state/manager/interactivity.ts +++ b/src/mol-plugin-state/manager/interactivity.ts @@ -1,5 +1,5 @@ /** - * Copyright (c) 2019-2020 mol* contributors, licensed under MIT, See LICENSE file for more info. + * Copyright (c) 2019-2021 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> @@ -74,7 +74,13 @@ namespace InteractivityManager { export interface DragEvent { current: Representation.Loci, buttons: ButtonsType, button: ButtonsType.Flag, modifiers: ModifiersKeys, pageStart: Vec2, pageEnd: Vec2 } export interface ClickEvent { current: Representation.Loci, buttons: ButtonsType, button: ButtonsType.Flag, modifiers: ModifiersKeys, page?: Vec2, position?: Vec3 } - export type LociMarkProvider = (loci: Representation.Loci, action: MarkerAction) => void + /** + * 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. + */ + export type LociMarkProvider = (loci: Representation.Loci, action: MarkerAction, /* test */ noRender?: boolean) => void export abstract class LociMarkManager { protected providers: LociMarkProvider[] = []; @@ -101,9 +107,9 @@ namespace InteractivityManager { return { loci: Loci.normalize(loci, granularity), repr }; } - protected mark(current: Representation.Loci, action: MarkerAction) { + protected mark(current: Representation.Loci, action: MarkerAction, noRender = false) { if (!Loci.isEmpty(current.loci)) { - for (let p of this.providers) p(current, action); + for (let p of this.providers) p(current, action, noRender); } } @@ -130,9 +136,9 @@ namespace InteractivityManager { this.prev.push(loci); } - clearHighlights = () => { + clearHighlights = (noRender = false) => { for (const p of this.prev) { - this.mark(p, MarkerAction.RemoveHighlight); + this.mark(p, MarkerAction.RemoveHighlight, noRender); } this.prev.length = 0; } @@ -147,21 +153,29 @@ namespace InteractivityManager { highlightOnly(current: Representation.Loci, applyGranularity = true) { const normalized = this.normalizedLoci(current, applyGranularity); if (!this.isHighlighted(normalized)) { - this.clearHighlights(); - this.addHighlight(normalized); + if (Loci.isEmpty(normalized.loci)) { + this.clearHighlights(); + } else { + this.clearHighlights(true); + this.addHighlight(normalized); + } } } highlightOnlyExtend(current: Representation.Loci, applyGranularity = true) { const normalized = this.normalizedLoci(current, applyGranularity); if (StructureElement.Loci.is(normalized.loci)) { - const loci = { + const extended = { loci: this.sel.tryGetRange(normalized.loci) || normalized.loci, repr: normalized.repr }; - if (!this.isHighlighted(loci)) { - this.clearHighlights(); - this.addHighlight(loci); + if (!this.isHighlighted(extended)) { + if (Loci.isEmpty(extended.loci)) { + this.clearHighlights(); + } else { + this.clearHighlights(true); + this.addHighlight(extended); + } } } } @@ -241,7 +255,7 @@ namespace InteractivityManager { // do a full deselect/select for the current structure so visuals that are // marked with granularity unequal to 'element' and join/intersect operations // are handled properly - super.mark({ loci: Structure.Loci(loci.structure) }, MarkerAction.Deselect); + super.mark({ loci: Structure.Loci(loci.structure) }, MarkerAction.Deselect, true); super.mark({ loci: this.sel.getLoci(loci.structure) }, MarkerAction.Select); } else { super.mark(current, action); diff --git a/src/mol-plugin-state/manager/loci-label.ts b/src/mol-plugin-state/manager/loci-label.ts index a5ee1eba6..1b7322cec 100644 --- a/src/mol-plugin-state/manager/loci-label.ts +++ b/src/mol-plugin-state/manager/loci-label.ts @@ -1,5 +1,5 @@ /** - * Copyright (c) 2018-2020 mol* contributors, licensed under MIT, See LICENSE file for more info. + * Copyright (c) 2018-2021 mol* contributors, licensed under MIT, See LICENSE file for more info. * * @author David Sehnal <david.sehnal@gmail.com> * @author Alexander Rose <alexander.rose@weirdbyte.de> @@ -92,9 +92,9 @@ export class LociLabelManager { } constructor(public ctx: PluginContext) { - ctx.managers.interactivity.lociHighlights.addProvider((loci, action) => { + ctx.managers.interactivity.lociHighlights.addProvider((loci, action, noRender) => { this.mark(loci, action); - this.showLabels(); + if (!noRender) this.showLabels(); }); } } \ No newline at end of file diff --git a/src/mol-plugin-ui/structure/measurements.tsx b/src/mol-plugin-ui/structure/measurements.tsx index b02af800b..44fab5df3 100644 --- a/src/mol-plugin-ui/structure/measurements.tsx +++ b/src/mol-plugin-ui/structure/measurements.tsx @@ -143,7 +143,7 @@ export class MeasurementControls extends PurePluginUIComponent<{}, { isBusy: boo historyEntry(e: StructureSelectionHistoryEntry, idx: number) { const history = this.plugin.managers.structure.selection.additionsHistory; return <div className='msp-flex-row' key={e.id}> - <Button noOverflow title='Click to focus. Hover to highlight.' onClick={() => this.focusLoci(e.loci)} style={{ width: 'auto', textAlign: 'left' }} onMouseEnter={() => this.highlight(e.loci)} onMouseLeave={this.plugin.managers.interactivity.lociHighlights.clearHighlights}> + <Button noOverflow title='Click to focus. Hover to highlight.' onClick={() => this.focusLoci(e.loci)} style={{ width: 'auto', textAlign: 'left' }} onMouseEnter={() => this.highlight(e.loci)} onMouseLeave={() => this.plugin.managers.interactivity.lociHighlights.clearHighlights()}> {idx}. <span dangerouslySetInnerHTML={{ __html: e.label }} /> </Button> {history.length > 1 && <IconButton svg={ArrowUpwardSvg} small={true} className='msp-form-control' onClick={() => this.moveHistory(e, 'up')} flex='20px' title={'Move up'} />} diff --git a/src/mol-plugin-ui/structure/superposition.tsx b/src/mol-plugin-ui/structure/superposition.tsx index 5c2033197..779e2bfdb 100644 --- a/src/mol-plugin-ui/structure/superposition.tsx +++ b/src/mol-plugin-ui/structure/superposition.tsx @@ -210,7 +210,7 @@ export class SuperpositionControls extends PurePluginUIComponent<{ }, Superposit lociEntry(e: LociEntry, idx: number) { return <div className='msp-flex-row' key={idx}> - <Button noOverflow title='Click to focus. Hover to highlight.' onClick={() => this.focusLoci(e.loci)} style={{ width: 'auto', textAlign: 'left' }} onMouseEnter={() => this.highlight(e.loci)} onMouseLeave={this.plugin.managers.interactivity.lociHighlights.clearHighlights}> + <Button noOverflow title='Click to focus. Hover to highlight.' onClick={() => this.focusLoci(e.loci)} style={{ width: 'auto', textAlign: 'left' }} onMouseEnter={() => this.highlight(e.loci)} onMouseLeave={() => this.plugin.managers.interactivity.lociHighlights.clearHighlights()}> <span dangerouslySetInnerHTML={{ __html: e.label }} /> </Button> </div>; @@ -219,7 +219,7 @@ export class SuperpositionControls extends PurePluginUIComponent<{ }, Superposit historyEntry(e: StructureSelectionHistoryEntry, idx: number) { const history = this.plugin.managers.structure.selection.additionsHistory; return <div className='msp-flex-row' key={e.id}> - <Button noOverflow title='Click to focus. Hover to highlight.' onClick={() => this.focusLoci(e.loci)} style={{ width: 'auto', textAlign: 'left' }} onMouseEnter={() => this.highlight(e.loci)} onMouseLeave={this.plugin.managers.interactivity.lociHighlights.clearHighlights}> + <Button noOverflow title='Click to focus. Hover to highlight.' onClick={() => this.focusLoci(e.loci)} style={{ width: 'auto', textAlign: 'left' }} onMouseEnter={() => this.highlight(e.loci)} onMouseLeave={() => this.plugin.managers.interactivity.lociHighlights.clearHighlights()}> {idx}. <span dangerouslySetInnerHTML={{ __html: e.label }} /> </Button> {history.length > 1 && <IconButton svg={ArrowUpwardSvg} small={true} className='msp-form-control' onClick={() => this.moveHistory(e, 'up')} flex='20px' title={'Move up'} />} diff --git a/src/mol-plugin/behavior/dynamic/representation.ts b/src/mol-plugin/behavior/dynamic/representation.ts index 874d77c7b..45928cbe5 100644 --- a/src/mol-plugin/behavior/dynamic/representation.ts +++ b/src/mol-plugin/behavior/dynamic/representation.ts @@ -1,5 +1,5 @@ /** - * Copyright (c) 2018-2020 mol* contributors, licensed under MIT, See LICENSE file for more info. + * Copyright (c) 2018-2021 mol* contributors, licensed under MIT, See LICENSE file for more info. * * @author David Sehnal <david.sehnal@gmail.com> * @author Alexander Rose <alexander.rose@weirdbyte.de> @@ -42,9 +42,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) => { + private lociMarkProvider = (interactionLoci: Representation.Loci, action: MarkerAction, noRender?: boolean) => { if (!this.ctx.canvas3d || !this.params.mark) return; - this.ctx.canvas3d.mark(interactionLoci, action); + this.ctx.canvas3d.mark(interactionLoci, action, noRender); } register() { this.subscribeObservable(this.ctx.behaviors.interaction.hover, ({ current, buttons, modifiers }) => { @@ -104,9 +104,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) => { + private lociMarkProvider = (reprLoci: Representation.Loci, action: MarkerAction, noRender?: boolean) => { if (!this.ctx.canvas3d || !this.params.mark) return; - this.ctx.canvas3d.mark({ loci: reprLoci.loci }, action); + this.ctx.canvas3d.mark({ loci: reprLoci.loci }, action, noRender); } private applySelectMark(ref: string, clear?: boolean) { const cell = this.ctx.state.data.cells.get(ref); diff --git a/src/mol-plugin/behavior/dynamic/volume-streaming/behavior.ts b/src/mol-plugin/behavior/dynamic/volume-streaming/behavior.ts index b0291a7a8..2597e1c10 100644 --- a/src/mol-plugin/behavior/dynamic/volume-streaming/behavior.ts +++ b/src/mol-plugin/behavior/dynamic/volume-streaming/behavior.ts @@ -305,7 +305,7 @@ export namespace VolumeStreaming { private _invTransform: Mat4 = Mat4(); private getBoxFromLoci(loci: StructureElement.Loci | EmptyLoci): Box3D { - if (Loci.isEmpty(loci)) { + if (Loci.isEmpty(loci) || isEmptyLoci(loci)) { return Box3D(); } -- GitLab