refactor: point rendering system with WebGL context pooling#2297
refactor: point rendering system with WebGL context pooling#2297
Conversation
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #2297 +/- ##
==========================================
+ Coverage 85.18% 85.61% +0.42%
==========================================
Files 733 742 +9
Lines 39527 40086 +559
Branches 9380 9791 +411
==========================================
+ Hits 33673 34319 +646
+ Misses 5845 5755 -90
- Partials 9 12 +3
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
This PR refactors CODAP v3's point rendering system to address WebGL context limitations through a pooling architecture. The main goal is enabling documents with many graphs to function properly by implementing priority-based context allocation, renderer abstraction for graceful degradation, and state preservation across renderer switches.
Changes:
- Implemented WebGL context pooling with
WebGLContextManagersingleton (max 14 contexts) - Introduced
PointRendererBaseabstraction withPixiPointRenderer(WebGL) andNullPointRenderer(placeholder) implementations - Added
PointsStateclass to maintain canonical point data that survives renderer switches - Renamed event handlers from
onPoint*toonPointer*and removed PIXI-specific naming from public API - Integrated maps into WebGL context management via new hooks
Reviewed changes
Copilot reviewed 53 out of 53 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
en-US.json5 |
Added translation keys for WebGL context placeholder messages |
map-polygon-layer.tsx |
Renamed PixiBackgroundPassThroughEvent to BackgroundPassThroughEvent |
map-point-layer.tsx |
Refactored to use context-managed renderer via useLayerRenderer hook |
map-interior.tsx |
Removed setPixiPointsLayer prop passing |
map-component.tsx |
Integrated usePointRendererArray and PointRendererArrayContext |
map-background.tsx |
Renamed pixiPointsArray to rendererArray |
codap-map.tsx |
Updated to use rendererArray and renamed hooks |
image-utils.ts |
Changed pixiPoints to renderer parameter |
image-utils.test.ts |
Updated mock to use PointRendererBase |
graph-utils.ts |
Replaced PixiPoints with PointRendererBase throughout |
scatter-plot.tsx |
Updated to use renderer prop and renamed event handlers |
histogram.tsx |
Replaced pixiPoints with renderer |
dot-line-plot.tsx |
Updated to use renderer and renamed hooks |
dot-chart.tsx |
Renamed event handler to onPointerClick |
case-plot.tsx |
Updated drag handlers to use renderer methods |
binned-dot-plot.tsx |
Replaced pixiPoints with renderer |
bar-chart.tsx |
Updated to use renderer prop |
graph-controller.ts |
Changed pixiPoints to renderer property |
use-plot.ts |
Renamed hooks and improved renderer switch handling |
use-graph-controller.ts |
Updated to use rendererArray |
use-dot-plot.ts |
Replaced pixiPoints with renderer parameter |
use-dot-plot-drag-drop.ts |
Updated event handler signatures |
use-chart-dots.ts |
Replaced pixiPoints with renderer |
graphing-types.ts |
Changed IPlotProps to use renderer |
graph.tsx |
Added WebGL context placeholder support |
graph-component.tsx |
Integrated usePointRendererArray with context management |
webgl-context-manager.ts |
New singleton for WebGL context pooling |
webgl-context-manager.test.ts |
Comprehensive test suite for context manager |
use-point-renderer.ts |
New hook for renderer lifecycle management |
use-point-renderer-array.ts |
New hooks for multi-layer renderer support |
points-state.ts |
New class for canonical point data storage |
points-state.test.ts |
Test suite for PointsState |
point-renderer-types.ts |
Core type definitions for renderer system |
point-renderer-context.tsx |
React context for renderer access |
point-renderer-base.ts |
Abstract base class with Template Method pattern |
pixi-point-renderer.ts |
WebGL renderer implementation using PIXI.js |
null-point-renderer.ts |
No-op renderer for context-starved graphs |
null-point-renderer.test.ts |
Test suite for NullPointRenderer |
index.ts |
Public API exports for renderer module |
pixi-points.ts |
Deleted legacy PixiPoints class (892 lines removed) |
data-display-render-state.ts |
Updated to use rendererArray |
data-display-content-model.ts |
Changed hideDataTip to use PointerEvent |
use-renderer-pointer-down.ts |
Renamed from use-pixi-pointer-down.ts |
use-renderer-pointer-down-deselect.ts |
New file replacing pixi-specific version |
use-pixi-points-array.ts |
Deleted (39 lines removed) |
use-pixi-pointer-down-deselect.ts |
Deleted (15 lines removed) |
use-connecting-lines.ts |
Updated to use renderer and renamed event enum |
data-display-utils.ts |
Replaced PixiPoints with PointRendererBase |
no-webgl-context-placeholder.tsx |
New component for context unavailable message |
no-webgl-context-placeholder.scss |
Styles for placeholder component |
data-tip.tsx |
Updated to use renderer and renamed event handlers |
background.tsx |
Updated to use rendererArray and access metadata for coordinates |
point-rendering.md |
New comprehensive documentation (347 lines) |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
codap-v3
|
||||||||||||||||||||||||||||
| Project |
codap-v3
|
| Branch Review |
main
|
| Run status |
|
| Run duration | 02m 11s |
| Commit |
|
| Committer | eireland |
| View all properties for this run ↗︎ | |
| Test results | |
|---|---|
|
|
0
|
|
|
0
|
|
|
0
|
|
|
0
|
|
|
4
|
| View all changes introduced in this branch ↗︎ | |
Refactor point rendering to support WebGL context pooling, addressing the browser's ~16 context limit when documents have many graphs. Key changes: - Add PointRendererBase abstraction with PixiPointRenderer and NullPointRenderer implementations - Add WebGLContextManager singleton to manage context pool (max 14) - Add PointsState to preserve point data across renderer switches - Add usePointRenderer/usePointRendererArray hooks for React integration - Implement priority-based context allocation (point count + user clicks) - Show placeholder message when context unavailable; clicking prioritizes - Clean up unused PIXI imports throughout codebase Graphs without WebGL contexts now show a helpful message and can be prioritized by clicking/selecting them. When a graph becomes visible or is selected, it requests a context and may evict a lower-priority graph's context if the pool is full. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Document the WebGL context pooling system and point renderer abstraction, including motivation, architecture overview, key components, and usage examples. This provides context for engineers and LLMs working in this area. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Add comprehensive test coverage for the new point rendering architecture: - PointsState: state management and sync with case data - NullPointRenderer: no-op renderer behavior and base class template methods - WebGLContextManager: context pooling, priority, eviction, and yield/release - point-renderer-compat: type guards and compatibility utilities Also export WebGLContextManager class to enable test isolation. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Update MapPointLayer to use PixiPointRenderer instead of PixiPoints - Update usePixiPointsArray hook to use PointRendererBase types - Update Background, MapBackground, MapInterior to use PointRendererArray - Update Graph and GraphComponent to use PointRendererArray directly - Add x/y coordinates to IPointMetadata for marquee selection support - Update tests for new metadata structure This completes the migration of map components to the new renderer architecture, bringing maps in line with graphs. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Remove point-renderer-compat.ts and its tests - Rename PixiPointsCompatible -> PointRendererBase - Rename PixiPointsCompatibleArray -> PointRendererArray - Rename pixiPoints -> renderer throughout codebase - Rename pixiPointsArray -> rendererArray - Rename hook files: - use-pixi-points-array.ts -> use-renderer-array.ts - use-pixi-pointer-down.ts -> use-renderer-pointer-down.ts - use-pixi-pointer-down-deselect.ts -> use-renderer-pointer-down-deselect.ts - Rename hook functions: - usePixiPointsArray -> useRendererArray - usePixiPointerDown -> useRendererPointerDown - usePixiPointerDownDeselect -> useRendererPointerDownDeselect - Update point-rendering.md documentation Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Move event-to-renderer mapping from PixiPointRenderer to PointRendererBase - Rename getPixiPointRendererDispatcher to getRendererForEvent - Add registerDispatchedEvent() method for subclasses to use - Remove unused hasWebGLContext prop from Graph component - Update point-rendering.md documentation: - Fix usePointRendererArray description (maps, not graphs with multiple Y) - Clarify context recovery vs dynamic context limit in Future Considerations - Remove hasWebGLContext from examples Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Remove `any` types from DataTip, replacing with IPoint/IPointMetadata - Change hideDataTip to use PointerEvent for consistency with showDataTip - Rename event handlers from onPoint* to onPointer* (onPointerOver, onPointerLeave, onPointerClick, onPointerDragStart, onPointerDrag, onPointerDragEnd) since the pointer is moving, not the point - Move PointRendererArray type to point-renderer-base.ts to avoid circular dependency with inline import - Extract usePointRendererArray into its own file - Rename IPixiDragHandlers to IRendererDragHandlers and usePixiDragHandlers to useRendererDragHandlers - Remove CompatiblePointEventHandler type, use PointEventHandler instead - Fix types in plot components (scatter-plot, case-plot, dot-chart, dot-line-plot, binned-dot-plot, map-point-layer) - Fix case-plot drag to use renderer.setPointPosition() instead of direct property mutation - Remove outdated comments about old/new API compatibility - Update point-rendering.md documentation Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
… maps - Delete unused pixi-points.ts (legacy PixiPoints class) - Delete use-renderer-array.ts (replaced by context-based approach) - Remove unused getSpriteForPoint() and migration comments - Rename PixiBackgroundPassThroughEvent to BackgroundPassThroughEvent and move to point-renderer-types.ts for broader applicability - Add WebGL context management for map layers: - Add PointRendererArrayContext for sharing renderer settings - Add useLayerRenderer hook for child layers to get context-managed renderers - Update MapComponent/CodapMap/MapPointLayer to use new context system - Add setupResizeObserver to PointRendererBase for post-init configuration Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Add skipContextRegistration option to usePointRenderer to prevent unused primary renderer in usePointRendererArray from consuming context slots - Add effect in MapPointLayer to refresh points when renderer changes - Fix canvas replacement in MapPointLayer to clear old canvas before appending new one (old canvas was blocking new canvas from being added) - Update point-rendering.md documentation with new hooks and patterns Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
3c0f09c to
08536c5
Compare
08536c5 to
b81c249
Compare
b81c249 to
ae0d036
Compare
ae0d036 to
af359d7
Compare
59038d8 to
2bdab2f
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 64 out of 65 changed files in this pull request and generated 5 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Update Cypress helpers and fix test timing issues discovered during CI testing of the point renderer refactoring. Cypress helper updates: - Update axis-helper, graph-canvas-helper, and map-canvas-helper to use new renderer API (rendererArrayMap, getPointsBounds, etc.) Test stability fixes: - Add waits after drag operations for graph/axis rendering to complete - Re-select tiles after drag operations to ensure inspector panel updates - Add existence checks before clicking inspector panel buttons - Wait for cell values to change after formula rerandomize operations - Split table.spec.ts into table.spec.ts and table-index.spec.ts for better parallelization Code fixes discovered during testing: - Fix D3 transition race condition in connecting lines by interrupting existing transitions before starting new ones - Fix stale closure issue in showConnectingLines by reading from store - Add console.log/warn/error capture in Cypress for debugging CI failures Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
2bdab2f to
5b7e73d
Compare
…issues - Fix bar chart bars disappearing after dragging categories to reorder them by updating subPlotNum values in state before reapplying masks. Added updateSubPlotNums() method to renderer and caseDataWithSubPlot view usage. - Fix point selection issues by adding stopPropagation() to click/pointerdown handlers and canceling pending deselection when clicking on points. - Fix histogram bar positioning by using circleAnchor in setPointCoordinates. - Fix axis drag lag in histograms by only calling matchCirclesToData when the renderer actually changes, not when callbacks are recreated. - Improve PIXI mask lifecycle management: - Clear sprite mask references before destroying masks - Defer mask destruction via requestAnimationFrame - Set explicit boundsArea for masks (PIXI v8 requirement) - Trigger render after mask changes - Add timer guard in startAnimation to prevent stale callbacks from running when a new animation supersedes an old one. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Resolved conflicts: - axis-helper.ts: Combined force:true click with visible submenu filtering - data-tip.tsx: Removed unused React and PIXI imports - graph.tsx: Kept useEffect-wrapped canvas mounting with robust cleanup - use-plot.ts: Kept IRendererDragHandlers + added useAccumulatingDebouncedCallback - case-plot.tsx, dot-chart.tsx: Removed unused React and PIXI imports - scatter-plot.tsx: Combined clean imports + getLegendColor param + legendAttrID dep - map-point-layer.tsx: Kept robust canvas mounting with renderer API - pixi-points.ts: Deleted (replaced by new renderer abstraction) Additional fixes: - pixi-point-renderer.ts: Added failIfMajorPerformanceCaveat for CI environments - data-display-utils.test.ts: Updated to use new renderer API - map-interior.tsx: Added missing index parameter to .map() callback Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Fixes crash when switching to bars display type before binMap is fully populated. The non-bar code path already had this check. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Fix the IPointState interface documentation to match the actual implementation. Added missing fields (caseID, plotNum, datasetID, isVisible) and corrected subPlotIndex to subPlotNum (optional). Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Summary
This PR introduces a comprehensive refactoring of the point rendering system for graphs and maps in CODAP v3. The main goals are:
PointRendererBaseabstract class withPixiPointRenderer(WebGL) andNullPointRenderer(placeholder) implementations, allowing graceful degradation when contexts are unavailable.PointsStateclass maintains canonical point data that survives renderer switches, so graphs restore correctly when contexts become available.anytypes throughout, using properIPointandIPointMetadatatypes.onPoint*toonPointer*(since the pointer moves, not the point) and removes PIXI-specific naming from the public API.Key Changes
New Architecture:
WebGLContextManagersingleton manages context pool (max 14, leaving browser headroom)PointRendererBaseabstract class with Template Method patternPixiPointRendererfor full WebGL renderingNullPointRendererfor context-starved graphs (tracks state, no rendering)PointsStatefor canonical point data that survives renderer switchesReact Integration:
usePointRendererhook handles visibility observation, context management, and renderer lifecycleusePointRendererArrayhook for multi-layer support with context sharinguseLayerRendererhook for child layers to get context-managed renderersPointRendererArrayContextfor sharing renderer settings with child componentsuseRendererDragHandlershook (renamed fromusePixiDragHandlers)skipContextRegistrationoption for renderers that should not participate in context poolingMap Integration:
useLayerRendererMapComponentprovidesPointRendererArrayContextto child layersMapPointLayeruses context-managed renderers instead of creating them directlysetupResizeObserverandsetupBackgroundEventDistributionmethods for post-init configurationType Improvements:
IPointopaque handle instead of exposing PIXI.SpriteIPointMetadatawith caseID, plotNum, datasetID, position, stylePointEventHandlertype replacingCompatiblePointEventHandlerwithanyBackgroundPassThroughEventenum (renamed fromPixiBackgroundPassThroughEvent) moved topoint-renderer-types.tsCode Cleanup:
pixi-points.ts(legacy PixiPoints class)use-renderer-array.ts(replaced by context-based approach)getSpriteForPoint()method and migration commentsas anycastsAPI Changes:
onPointerOver,onPointerLeave,onPointerClick,onPointerDragStart,onPointerDrag,onPointerDragEndIRendererDragHandlersinterface (renamed fromIPixiDragHandlers)PointRendererArraytype moved to avoid circular dependencyCypress Test Fixes
The refactoring required updates to Cypress test helpers and uncovered several timing-related test flakiness issues that were fixed:
Helper Updates:
axis-helper.ts,graph-canvas-helper.ts, andmap-canvas-helper.tsto use new renderer API (rendererArrayMap,getPointsBounds, etc.)Test Stability Fixes:
table.spec.tsintotable.spec.tsandtable-index.spec.tsfor better parallelizationCode Fixes Discovered During Testing:
showConnectingLinesby reading from storeTest plan
🤖 Generated with Claude Code