CODAP-1509: draw and hit test point shapes in the PIXI renderer

v3/src/components/data-display/renderer/pixi-point-renderer.test.ts

@@ -18,6 +18,7 @@ jest.mock("pixi.js", () => {
     destroy() { this.children = [] }
   }
   class MockSprite {
+    hitArea: any = null
     anchor = { x: 0, y: 0, copyFrom(p: any) { this.x = p.x; this.y = p.y } }
     position = { x: 0, y: 0, set(x: number, y: number) { this.x = x; this.y = y } }
     scale = { x: 1, y: 1, set(x: number, y: number) { this.x = x; this.y = y } }
@@ -39,8 +40,11 @@ jest.mock("pixi.js", () => {
   }
   class MockGraphics {
     boundsArea: any = null
-    rect() { return this }
-    circle() { return this }
+    // records what was traced, so a test can tell a polygon from an arc
+    traced: Array<{ op: string, args: any[] }> = []
+    rect(...args: any[]) { this.traced.push({ op: "rect", args }); return this }
+    circle(...args: any[]) { this.traced.push({ op: "circle", args }); return this }
+    poly(...args: any[]) { this.traced.push({ op: "poly", args }); return this }
     fill() { return this }
     stroke() { return this }
     destroy() {}
@@ -68,7 +72,12 @@ jest.mock("pixi.js", () => {
     }
     resize() {}
     render() {}
-    generateTexture() { return new MockTexture() }
+    // keeps every call, so a test can inspect the graphics traced and the frame requested
+    generateTextureCalls: any[] = []
+    generateTexture(options: any) {
+      this.generateTextureCalls.push(options)
+      return new MockTexture()
+    }
     destroy() {}
   }
   return {
@@ -96,6 +105,107 @@ describe("PixiPointRenderer", () => {
     subPlotNum
   })
 
+  describe("point shapes", () => {
+    const setUp = async (style: IPointStyle) => {
+      const pixiRenderer = new PixiPointRenderer(new PointsState())
+      await pixiRenderer.init()
+      pixiRenderer.matchPointsToData("dataset1", [createCaseData(0, "case1")], "points", style)
+      const renderer = (pixiRenderer as any).renderer
+      const sprite = (pixiRenderer as any).sprites.get(
+        (pixiRenderer as any).state.getPointIdForCaseData(createCaseData(0, "case1"))
+      )
+      return { pixiRenderer, renderer, sprite }
+    }
+
+    const lastTexture = (renderer: any) =>
+      renderer.generateTextureCalls[renderer.generateTextureCalls.length - 1]
+
+    it("traces the shape's polygon rather than an arc", async () => {
+      const { renderer } = await setUp({ ...defaultStyle, shape: "square" })
+      const traced = lastTexture(renderer).target.traced.map((t: any) => t.op)
+
+      expect(traced).toContain("poly")
+      expect(traced).not.toContain("circle")
+    })
+
+    it("still draws a circle as an arc, as it always has", async () => {
+      // the shape CODAP has always drawn keeps its own path, including sizing its own texture
+      const { renderer } = await setUp(defaultStyle)
+      const call = lastTexture(renderer)
+
+      expect(call.target.traced.map((t: any) => t.op)).toContain("circle")
+      expect(call.frame).toBeUndefined()
+    })
+
+    it("centers a triangle's texture on the point rather than on its ink", async () => {
+      /*
+       * The sprite's anchor sits at the middle of its texture, so a texture sized to the ink would
+       * put the middle of a triangle's ink on the point -- and a triangle is centered on its center
+       * of area, which is not the middle of its outline. It would be drawn low.
+       */
+      const { renderer } = await setUp({ ...defaultStyle, shape: "triangle" })
+      const { frame } = lastTexture(renderer)
+
+      expect(frame).toBeDefined()
+      expect(frame.x + frame.width / 2).toBeCloseTo(0, 6)
+      expect(frame.y + frame.height / 2).toBeCloseTo(0, 6)
+    })
+
+    it("gives two shapes two textures rather than sharing one", async () => {
+      // the texture cache keys on the whole style, so a shape cannot collide with another
+      const { pixiRenderer, renderer } = await setUp({ ...defaultStyle, shape: "square" })
+      const before = renderer.generateTextureCalls.length
+      const pointId = (pixiRenderer as any).sprites.keys().next().value
+      ;(pixiRenderer as any).doSetPointStyle(pointId, { shape: "star" })
+
+      expect(renderer.generateTextureCalls.length).toBe(before + 1)
+      expect(lastTexture(renderer).target.traced.map((t: any) => t.op)).toContain("poly")
+    })
+
+    describe("hit area", () => {
+      it("tests the drawn shape rather than the sprite's rectangle", async () => {
+        const { sprite } = await setUp({ ...defaultStyle, shape: "star", radius: 8 })
+
+        // straight up along a tip, past the radius but on the ink
+        expect(sprite.hitArea.contains(0, -10)).toBe(true)
+        // the same distance out between two arms, where the star is not drawn
+        const rad = -54 * Math.PI / 180
+        expect(sprite.hitArea.contains(Math.cos(rad) * 10, Math.sin(rad) * 10)).toBe(false)
+      })
+
+      it("keeps every shape at least as easy to hit as a circle", async () => {
+        const { sprite } = await setUp({ ...defaultStyle, shape: "plus", radius: 8 })
+
+        for (let deg = 0; deg < 360; deg += 30) {
+          const rad = deg * Math.PI / 180
+          expect(sprite.hitArea.contains(Math.cos(rad) * 7.9, Math.sin(rad) * 7.9)).toBe(true)
+        }
+      })
+
+      it("follows the shape when the style changes", async () => {
+        const { pixiRenderer, sprite } = await setUp({ ...defaultStyle, shape: "circle", radius: 8 })
+        expect(sprite.hitArea.contains(0, -10)).toBe(false)
+
+        const pointId = (pixiRenderer as any).sprites.keys().next().value
+        ;(pixiRenderer as any).doSetPointStyle(pointId, { shape: "star" })
+
+        // the tip is on the ink now, so the same click that missed the circle hits the star
+        expect(sprite.hitArea.contains(0, -10)).toBe(true)
+      })
+
+      it("leaves bars to the sprite's own rectangular test", async () => {
+        // a bar is a rectangle, which is exactly what a sprite hit tests against by default
+        const pixiRenderer = new PixiPointRenderer(new PointsState())
+        await pixiRenderer.init()
+        pixiRenderer.matchPointsToData("dataset1", [createCaseData(0, "case1")], "bars",
+          { ...defaultStyle, width: 20, height: 40 })
+        const sprite = (pixiRenderer as any).sprites.values().next().value
+
+        expect(sprite.hitArea).toBeNull()
+      })
+    })
+  })
+
   describe("setPointsInteractive", () => {
     it("toggles hit-testing of the points container", async () => {
       const pixiRenderer = new PixiPointRenderer(new PointsState())

v3/src/components/data-display/renderer/pixi-point-renderer.ts

@@ -12,6 +12,8 @@ import {
 } from "./point-renderer-base"
 import { PointsState } from "./points-state"
 import { coalesceBars, IBarPiece, pointStateToBarPiece } from "./bar-coalescing"
+import { kDefaultPointShape, PointShape } from "../../../utilities/point-shape-utils"
+import { isPointInShape, pointShapeGeometry, pointShapeSymmetricExtent } from "./point-shapes"
 import {
   IBackgroundEventDistributionOptions,
   IPoint,
@@ -21,6 +23,40 @@ import {
   RendererCapability
 } from "./point-renderer-types"
 
+/*
+ * The region of the shape's own coordinates that becomes the texture.
+ *
+ * A sprite draws its texture around its anchor, which for points is the middle of the texture, so
+ * the texture has to be centered on the point rather than on the ink. Left to size itself it takes
+ * the bounds of the ink, and a triangle's ink sits high -- centering that would draw the triangle
+ * low by about a third of its radius.
+ *
+ * Padded by the stroke, which straddles the outline and would otherwise clip at the widest vertices.
+ */
+function symmetricFrame(shape: PointShape, radius: number, strokeWidth: number): PIXI.Rectangle {
+  const { w, h } = pointShapeSymmetricExtent(shape, radius)
+  const paddedW = w + 2 * strokeWidth
+  const paddedH = h + 2 * strokeWidth
+  return new PIXI.Rectangle(-paddedW / 2, -paddedH / 2, paddedW, paddedH)
+}
+
+/*
+ * Hit tests a sprite against the shape drawn on it rather than the rectangle of its texture, which
+ * is what a sprite falls back to and is looser than even the circle CODAP has always drawn.
+ *
+ * PIXI hands `contains` the pointer in the sprite's own coordinates, where the origin is the point's
+ * position, so these are the same offsets the canvas hit tester works in and both renderers agree
+ * on what counts as a hit. The shape and radius are held rather than looked up: this runs per point
+ * per pointer event, and a hover over a dense plot cannot afford a map lookup for each one.
+ */
+class PointShapeHitArea {
+  constructor(public shape: PointShape, public radius: number) {}
+
+  contains(x: number, y: number): boolean {
+    return isPointInShape(this.shape, this.radius, x, y)
+  }
+}
+
 const DEFAULT_Z_INDEX = 0
 const RAISED_Z_INDEX = 100
 const MAX_SPRITE_SCALE = 2
@@ -450,7 +486,7 @@ export class PixiPointRenderer extends PointRendererBase {
     // Create sprites for added points (skip any already created by syncFromState above)
     added.forEach(pointId => {
       if (!this.sprites.has(pointId)) {
-        const sprite = this.getNewSprite(pointId, texture)
+        const sprite = this.getNewSprite(pointId, texture, style)
         this.pointsContainer.addChild(sprite)
         this.sprites.set(pointId, sprite)
       }
@@ -501,6 +537,7 @@ export class PixiPointRenderer extends PointRendererBase {
     if (sprite.texture !== texture) {
       sprite.texture = texture
     }
+    this.syncHitArea(sprite, newStyle)
 
     this.doStartRendering()
   }
@@ -739,7 +776,7 @@ export class PixiPointRenderer extends PointRendererBase {
       if (!this.sprites.has(pointState.id)) {
         try {
           const texture = this.getPointTexture(pointState.style)
-          const sprite = this.getNewSprite(pointState.id, texture)
+          const sprite = this.getNewSprite(pointState.id, texture, pointState.style)
           sprite.position.set(pointState.x, pointState.y)
           sprite.scale.set(pointState.scale)
           sprite.zIndex = pointState.isRaised ? RAISED_Z_INDEX : DEFAULT_Z_INDEX
@@ -752,14 +789,33 @@ export class PixiPointRenderer extends PointRendererBase {
     })
   }
 
-  private getNewSprite(pointId: string, texture: PIXI.Texture): PIXI.Sprite {
+  private getNewSprite(pointId: string, texture: PIXI.Texture, style: IPointStyle): PIXI.Sprite {
     const sprite = new PIXI.Sprite(texture)
     sprite.anchor.copyFrom(this._anchor)
     sprite.zIndex = DEFAULT_Z_INDEX
+    this.syncHitArea(sprite, style)
     this.setupSpriteInteractivity(pointId, sprite)
     return sprite
   }
 
+  /*
+   * Bars keep the sprite's own rectangular test, which is what a bar is. Only a point carries a
+   * shape to test against.
+   */
+  private syncHitArea(sprite: PIXI.Sprite, style: IPointStyle): void {
+    if (this._displayType !== "points") {
+      sprite.hitArea = null
+      return
+    }
+    const shape = style.shape ?? kDefaultPointShape
+    if (sprite.hitArea instanceof PointShapeHitArea) {
+      sprite.hitArea.shape = shape
+      sprite.hitArea.radius = style.radius
+    } else {
+      sprite.hitArea = new PointShapeHitArea(shape, style.radius)
+    }
+  }
+
   private setPointXyProperty(prop: TransitionProp, sprite: PIXI.Sprite, x: number, y: number): void {
     if (this.currentTransition) {
       this.setTargetXyProp(prop, sprite, x, y)
@@ -802,23 +858,33 @@ export class PixiPointRenderer extends PointRendererBase {
   private getPointTexture(style: IPointStyle, includeDimensions = false): PIXI.Texture {
     return this._displayType === "bars"
       ? this.getRectTexture(style, includeDimensions)
-      : this.getCircleTexture(style)
+      : this.getShapeTexture(style)
   }
 
-  private getCircleTexture(style: IPointStyle): PIXI.Texture {
+  private getShapeTexture(style: IPointStyle): PIXI.Texture {
     const { radius, fill, stroke, strokeWidth, strokeOpacity } = style
     const key = this.textureKey(style)
 
     if (this.textures.has(key)) {
       return this.textures.get(key) as PIXI.Texture
     }
 
+    const shape = style.shape ?? kDefaultPointShape
+    const geometry = pointShapeGeometry(shape, radius)
     const graphics = new PIXI.Graphics()
-      .circle(0, 0, radius)
+    if (geometry.kind === "circle") {
+      graphics.circle(0, 0, geometry.radius)
+    } else {
+      graphics.poly(geometry.points.flatMap(({ x, y }) => [x, y]))
+    }
+    graphics
       .fill(fill)
       .stroke({ color: stroke, width: strokeWidth, alpha: strokeOpacity ?? 0.4 })
 
-    return this.generateTexture(graphics, key)
+    // Circles keep the self-sizing path they have always used; everything else needs an explicit
+    // frame, for the reason given on symmetricFrame.
+    const frame = geometry.kind === "circle" ? undefined : symmetricFrame(shape, radius, strokeWidth)
+    return this.generateTexture(graphics, key, frame)
   }
 
   private getRectTexture(style: IPointStyle, includeDimensions = false): PIXI.Texture {
@@ -849,13 +915,14 @@ export class PixiPointRenderer extends PointRendererBase {
     return this.generateTexture(graphics, key)
   }
 
-  private generateTexture(graphics: PIXI.Graphics, key: string): PIXI.Texture {
+  private generateTexture(graphics: PIXI.Graphics, key: string, frame?: PIXI.Rectangle): PIXI.Texture {
     if (!this.renderer) {
       throw new Error("PixiPointRenderer renderer not initialized")
     }
     const texture = this.renderer.generateTexture({
       target: graphics,
       resolution: devicePixelRatio * MAX_SPRITE_SCALE,
+      ...(frame ? { frame } : {})
     })
 
     this.textures.set(key, texture)
@@ -927,6 +994,8 @@ export class PixiPointRenderer extends PointRendererBase {
     if (pointState) {
       const newStyle = { ...pointState.style, ...style }
       const texture = this.getPointTexture(newStyle, true)
+      // the display type has changed by now, so what the sprite should be hit tested against has too
+      this.syncHitArea(sprite, newStyle)
 
       if (sprite.texture !== texture) {
         sprite.texture = texture

v3/src/components/data-display/renderer/point-shapes.test.ts

@@ -1,7 +1,7 @@
 import { PointShapes } from "../../../utilities/point-shape-utils"
 import {
   IShapePoint, isPointInShape, pointShapeArea, pointShapeBoundingRadius, pointShapeExtent,
-  pointShapeGeometry
+  pointShapeGeometry, pointShapeSymmetricExtent
 } from "./point-shapes"
 
 /*
@@ -205,6 +205,48 @@ describe("point shape geometry", () => {
     })
   })
 
+  describe("symmetric extent", () => {
+    it("contains the whole outline, centered on the point", () => {
+      PointShapes.filter(s => s !== "circle").forEach(shape => {
+        const geometry = pointShapeGeometry(shape, 8)
+        if (geometry.kind !== "polygon") throw new Error(`${shape} should be a polygon`)
+        const { w, h } = pointShapeSymmetricExtent(shape, 8)
+        geometry.points.forEach(({ x, y }) => {
+          expect(Math.abs(x)).toBeLessThanOrEqual(w / 2 + 1e-9)
+          expect(Math.abs(y)).toBeLessThanOrEqual(h / 2 + 1e-9)
+        })
+      })
+    })
+
+    it("is larger than the drawn box exactly where the box is off center", () => {
+      /*
+       * The triangle and the star hang off center because they are centered on their ink. A
+       * renderer positioning them by the middle of a box has to use this larger box, or it puts
+       * the middle of the drawn ink somewhere other than the point.
+       */
+      const offCenter = ["triangle", "star"] as const
+      offCenter.forEach(shape => {
+        expect(pointShapeSymmetricExtent(shape, 8).h).toBeGreaterThan(pointShapeExtent(shape, 8).h)
+      })
+
+      PointShapes.filter(s => !offCenter.includes(s as any)).forEach(shape => {
+        const symmetric = pointShapeSymmetricExtent(shape, 8)
+        const drawn = pointShapeExtent(shape, 8)
+        expect(symmetric.w).toBeCloseTo(drawn.w, 6)
+        expect(symmetric.h).toBeCloseTo(drawn.h, 6)
+      })
+    })
+
+    it("scales linearly with the radius", () => {
+      PointShapes.forEach(shape => {
+        const small = pointShapeSymmetricExtent(shape, 3)
+        const large = pointShapeSymmetricExtent(shape, 12)
+        expect(large.w / small.w).toBeCloseTo(4, 6)
+        expect(large.h / small.h).toBeCloseTo(4, 6)
+      })
+    })
+  })
+
   describe("containment", () => {
     const r = 8