Fix bokehjs' SVG export when ToolbarBox is present (#10713)

* Move CanvasLayer to core/util/canvas

* Fix expect(fn).to.not.throw assertion

* Make ToolbarBase behave even more like LayoutDOM

* Add integration tests

bokehjs/src/lib/core/util/canvas.ts

@@ -1,3 +1,8 @@
+import {SVGRenderingContext2D} from "./svg"
+import {BBox} from "./bbox"
+import {div, canvas} from "../dom"
+import {OutputBackend} from "../enums"
+
 export type Context2d = {
   setImageSmoothingEnabled(value: boolean): void
   getImageSmoothingEnabled(): boolean
@@ -73,9 +78,109 @@ function fixup_ellipse(ctx: any): void {
     ctx.ellipse = ellipse_bezier
 }
 
-export function fixup_ctx(ctx: any): void {
+function fixup_ctx(ctx: any): void {
   fixup_line_dash(ctx)
   fixup_image_smoothing(ctx)
   fixup_measure_text(ctx)
   fixup_ellipse(ctx)
 }
+
+const style = {
+  position: "absolute",
+  top: "0",
+  left: "0",
+  width: "100%",
+  height: "100%",
+}
+
+export class CanvasLayer {
+  private readonly _canvas: HTMLCanvasElement | SVGSVGElement
+  get canvas(): HTMLCanvasElement {
+    return this._canvas as HTMLCanvasElement
+  }
+
+  private readonly _ctx: CanvasRenderingContext2D | SVGRenderingContext2D
+  get ctx(): Context2d {
+    return this._ctx as Context2d
+  }
+
+  private readonly _el: HTMLElement
+  get el(): HTMLElement {
+    return this._el
+  }
+
+  readonly pixel_ratio: number = 1
+
+  bbox: BBox = new BBox()
+
+  constructor(readonly backend: OutputBackend, readonly hidpi: boolean) {
+    switch (backend) {
+      case "webgl":
+      case "canvas": {
+        this._el = this._canvas = canvas({style})
+        const ctx = this.canvas.getContext('2d')
+        if (ctx == null)
+          throw new Error("unable to obtain 2D rendering context")
+        this._ctx = ctx
+        if (hidpi) {
+          this.pixel_ratio = devicePixelRatio
+        }
+        break
+      }
+      case "svg": {
+        const ctx = new SVGRenderingContext2D()
+        this._ctx = ctx
+        this._canvas = ctx.get_svg()
+        this._el = div({style}, this._canvas)
+        break
+      }
+    }
+
+    fixup_ctx(this._ctx)
+  }
+
+  resize(width: number, height: number): void {
+    this.bbox = new BBox({left: 0, top: 0, width, height})
+
+    const target = this._ctx instanceof SVGRenderingContext2D ? this._ctx : this.canvas
+    target.width = width*this.pixel_ratio
+    target.height = height*this.pixel_ratio
+  }
+
+  prepare(): void {
+    const {ctx, hidpi, pixel_ratio} = this
+    ctx.save()
+    if (hidpi) {
+      ctx.scale(pixel_ratio, pixel_ratio)
+      ctx.translate(0.5, 0.5)
+    }
+    this.clear()
+  }
+
+  clear(): void {
+    const {x, y, width, height} = this.bbox
+    this.ctx.clearRect(x, y, width, height)
+  }
+
+  finish(): void {
+    this.ctx.restore()
+  }
+
+  to_blob(): Promise<Blob> {
+    const {_canvas} = this
+    if (_canvas instanceof HTMLCanvasElement) {
+      if (_canvas.msToBlob != null) {
+        return Promise.resolve(_canvas.msToBlob())
+      } else {
+        return new Promise((resolve, reject) => {
+          _canvas.toBlob((blob) => blob != null ? resolve(blob) : reject(), "image/png")
+        })
+      }
+    } else {
+      const ctx = this._ctx as SVGRenderingContext2D
+      const svg = ctx.get_serialized_svg(true)
+      const blob = new Blob([svg], {type: "image/svg+xml"})
+      return Promise.resolve(blob)
+    }
+  }
+}

bokehjs/src/lib/core/visuals.ts

@@ -9,7 +9,7 @@ import {LineJoin, LineCap, FontStyle, TextAlign, TextBaseline} from "./enums"
 import {View} from "./view"
 import {Texture} from "models/textures/texture"
 import {SVGRenderingContext2D} from "core/util/svg"
-import {CanvasLayer} from "models/canvas/canvas"
+import {CanvasLayer} from "core/util/canvas"
 
 const {hasOwnProperty} = Object.prototype
 

bokehjs/src/lib/models/canvas/canvas.ts

@@ -2,13 +2,12 @@ import {HasProps} from "core/has_props"
 import {DOMView} from "core/dom_view"
 import {logger} from "core/logging"
 import * as p from "core/properties"
-import {div, canvas, append} from "core/dom"
+import {div, append} from "core/dom"
 import {OutputBackend} from "core/enums"
 import {extend} from "core/util/object"
 import {UIEventBus} from "core/ui_events"
 import {BBox} from "core/util/bbox"
-import {Context2d, fixup_ctx} from "core/util/canvas"
-import {SVGRenderingContext2D} from "core/util/svg"
+import {Context2d, CanvasLayer} from "core/util/canvas"
 import {PlotView} from "../plots/plot"
 
 export type FrameBox = [number, number, number, number]
@@ -53,98 +52,6 @@ const style = {
   height: "100%",
 }
 
-export class CanvasLayer {
-  private readonly _canvas: HTMLCanvasElement | SVGSVGElement
-  get canvas(): HTMLCanvasElement {
-    return this._canvas as HTMLCanvasElement
-  }
-
-  private readonly _ctx: CanvasRenderingContext2D | SVGRenderingContext2D
-  get ctx(): Context2d {
-    return this._ctx as Context2d
-  }
-
-  private readonly _el: HTMLElement
-  get el(): HTMLElement {
-    return this._el
-  }
-
-  readonly pixel_ratio: number = 1
-
-  bbox: BBox = new BBox()
-
-  constructor(readonly backend: OutputBackend, readonly hidpi: boolean) {
-    switch (backend) {
-      case "webgl":
-      case "canvas": {
-        this._el = this._canvas = canvas({style})
-        const ctx = this.canvas.getContext('2d')
-        if (ctx == null)
-          throw new Error("unable to obtain 2D rendering context")
-        this._ctx = ctx
-        if (hidpi) {
-          this.pixel_ratio = devicePixelRatio
-        }
-        break
-      }
-      case "svg": {
-        const ctx = new SVGRenderingContext2D()
-        this._ctx = ctx
-        this._canvas = ctx.get_svg()
-        this._el = div({style}, this._canvas)
-        break
-      }
-    }
-
-    fixup_ctx(this._ctx)
-  }
-
-  resize(width: number, height: number): void {
-    this.bbox = new BBox({left: 0, top: 0, width, height})
-
-    const target = this._ctx instanceof SVGRenderingContext2D ? this._ctx : this.canvas
-    target.width = width*this.pixel_ratio
-    target.height = height*this.pixel_ratio
-  }
-
-  prepare(): void {
-    const {ctx, hidpi, pixel_ratio} = this
-    ctx.save()
-    if (hidpi) {
-      ctx.scale(pixel_ratio, pixel_ratio)
-      ctx.translate(0.5, 0.5)
-    }
-    this.clear()
-  }
-
-  clear(): void {
-    const {x, y, width, height} = this.bbox
-    this.ctx.clearRect(x, y, width, height)
-  }
-
-  finish(): void {
-    this.ctx.restore()
-  }
-
-  to_blob(): Promise<Blob> {
-    const {_canvas} = this
-    if (_canvas instanceof HTMLCanvasElement) {
-      if (_canvas.msToBlob != null) {
-        return Promise.resolve(_canvas.msToBlob())
-      } else {
-        return new Promise((resolve, reject) => {
-          _canvas.toBlob((blob) => blob != null ? resolve(blob) : reject(), "image/png")
-        })
-      }
-    } else {
-      const ctx = this._ctx as SVGRenderingContext2D
-      const svg = ctx.get_serialized_svg(true)
-      const blob = new Blob([svg], {type: "image/svg+xml"})
-      return Promise.resolve(blob)
-    }
-  }
-}
-
 export class CanvasView extends DOMView {
   model: Canvas
 

bokehjs/src/lib/models/layouts/layout_dom.ts

@@ -10,7 +10,7 @@ import {build_views} from "core/build_views"
 import {DOMView} from "core/dom_view"
 import {SizingPolicy, BoxSizing, Size, Layoutable} from "core/layout"
 import {bk_root} from "styles/root"
-import {CanvasLayer} from "../canvas/canvas"
+import {CanvasLayer} from "core/util/canvas"
 import {SerializableState} from "core/view"
 
 export abstract class LayoutDOMView extends DOMView {

bokehjs/src/lib/models/plots/plot_canvas.ts

@@ -1,5 +1,5 @@
 import {CartesianFrame} from "../canvas/cartesian_frame"
-import {Canvas, CanvasView, FrameBox, CanvasLayer} from "../canvas/canvas"
+import {Canvas, CanvasView, FrameBox} from "../canvas/canvas"
 import {Renderer, RendererView} from "../renderers/renderer"
 import {DataRenderer} from "../renderers/data_renderer"
 import {Tool, ToolView} from "../tools/tool"
@@ -20,7 +20,7 @@ import {SerializableState} from "core/view"
 import {throttle} from "core/util/throttle"
 import {isArray} from "core/util/types"
 import {copy, reversed} from "core/util/array"
-import {Context2d} from "core/util/canvas"
+import {Context2d, CanvasLayer} from "core/util/canvas"
 import {SizingPolicy, Layoutable} from "core/layout"
 import {HStack, VStack} from "core/layout/alignments"
 import {BorderLayout} from "core/layout/border"

bokehjs/src/lib/models/renderers/renderer.ts

@@ -4,9 +4,9 @@ import {RenderLevel} from "core/enums"
 import * as p from "core/properties"
 import {Model} from "../../model"
 import {BBox} from "core/util/bbox"
+import {CanvasLayer} from "core/util/canvas"
 
 import type {Plot, PlotView} from "../plots/plot"
-import type {CanvasLayer} from "../canvas/canvas"
 import {CoordinateTransform} from "../canvas/coordinates"
 
 export abstract class RendererView extends View {

bokehjs/src/lib/models/tools/toolbar_base.ts

@@ -8,6 +8,8 @@ import {EventType} from "core/ui_events"
 import {some, every} from "core/util/array"
 import {values} from "core/util/object"
 import {isString} from "core/util/types"
+import {CanvasLayer} from "core/util/canvas"
+import {BBox} from "core/util/bbox"
 import {Model} from "model"
 import {Tool} from "./tool"
 import {ButtonTool, ButtonToolButtonView} from "./button_tool"
@@ -153,13 +155,22 @@ export class ToolbarBaseView extends DOMView {
     }
   }
 
+  layout = {bbox: new BBox()}
+
   update_layout(): void {}
 
   update_position(): void {}
 
   after_layout(): void {
     this._has_finished = true
   }
+
+  export(type: "png" | "svg", hidpi: boolean = true): CanvasLayer {
+    const output_backend = type == "png" ? "canvas" : "svg"
+    const canvas = new CanvasLayer(output_backend, hidpi)
+    canvas.resize(0, 0)
+    return canvas
+  }
 }
 
 export type GesturesMap = {

bokehjs/test/integration/layouts.ts

@@ -1,3 +1,4 @@
+import {expect} from "../unit/assertions"
 import {display, fig, row, column, grid} from "./_util"
 
 import {Spacer, Tabs, Panel} from "@bokehjs/models/layouts"
@@ -332,18 +333,22 @@ describe("ToolbarBox", () => {
   }
 
   it("should allow placement above a figure", async () => {
-    await display(tb_above(), [300, 300])
+    const {view} = await display(tb_above(), [300, 300])
+    expect(() => view.export("svg")).to.not.throw()
   })
 
   it("should allow placement below a figure", async () => {
-    await display(tb_below(), [300, 300])
+    const {view} = await display(tb_below(), [300, 300])
+    expect(() => view.export("svg")).to.not.throw()
   })
 
   it("should allow placement left of a figure", async () => {
-    await display(tb_left(),  [300, 300])
+    const {view} = await display(tb_left(),  [300, 300])
+    expect(() => view.export("svg")).to.not.throw()
   })
 
   it("should allow placement right of a figure", async () => {
-    await display(tb_right(), [300, 300])
+    const {view} = await display(tb_right(), [300, 300])
+    expect(() => view.export("svg")).to.not.throw()
   })
 })

bokehjs/test/unit/assertions.ts

@@ -188,20 +188,22 @@ function NotThrows(fn: () => unknown) {
     try {
       fn()
     } catch (error: unknown) {
-      if (!(error instanceof Error)) {
+      if (error_type == null && pattern == null) {
         throw new ExpectationError(`expected ${to_string(fn)} to not throw, got ${to_string(error)}`)
-      }
-
-      if (error_type != null && error instanceof error_type) {
-        throw new ExpectationError(`expected ${to_string(fn)} to not throw an exception of type ${error_type}, got ${to_string(error)}`)
-      }
+      } else {
+        if (error_type != null && error instanceof error_type) {
+          throw new ExpectationError(`expected ${to_string(fn)} to not throw an exception of type ${error_type}, got ${to_string(error)}`)
+        }
 
-      if (pattern instanceof RegExp) {
-        if (error.message.match(pattern))
-          throw new ExpectationError(`expected ${to_string(fn)} to not throw an exception matching ${to_string(pattern)}, got ${to_string(error)}`)
-      } else if (isString(pattern)) {
-        if (error.message.includes(pattern))
-          throw new ExpectationError(`expected ${to_string(fn)} to not throw an exception including ${to_string(pattern)}, got ${to_string(error)}`)
+        if (pattern != null && error instanceof Error) {
+          if (pattern instanceof RegExp) {
+            if (error.message.match(pattern))
+              throw new ExpectationError(`expected ${to_string(fn)} to not throw an exception matching ${to_string(pattern)}, got ${to_string(error)}`)
+          } else if (isString(pattern)) {
+            if (error.message.includes(pattern))
+              throw new ExpectationError(`expected ${to_string(fn)} to not throw an exception including ${to_string(pattern)}, got ${to_string(error)}`)
+          }
+        }
       }
     }
   }