From ec96a7c11f352d7facd247d27f2056bc663a23ed Mon Sep 17 00:00:00 2001 From: Kirk Swenson Date: Wed, 9 Sep 2026 22:49:02 -0700 Subject: [PATCH 1/3] CODAP-1509: add the symmetric extent of a shape The smallest box centered on the point that contains the drawn shape, which is not the box that hugs the ink: a triangle sits on its center of area, so its outline reaches further above the point than below. A renderer that positions a shape by the middle of a box needs this rather than the drawn extent. Drawing into a texture and anchoring it at its center is exactly that, which is what the PIXI renderer does. Co-Authored-By: Claude Opus 5 --- .../renderer/point-shapes.test.ts | 44 ++++++++++++++++++- .../data-display/renderer/point-shapes.ts | 27 ++++++++++-- 2 files changed, 67 insertions(+), 4 deletions(-) diff --git a/v3/src/components/data-display/renderer/point-shapes.test.ts b/v3/src/components/data-display/renderer/point-shapes.test.ts index 6e9228d045..cf1b9af5f7 100644 --- a/v3/src/components/data-display/renderer/point-shapes.test.ts +++ b/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 diff --git a/v3/src/components/data-display/renderer/point-shapes.ts b/v3/src/components/data-display/renderer/point-shapes.ts index b02c459c39..c047c5ffa2 100644 --- a/v3/src/components/data-display/renderer/point-shapes.ts +++ b/v3/src/components/data-display/renderer/point-shapes.ts @@ -187,12 +187,33 @@ export function pointShapeExtent(shape: PointShape, r: number): IShapeExtent { return kShapeDefs[shape].extent(r) } +/* + * The smallest box centered on the point that contains the drawn shape, which for a triangle or a + * star is larger than the box that hugs the ink. + * + * What a renderer needs when it positions a shape by the middle of a box -- drawing into a texture + * and anchoring it at 0.5, 0.5 does exactly that -- since the drawn box is not centered on the + * point it belongs to. + */ +export function pointShapeSymmetricExtent(shape: PointShape, r: number): IShapeExtent { + const geometry = pointShapeGeometry(shape, r) + if (geometry.kind === "circle") return { w: 2 * geometry.radius, h: 2 * geometry.radius } + + let maxAbsX = 0 + let maxAbsY = 0 + geometry.points.forEach(({ x, y }) => { + maxAbsX = Math.max(maxAbsX, Math.abs(x)) + maxAbsY = Math.max(maxAbsY, Math.abs(y)) + }) + return { w: 2 * maxAbsX, h: 2 * maxAbsY } +} + /* * The distance from the center to the furthest vertex, which is what hit testing needs. * - * Measured from the vertices rather than from the extent: the extent is the size of the drawn box, - * and a triangle's box is not centered on the point, so half its larger side stops short of the ink - * -- the apex sits at 2h/3 while half the width is s/2. A hit area sized that way misses the apex. + * Measured from the vertices rather than from the extent: a triangle's box is not centered on the + * point, so half its larger side stops short of the ink -- the apex sits at 2h/3 while half the + * width is s/2. A hit area sized that way misses the apex. */ export function pointShapeBoundingRadius(shape: PointShape, r: number): number { const geometry = pointShapeGeometry(shape, r) From ecc6cb167dcee1712665368d91bf9bf245606057 Mon Sep 17 00:00:00 2001 From: Kirk Swenson Date: Wed, 9 Sep 2026 22:49:03 -0700 Subject: [PATCH 2/3] CODAP-1509: draw and hit test point shapes in the PIXI renderer Textures are built from the shared geometry rather than an arc, so the WebGL renderer draws what the canvas renderer draws. The texture cache needed no change: its key is the whole style, which now carries the shape, so two shapes can never collide on one texture. A sprite's texture is drawn around its anchor, which for points is the middle of the texture. Left to size itself a texture takes the bounds of the ink, and for a triangle that box is not centered on the point -- the ink sits above the center of area, so the triangle would be drawn low by about a third of its radius, reinstating in WebGL the bias the canvas renderer was fixed for. The texture is given an explicit frame centered on the point instead. Circles keep the path they have always had, including sizing their own texture. Sprites get a hit area testing the drawn shape. PIXI hands it the pointer in the sprite's own coordinates, where the origin is the point's position, so it can defer to the same containment the canvas renderer uses and the two agree on what counts as a hit. Note this also tightens picking for circles, which until now were hit tested against the rectangle of their texture -- a click well outside a circular point could select it. Bars keep the rectangular test, which is what a bar is. The hit area holds the shape and radius rather than looking them up, since it runs per point per pointer event. It is kept in step with the texture at all three places a sprite's texture is assigned: creation, a style change, and the end of a points/bars transition. Co-Authored-By: Claude Opus 5 --- .../renderer/pixi-point-renderer.test.ts | 116 +++++++++++++++++- .../renderer/pixi-point-renderer.ts | 85 +++++++++++-- 2 files changed, 190 insertions(+), 11 deletions(-) diff --git a/v3/src/components/data-display/renderer/pixi-point-renderer.test.ts b/v3/src/components/data-display/renderer/pixi-point-renderer.test.ts index 798401d171..2576dc4dfa 100644 --- a/v3/src/components/data-display/renderer/pixi-point-renderer.test.ts +++ b/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()) diff --git a/v3/src/components/data-display/renderer/pixi-point-renderer.ts b/v3/src/components/data-display/renderer/pixi-point-renderer.ts index b9688689c6..b1ab434256 100644 --- a/v3/src/components/data-display/renderer/pixi-point-renderer.ts +++ b/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,35 @@ import { RendererCapability } from "./point-renderer-types" +/* + * The region of the shape's own coordinates that becomes the texture, centered on the point so the + * sprite's anchor lands on it. Padded by the stroke, which straddles the outline and would + * otherwise be clipped 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 +481,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 +532,7 @@ export class PixiPointRenderer extends PointRendererBase { if (sprite.texture !== texture) { sprite.texture = texture } + this.syncHitArea(sprite, newStyle) this.doStartRendering() } @@ -739,7 +771,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 +784,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,10 +853,10 @@ 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) @@ -813,12 +864,27 @@ export class PixiPointRenderer extends PointRendererBase { 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) + /* + * A texture is drawn into the sprite around its anchor, which for points is the middle of the + * texture, so the texture has to be centered on the point's own origin. Left to size itself the + * texture takes the bounds of the ink, and a triangle's ink sits above its center of area -- + * centering that box would draw the triangle low by about a third of its radius. Circles keep + * the self-sizing path they have always used. + */ + 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 From e8f1e9073670924b8155799749167d2602f7d032 Mon Sep 17 00:00:00 2001 From: Kirk Swenson Date: Thu, 10 Sep 2026 08:45:30 -0700 Subject: [PATCH 3/3] CODAP-1509: say the centering reason once Why a texture has to be centered on the point rather than on the ink was explained at the helper that computes the frame and again where it is called. It lives at the helper, which exists for no other reason; the call site says only that circles keep sizing their own. The symmetric extent restated why a triangle's box is off center, which the triangle's own geometry already explains. It now says what it is and who needs it. Co-Authored-By: Claude Opus 5 --- .../renderer/pixi-point-renderer.ts | 20 +++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/v3/src/components/data-display/renderer/pixi-point-renderer.ts b/v3/src/components/data-display/renderer/pixi-point-renderer.ts index b1ab434256..edffd5bc11 100644 --- a/v3/src/components/data-display/renderer/pixi-point-renderer.ts +++ b/v3/src/components/data-display/renderer/pixi-point-renderer.ts @@ -24,9 +24,14 @@ import { } from "./point-renderer-types" /* - * The region of the shape's own coordinates that becomes the texture, centered on the point so the - * sprite's anchor lands on it. Padded by the stroke, which straddles the outline and would - * otherwise be clipped at the widest vertices. + * 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) @@ -876,13 +881,8 @@ export class PixiPointRenderer extends PointRendererBase { .fill(fill) .stroke({ color: stroke, width: strokeWidth, alpha: strokeOpacity ?? 0.4 }) - /* - * A texture is drawn into the sprite around its anchor, which for points is the middle of the - * texture, so the texture has to be centered on the point's own origin. Left to size itself the - * texture takes the bounds of the ink, and a triangle's ink sits above its center of area -- - * centering that box would draw the triangle low by about a third of its radius. Circles keep - * the self-sizing path they have always used. - */ + // 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) }