Skip to content

Commit

Permalink
Make layout code not use observable map again to fix slowness (#2097)
Browse files Browse the repository at this point in the history
* Drive mouseovers using layout data structure

* Intermediate

* Fixup breakpoint split view

* Fix unit tests

* Fix up some lint and tsc

* Standardize getById to return recttuple for both layouts

* Remove unused breakpoint split viewer from linear-comparative-view
  • Loading branch information
cmdcolin committed Jul 5, 2021
1 parent 7569d27 commit 46e5173
Show file tree
Hide file tree
Showing 21 changed files with 154 additions and 602 deletions.
1 change: 1 addition & 0 deletions packages/core/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@
"load-script2": "^2.0.5",
"object.fromentries": "^2.0.0",
"pako": "^1.0.10",
"rbush": "^3.0.1",
"react-error-boundary": "^3.0.0",
"react-intersection-observer": "^8.31.0",
"react-measure": "^2.3.0",
Expand Down
41 changes: 21 additions & 20 deletions packages/core/util/layouts/GranularRectLayout.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
import { ObservableMap } from 'mobx'
import { objectFromEntries } from '../index'
import {
RectTuple,
Expand Down Expand Up @@ -32,13 +31,13 @@ interface RowState<T> {
min: number
max: number
offset: number
bits: (Record<string, T> | boolean | undefined)[]
bits: (Record<string, T> | string | undefined)[]
}
// a single row in the layout
class LayoutRow<T> {
private padding: number

private allFilled?: Record<string, T> | boolean
private allFilled?: Record<string, T> | string

private widthLimit: number

Expand All @@ -61,11 +60,11 @@ class LayoutRow<T> {
// console.log(`r${this.rowNumber}: ${msg}`)
// }

setAllFilled(data: Record<string, T> | boolean): void {
setAllFilled(data: Record<string, T> | string): void {
this.allFilled = data
}

getItemAt(x: number): Record<string, T> | boolean | undefined {
getItemAt(x: number): Record<string, T> | string | undefined {
if (this.allFilled) {
return this.allFilled
}
Expand Down Expand Up @@ -129,7 +128,7 @@ class LayoutRow<T> {
// this.log(`initialize ${this.rowState.min} - ${this.rowState.max} (${this.rowState.bits.length})`)
}

addRect(rect: Rectangle<T>, data: Record<string, T> | boolean): void {
addRect(rect: Rectangle<T>, data: Record<string, T> | string): void {
const left = rect.l
const right = rect.r + this.padding // only padding on the right
if (!this.rowState) {
Expand Down Expand Up @@ -319,7 +318,7 @@ export default class GranularRectLayout<T> implements BaseLayout<T> {

private bitmap: LayoutRow<T>[]

private rectangles: ObservableMap<string, Rectangle<T>>
private rectangles: Map<string, Rectangle<T>>

public maxHeightReached: boolean

Expand All @@ -329,18 +328,21 @@ export default class GranularRectLayout<T> implements BaseLayout<T> {

private pTotalHeight: number

/*
*
* pitchX - layout grid pitch in the X direction
* pitchY - layout grid pitch in the Y direction
* maxHeight - maximum layout height, default Infinity (no max)
*/
constructor({
pitchX = 10,
pitchY = 10,
maxHeight = 10000,
hardRowLimit = 3000,
displayMode = 'normal',
}: {
/** layout grid pitch in the X direction */
pitchX?: number
/** layout grid pitch in the Y direction */
pitchY?: number
/** maximum layout height, default Infinity (no max) */
maxHeight?: number
displayMode?: string
hardRowLimit?: number
Expand All @@ -358,7 +360,7 @@ export default class GranularRectLayout<T> implements BaseLayout<T> {
}

this.bitmap = []
this.rectangles = new ObservableMap()
this.rectangles = new Map()
this.maxHeight = Math.ceil(maxHeight / this.pitchY)
this.pTotalHeight = 0 // total height, in units of bitmap squares (px/pitchY)
}
Expand All @@ -375,14 +377,14 @@ export default class GranularRectLayout<T> implements BaseLayout<T> {
data?: Record<string, T>,
): number | null {
// if we have already laid it out, return its layout
// console.log(`${this.id} add ${id}`)
const storedRec = this.rectangles.get(id)
if (storedRec) {
if (storedRec.top === null) {
return null
}

// add it to the bitmap again, since that bitmap range may have been discarded
// add it to the bitmap again, since that bitmap range may have been
// discarded
this.addRectToBitmap(storedRec)
return storedRec.top * this.pitchY
}
Expand All @@ -391,13 +393,11 @@ export default class GranularRectLayout<T> implements BaseLayout<T> {
const pRight = Math.floor(right / this.pitchX)
const pHeight = Math.ceil(height / this.pitchY)

// const midX = Math.floor((pLeft + pRight) / 2)
const rectangle: Rectangle<T> = {
id,
l: pLeft,
r: pRight,
top: null,
// mX: midX,
h: pHeight,
originalHeight: height,
data,
Expand All @@ -423,7 +423,6 @@ export default class GranularRectLayout<T> implements BaseLayout<T> {
this.addRectToBitmap(rectangle)
this.rectangles.set(id, rectangle)
this.pTotalHeight = Math.max(this.pTotalHeight || 0, top + pHeight)
// console.log(`G2 ${data.get('name')} ${top}`)
return top * this.pitchY
}

Expand Down Expand Up @@ -467,7 +466,7 @@ export default class GranularRectLayout<T> implements BaseLayout<T> {
return
}

const data = rect.data || true
const data = rect.data || rect.id
const { bitmap } = this
const yEnd = rect.top + rect.h
if (rect.r - rect.l > maxFeaturePitchWidth) {
Expand Down Expand Up @@ -508,7 +507,7 @@ export default class GranularRectLayout<T> implements BaseLayout<T> {
return this.rectangles.has(id)
}

getByCoord(x: number, y: number): Record<string, T> | boolean | undefined {
getByCoord(x: number, y: number): Record<string, T> | string | undefined {
const pY = Math.floor(y / this.pitchY)
const row = this.bitmap[pY]
if (!row) {
Expand All @@ -518,11 +517,13 @@ export default class GranularRectLayout<T> implements BaseLayout<T> {
return row.getItemAt(pX)
}

getByID(id: string): (Record<string, T> | boolean) | undefined {
getByID(id: string): RectTuple | undefined {
const r = this.rectangles.get(id)
if (r) {
return r.data || true
const t = (r.top as number) * this.pitchX
return [r.l * this.pitchX, t, r.r * this.pitchX, t + r.originalHeight]
}

return undefined
}

Expand Down
30 changes: 30 additions & 0 deletions packages/core/util/layouts/PrecomputedLayout.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,15 @@ import {
BaseLayout,
Rectangle,
} from './BaseLayout'
import RBush from 'rbush'

export interface Layout {
minX: number
minY: number
maxX: number
maxY: number
name: string
}

export default class PrecomputedLayout<T> implements BaseLayout<T> {
private rectangles: Map<string, RectTuple>
Expand All @@ -13,11 +22,23 @@ export default class PrecomputedLayout<T> implements BaseLayout<T> {

public maxHeightReached: boolean

private rbush: RBush<Layout>

constructor({ rectangles, totalHeight, maxHeightReached }: SerializedLayout) {
this.rectangles = new Map(Object.entries(rectangles))
// rectangles is of the form "featureId": [leftPx, topPx, rightPx, bottomPx]
this.totalHeight = totalHeight
this.maxHeightReached = maxHeightReached
this.rbush = new RBush()
for (const [key, layout] of Object.entries(rectangles)) {
this.rbush.insert({
minX: layout[0],
minY: layout[1],
maxX: layout[2],
maxY: layout[3],
name: key,
})
}
}

addRect(id: string) {
Expand All @@ -44,6 +65,15 @@ export default class PrecomputedLayout<T> implements BaseLayout<T> {
throw new Error('Method not implemented.')
}

getByCoord(x: number, y: number) {
const rect = { minX: x, minY: y, maxX: x + 1, maxY: y + 1 }
return this.rbush.collides(rect) ? this.rbush.search(rect)[0].name : []
}

getByID(id: string) {
return this.rectangles.get(id)
}

addRectToBitmap(_rect: Rectangle<T>, _data: Record<string, T>): void {
throw new Error('Method not implemented.')
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -65,8 +65,8 @@ const stateModelFactory = (
}
},

get layoutFeatures() {
return self.PileupDisplay.layoutFeatures
getFeatureByID(id: string) {
return self.PileupDisplay.getFeatureByID(id)
},

get features() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -151,12 +151,11 @@ function PileupRendering(props: {
const px = region.reversed ? width - offsetX : offsetX
const clientBp = region.start + bpPerPx * px

const feats = displayModel.getFeatureOverlapping(
const featIdUnderMouse = displayModel.getFeatureOverlapping(
blockKey,
clientBp,
offsetY,
)
const featIdUnderMouse = feats.length ? feats[0].name : undefined

if (onMouseMove) {
onMouseMove(event, featIdUnderMouse)
Expand Down
24 changes: 14 additions & 10 deletions plugins/breakpoint-split-view/src/BreakpointSplitView.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -23,25 +23,29 @@ const getView = () => {
type: 'FakeTrack',
configuration: types.frozen(),
displays: types.array(
types.model('FakeDisplay', {
type: 'FakeDisplay',
displayId: 'FakeDisplay',
configuration: types.frozen(),
features: types.frozen(),
layoutFeatures: types.frozen(),
}),
types
.model('FakeDisplay', {
type: 'FakeDisplay',
displayId: 'FakeDisplay',
configuration: types.frozen(),
layoutFeatures: types.frozen(),
features: types.frozen(),
})
.views(self => ({
getFeatureByID(id: string) {
return self.layoutFeatures[id]
},
})),
),
})
.actions(self => ({
afterCreate() {
self.displays[0].features = new Map(
Object.entries(self.displays[0].features),
)
self.displays[0].layoutFeatures = new Map(
Object.entries(self.displays[0].layoutFeatures),
)
},
}))

stubManager.addViewType(
() =>
new ViewType({
Expand Down
23 changes: 13 additions & 10 deletions plugins/breakpoint-split-view/src/model.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -23,23 +23,26 @@ const getView = () => {
type: 'FakeTrack',
configuration: types.frozen(),
displays: types.array(
types.model('FakeDisplay', {
type: 'FakeDisplay',
displayId: 'FakeDisplay',
configuration: types.frozen(),
features: types.frozen(),
layoutFeatures: types.frozen(),
}),
types
.model('FakeDisplay', {
type: 'FakeDisplay',
displayId: 'FakeDisplay',
configuration: types.frozen(),
features: types.frozen(),
layoutFeatures: types.frozen(),
})
.views(self => ({
getFeatureByID(id: string) {
return self.layoutFeatures[id]
},
})),
),
})
.actions(self => ({
afterCreate() {
self.displays[0].features = new Map(
Object.entries(self.displays[0].features),
)
self.displays[0].layoutFeatures = new Map(
Object.entries(self.displays[0].layoutFeatures),
)
},
}))
stubManager.addViewType(
Expand Down
5 changes: 3 additions & 2 deletions plugins/breakpoint-split-view/src/model.ts
Original file line number Diff line number Diff line change
Expand Up @@ -229,8 +229,9 @@ export default function stateModelFactory(pluginManager: any) {
// use reverse to search the second track first
const tracks = this.getMatchedTracks(trackConfigId)

const calc = (track: any, feat: Feature) =>
track.displays[0].layoutFeatures.get(feat.id())
const calc = (track: any, feat: Feature) => {
return track.displays[0].getFeatureByID(feat.id())
}

return features.map(c =>
c.map(feature => {
Expand Down
Loading

0 comments on commit 46e5173

Please sign in to comment.