Skip to content

Commit

Permalink
Robustify is_paused and hold_render logic
Browse files Browse the repository at this point in the history
  • Loading branch information
mattpap committed May 1, 2024
1 parent c474ec6 commit 960557d
Show file tree
Hide file tree
Showing 2 changed files with 19 additions and 32 deletions.
41 changes: 14 additions & 27 deletions bokehjs/src/lib/models/plots/plot_canvas.ts
Original file line number Diff line number Diff line change
Expand Up @@ -114,8 +114,6 @@ export class PlotView extends LayoutDOMView implements Paintable {
this._range_manager.invalidate_dataranges = value
}

protected _is_paused?: number

protected lod_started: boolean

protected _initial_state: StateInfo
Expand Down Expand Up @@ -168,29 +166,22 @@ export class PlotView extends LayoutDOMView implements Paintable {
yield* this.tool_views.values()
}

get is_paused(): boolean {
return this._is_paused != null && this._is_paused !== 0
}

get child_models(): LayoutDOM[] {
return []
}

private _is_paused: number = 0
get is_paused(): boolean {
return this._is_paused != 0
}

pause(): void {
if (this._is_paused == null) {
this._is_paused = 1
} else {
this._is_paused += 1
}
this._is_paused += 1
}

unpause(no_render: boolean = false): void {
if (this._is_paused == null) {
throw new Error("wasn't paused")
}

this._is_paused = Math.max(this._is_paused - 1, 0)
if (this._is_paused == 0 && !no_render) {
this._is_paused = max(this._is_paused - 1, 0)
if (!this.is_paused && !no_render) {
this.request_repaint()
}
}
Expand Down Expand Up @@ -869,7 +860,11 @@ export class PlotView extends LayoutDOMView implements Paintable {
})

const {hold_render} = this.model.properties
this.on_change(hold_render, () => this._hold_render_changed())
this.on_change(hold_render, () => {
if (!this.model.hold_render) {
this.request_repaint()
}
})
}

override has_finished(): boolean {
Expand Down Expand Up @@ -972,7 +967,7 @@ export class PlotView extends LayoutDOMView implements Paintable {
}

paint(): void {
if (this.is_paused) {
if (this.is_paused || this.model.hold_render) {
return
}

Expand Down Expand Up @@ -1198,14 +1193,6 @@ export class PlotView extends LayoutDOMView implements Paintable {
return {...state, children: [...children ?? [], frame, ...renderers]}
}

protected _hold_render_changed(): void {
if (this.model.hold_render) {
this.pause()
} else {
this.unpause()
}
}

override resolve_frame(): View | null {
return this.frame as any // TODO CartesianFrameView (PR #13286)
}
Expand Down
10 changes: 5 additions & 5 deletions bokehjs/test/integration/plots.ts
Original file line number Diff line number Diff line change
Expand Up @@ -356,11 +356,11 @@ describe("Plot", () => {
p.hold_render = false
await view.ready

// TODO: expect(spy.callCount).to.be.equal(1)
//
// There should be exactly one paint after all this, but currently
// it varies between 2 and 3, due to layout and other factors. It's
// sufficient though, to compare images for a complete test.
// Two _actual_paint() calls because of how layout is integrated into
// the painting pipeline. This doesn't mean that renderers are actually
// painted twice, because paint invalidation logic prevents this. It's
// still quite confusing and requires a redesign.
expect(spy.callCount).to.be.equal(2)
})
})
})

0 comments on commit 960557d

Please sign in to comment.