Skip to content

refactor: streamline selection renderer by removing unused state and simplifying context setting #709

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
lcottercertinia merged 1 commit into from
Jan 22, 2026
+26 −99
Merged

refactor: streamline selection renderer by removing unused state and simplifying context setting #709

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
38 changes: 7 additions & 31 deletions log-viewer/src/features/timeline/optimised/FlameChart.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -344,11 +344,7 @@ export class FlameChart<E extends EventNode = EventNode> {
if (this.worldContainer) {
this.selectionRenderer = new SelectionHighlightRenderer(this.worldContainer);
// Set marker context for marker selection highlighting
this.selectionRenderer.setMarkerContext(
this.markers,
this.index.totalDuration,
this.index.maxDepth,
);
this.selectionRenderer.setMarkerContext(this.markers, this.index.totalDuration);
}

// Setup interaction handler
Expand Down Expand Up @@ -494,9 +490,6 @@ export class FlameChart<E extends EventNode = EventNode> {

// Initialize search renderer (for borders/overlays on current match)
this.searchRenderer = new SearchHighlightRenderer(this.worldContainer);

// Ensure zIndex layering is honored for highlight graphics
this.worldContainer.sortableChildren = true;
}

/**
Expand Down Expand Up @@ -1105,11 +1098,6 @@ export class FlameChart<E extends EventNode = EventNode> {
private selectFrame(node: TreeNode<E>): void {
this.selectionManager?.select(node);

// Update selection renderer
if (this.selectionRenderer) {
this.selectionRenderer.setSelection(node as TreeNode<EventNode>);
}

// Notify callback
if (this.callbacks.onSelect) {
this.callbacks.onSelect(node.data);
Expand Down Expand Up @@ -1158,11 +1146,6 @@ export class FlameChart<E extends EventNode = EventNode> {
private selectMarker(marker: TimelineMarker): void {
this.selectionManager?.selectMarker(marker);

// Update selection renderer
if (this.selectionRenderer) {
this.selectionRenderer.setMarkerSelection(marker);
}

// Notify callback
if (this.callbacks.onMarkerSelect) {
this.callbacks.onMarkerSelect(marker);
Expand All @@ -1187,13 +1170,8 @@ export class FlameChart<E extends EventNode = EventNode> {
// Navigate and get the new marker (or null if at boundary)
const nextMarker = this.selectionManager.navigateMarker(direction);

// If navigation found a valid marker, update renderer and center viewport
// If navigation found a valid marker, center viewport and notify
if (nextMarker) {
// Update selection renderer
if (this.selectionRenderer) {
this.selectionRenderer.setMarkerSelection(nextMarker);
}

// Notify callback
if (this.callbacks.onMarkerSelect) {
this.callbacks.onMarkerSelect(nextMarker);
Expand Down Expand Up @@ -1235,13 +1213,8 @@ export class FlameChart<E extends EventNode = EventNode> {
// Navigate and get the new node (or null if at boundary)
const nextNode = this.selectionManager.navigate(direction);

// If navigation found a valid node, update renderer and center viewport
// If navigation found a valid node, center viewport and notify
if (nextNode) {
// Update selection renderer
if (this.selectionRenderer) {
this.selectionRenderer.setSelection(nextNode as TreeNode<EventNode>);
}

// Notify callback
if (this.callbacks.onSelect) {
this.callbacks.onSelect(nextNode.data);
Expand Down Expand Up @@ -1586,7 +1559,10 @@ export class FlameChart<E extends EventNode = EventNode> {
const highlightMode = this.getHighlightMode();
if (highlightMode === 'selection') {
// Selection highlight mode: show selection highlight
this.selectionRenderer?.render(viewportState);
// Pass selection state from SelectionManager (single source of truth)
const selectedNode = this.selectionManager?.getSelected() as TreeNode<EventNode> | null;
const selectedMarker = this.selectionManager?.getSelectedMarker() ?? null;
this.selectionRenderer?.render(viewportState, selectedNode, selectedMarker);
this.searchRenderer?.clear();
} else if (highlightMode === 'search') {
// Search highlight mode: show search match highlight
Expand Down
87 changes: 19 additions & 68 deletions log-viewer/src/features/timeline/optimised/selection/SelectionHighlightRenderer.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,21 +61,12 @@ export class SelectionHighlightRenderer {
/** Graphics for marker selection highlight (renders behind frames) */
private markerGraphics: PIXI.Graphics;

/** Currently selected node (frame) */
private selectedNode: TreeNode<EventNode> | null = null;

/** Currently selected marker */
private selectedMarker: TimelineMarker | null = null;

/** All markers for duration calculation */
private markers: TimelineMarker[] = [];

/** Timeline end time for last marker duration calculation */
private timelineEnd = 0;

/** Max depth in timeline for marker height calculation */
private maxDepth = 0;

/** Highlight colors extracted from CSS variables (same as search) */
private colors: HighlightColors;

Expand All @@ -97,84 +88,49 @@ export class SelectionHighlightRenderer {
this.colors = extractHighlightColors();
}

/**
* Set the currently selected node (frame).
* Clears marker selection (mutually exclusive).
*
* @param node - TreeNode to select, or null to clear selection
*/
public setSelection(node: TreeNode<EventNode> | null): void {
this.selectedNode = node;
this.selectedMarker = null; // Clear marker selection
}

/**
* Set the currently selected marker.
* Clears frame selection (mutually exclusive).
*
* @param marker - TimelineMarker to select, or null to clear selection
*/
public setMarkerSelection(marker: TimelineMarker | null): void {
this.selectedMarker = marker;
this.selectedNode = null; // Clear frame selection
}

/**
* Set markers array and timeline parameters for marker selection.
* Required for calculating marker duration.
*
* @param markers - Array of timeline markers (sorted by startTime)
* @param timelineEnd - End time of timeline in nanoseconds
* @param maxDepth - Maximum depth in timeline
*/
public setMarkerContext(markers: TimelineMarker[], timelineEnd: number, maxDepth: number): void {
public setMarkerContext(markers: TimelineMarker[], timelineEnd: number): void {
this.markers = markers;
this.timelineEnd = timelineEnd;
this.maxDepth = maxDepth;
}

/**
* Get the currently selected node (frame).
*
* @returns Currently selected TreeNode, or null if none
*/
public getSelection(): TreeNode<EventNode> | null {
return this.selectedNode;
}

/**
* Get the currently selected marker.
*
* @returns Currently selected TimelineMarker, or null if none
*/
public getMarkerSelection(): TimelineMarker | null {
return this.selectedMarker;
}

/**
* Render the selection highlight (frame or marker).
* This renderer is stateless - selection state is passed in from SelectionManager.
*
* @param viewport - Viewport state for culling and transforms
* @param selectedNode - Currently selected frame node, or null
* @param selectedMarker - Currently selected marker, or null
*/
public render(viewport: ViewportState): void {
public render(
viewport: ViewportState,
selectedNode: TreeNode<EventNode> | null,
selectedMarker: TimelineMarker | null,
): void {
// Clear both graphics
this.frameGraphics.clear();
this.markerGraphics.clear();

// Render marker selection if present (uses markerGraphics - behind frames)
if (this.selectedMarker) {
this.renderMarkerHighlight(viewport);
if (selectedMarker) {
this.renderMarkerHighlight(viewport, selectedMarker);
return;
}

// Render frame selection if present (uses frameGraphics - on top of frames)
if (!this.selectedNode) {
if (!selectedNode) {
return;
}

const bounds = this.calculateBounds(viewport);
const event = this.selectedNode.data;
const depth = this.selectedNode.depth ?? 0;
const event = selectedNode.data;
const depth = selectedNode.depth ?? 0;

// Check visibility
if (!this.isVisible(event, depth, bounds)) {
Expand All @@ -197,20 +153,17 @@ export class SelectionHighlightRenderer {
* Markers render as full-height vertical bands that cover the entire viewport height.
*
* @param viewport - Viewport state for transforms
* @param selectedMarker - The marker to highlight
*/
private renderMarkerHighlight(viewport: ViewportState): void {
if (!this.selectedMarker) {
return;
}

private renderMarkerHighlight(viewport: ViewportState, selectedMarker: TimelineMarker): void {
// Calculate marker duration (extends to next marker or timeline end)
const markerIndex = this.markers.findIndex((m) => m.id === this.selectedMarker!.id);
const markerIndex = this.markers.findIndex((m) => m.id === selectedMarker.id);
const nextMarker = this.markers[markerIndex + 1];
const markerEnd = nextMarker?.startTime ?? this.timelineEnd;
const duration = markerEnd - this.selectedMarker.startTime;
const duration = markerEnd - selectedMarker.startTime;

// Calculate screen position
const screenX = this.selectedMarker.startTime * viewport.zoom;
const screenX = selectedMarker.startTime * viewport.zoom;
const screenWidth = duration * viewport.zoom;

// Full height to cover entire visible viewport regardless of pan position
Expand Down Expand Up @@ -253,8 +206,6 @@ export class SelectionHighlightRenderer {
public clear(): void {
this.frameGraphics.clear();
this.markerGraphics.clear();
this.selectedNode = null;
this.selectedMarker = null;
}

/**
Expand Down
Loading