Skip to content

Commit 2b29676

Browse files
lukecotterlukecotter
committed
feat: optimize ResizeObserver setup to prevent double rendering on init
1 parent 634247b commit 2b29676

2 files changed

Lines changed: 99 additions & 109 deletions

File tree

‎log-viewer/src/features/timeline/optimised/FlameChart.ts‎

Lines changed: 74 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -153,6 +153,9 @@ export class FlameChart<E extends EventNode = EventNode> {
153153

154154
private hitTestManager: HitTestManager | null = null;
155155

156+
// Flag to track if ResizeObserver has been set up (deferred until after first render)
157+
private resizeObserverActive = false;
158+
156159
// Selection orchestrator (owns selection state and rendering)
157160
private selectionOrchestrator: SelectionOrchestrator<E> | null = null;
158161
private viewportAnimator: ViewportAnimator | null = null;
@@ -239,7 +242,7 @@ export class FlameChart<E extends EventNode = EventNode> {
239242
// Store truncation markers for rendering
240243
this.markers.push(...markers);
241244

242-
// Get container dimensions
245+
// Get container dimensions for validation
243246
const { width, height } = container.getBoundingClientRect();
244247
if (width === 0 || height === 0) {
245248
throw new TimelineError(
@@ -256,28 +259,22 @@ export class FlameChart<E extends EventNode = EventNode> {
256259
: undefined,
257260
);
258261

259-
// Calculate minimap and metric strip heights BEFORE creating viewport
260-
// Viewport needs the available height for main timeline (excluding minimap + metric strip + gaps)
261-
// Metric strip starts collapsed, so use collapsed height for initial layout
262-
const minimapHeight = calculateMinimapHeight(height);
263-
const metricStripHeight = METRIC_STRIP_COLLAPSED_HEIGHT;
264-
const totalOverheadHeight = minimapHeight + MINIMAP_GAP + metricStripHeight + METRIC_STRIP_GAP;
265-
const mainTimelineHeight = height - totalOverheadHeight;
262+
// Initialize PixiJS Application first - creates DOM and measures dimensions after RAF.
263+
// This ensures flex layout is computed before we measure mainDiv's actual height.
264+
const { mainTimelineHeight } = await this.setupPixiApplication(width, height);
266265

267-
// Store offset for converting canvas-relative to container-relative coordinates
268-
this.mainTimelineYOffset = totalOverheadHeight;
266+
// Calculate mainTimelineYOffset from measured height
267+
// This is the vertical offset from container top to main timeline canvas
268+
this.mainTimelineYOffset = height - mainTimelineHeight;
269269

270-
// Create viewport manager with adjusted height for main timeline area
270+
// Create viewport manager with measured height for main timeline area
271271
this.viewport = new TimelineViewport(
272272
width,
273273
mainTimelineHeight,
274274
this.index.totalDuration,
275275
this.index.maxDepth,
276276
);
277277

278-
// Initialize PixiJS Application
279-
await this.setupPixiApplication(width, height);
280-
281278
// Setup coordinate system (Y-axis inversion)
282279
this.setupCoordinateSystem();
283280

@@ -410,12 +407,10 @@ export class FlameChart<E extends EventNode = EventNode> {
410407
// Setup keyboard handler
411408
this.setupKeyboardHandler();
412409

413-
// Pass the same dimensions that init() used to create the viewport.
414-
// This ensures ResizeObserver's initial callback (which fires with current container
415-
// dimensions) is correctly skipped, even if DOM manipulation during init caused
416-
// a layout shift that changed the container size.
417-
this.resizeHandler = new TimelineResizeHandler(container, this, width, height);
418-
this.resizeHandler.setupResizeObserver();
410+
// Container dimensions are unchanged by DOM setup (wrapper fills 100%).
411+
// The fix is measuring mainTimelineHeight from actual flexbox layout.
412+
// ResizeObserver setup is deferred until after first render to avoid double render on init.
413+
this.resizeHandler = new TimelineResizeHandler(container, this);
419414

420415
// Initialize search if enabled via options
421416
if (options.enableSearch) {
@@ -691,6 +686,13 @@ export class FlameChart<E extends EventNode = EventNode> {
691686
if (this.state && this.state.needsRender) {
692687
this.render();
693688
this.state.needsRender = false;
689+
690+
// Setup ResizeObserver after first render to avoid double render on init.
691+
// By this point, layout is finalized and ResizeObserver baseline matches rendered state.
692+
if (this.resizeHandler && !this.resizeObserverActive) {
693+
this.resizeHandler.setupResizeObserver();
694+
this.resizeObserverActive = true;
695+
}
694696
}
695697
this.renderLoopId = null;
696698
});
@@ -699,6 +701,7 @@ export class FlameChart<E extends EventNode = EventNode> {
699701

700702
/**
701703
* Handle window resize.
704+
* Calculates main timeline height by subtracting visible component heights.
702705
*/
703706
public resize(newWidth: number, newHeight: number): void {
704707
if (!this.app || !this.viewport || !this.container || !this.index) {
@@ -718,20 +721,31 @@ export class FlameChart<E extends EventNode = EventNode> {
718721

719722
const visibleWorldYBottom = -oldState.offsetY;
720723

721-
// Calculate new minimap, metric strip, and main timeline heights
722-
// Query actual metric strip height (respects collapsed/expanded state)
724+
// Calculate component heights, only including visible components
723725
const minimapHeight = calculateMinimapHeight(newHeight);
724-
const metricStripHeight =
725-
this.metricStripOrchestrator?.getHeight() ?? METRIC_STRIP_COLLAPSED_HEIGHT;
726-
const totalOverheadHeight = minimapHeight + MINIMAP_GAP + metricStripHeight + METRIC_STRIP_GAP;
726+
727+
// Only include metric strip in calculation if it's visible (has data)
728+
const metricStripVisible = this.metricStripOrchestrator?.getIsVisible() ?? false;
729+
const metricStripHeight = metricStripVisible
730+
? (this.metricStripOrchestrator?.getHeight() ?? METRIC_STRIP_COLLAPSED_HEIGHT)
731+
: 0;
732+
const metricStripGap = metricStripVisible ? METRIC_STRIP_GAP : 0;
733+
734+
// Calculate main timeline height by subtracting all visible component heights
735+
const totalOverheadHeight = minimapHeight + MINIMAP_GAP + metricStripHeight + metricStripGap;
727736
const mainTimelineHeight = newHeight - totalOverheadHeight;
728737

738+
if (mainTimelineHeight <= 0) {
739+
return; // Invalid state, skip resize
740+
}
741+
729742
// Update offset for converting canvas-relative to container-relative coordinates
730743
this.mainTimelineYOffset = totalOverheadHeight;
731744

732-
// Update orchestrators with new offset
733-
this.selectionOrchestrator?.setMainTimelineYOffset(this.mainTimelineYOffset);
734-
this.searchOrchestrator?.setMainTimelineYOffset(this.mainTimelineYOffset);
745+
// Update minimap div height
746+
if (this.minimapDiv) {
747+
this.minimapDiv.style.height = `${minimapHeight}px`;
748+
}
735749

736750
// Resize minimap orchestrator
737751
if (this.minimapOrchestrator) {
@@ -743,17 +757,13 @@ export class FlameChart<E extends EventNode = EventNode> {
743757
this.metricStripOrchestrator.resize(newWidth);
744758
}
745759

760+
// Update orchestrators with new offset
761+
this.selectionOrchestrator?.setMainTimelineYOffset(this.mainTimelineYOffset);
762+
this.searchOrchestrator?.setMainTimelineYOffset(this.mainTimelineYOffset);
763+
746764
// Resize main timeline app
747765
this.app.renderer.resize(newWidth, mainTimelineHeight);
748766

749-
// Update wrapper div heights
750-
if (this.wrapper) {
751-
const minimapDiv = this.wrapper.children[0] as HTMLElement;
752-
if (minimapDiv) {
753-
minimapDiv.style.height = `${minimapHeight}px`;
754-
}
755-
}
756-
757767
const newZoom = newWidth / visibleTimeRange;
758768
const newOffsetX = visibleTimeStart * newZoom;
759769
const newOffsetY = -visibleWorldYBottom;
@@ -806,6 +816,7 @@ export class FlameChart<E extends EventNode = EventNode> {
806816
/**
807817
* Update metric strip visibility based on whether there's data to display.
808818
* Hides the metric strip container and gap if no governor limit data exists.
819+
* Triggers resize to recalculate main timeline height after visibility change.
809820
*/
810821
private updateMetricStripVisibility(): void {
811822
const isVisible = this.metricStripOrchestrator?.getIsVisible() ?? false;
@@ -817,13 +828,22 @@ export class FlameChart<E extends EventNode = EventNode> {
817828
if (this.metricStripGapDiv) {
818829
this.metricStripGapDiv.style.display = display;
819830
}
831+
832+
// Trigger resize to recalculate main timeline height after visibility change
833+
if (this.container && isVisible) {
834+
const { width, height } = this.container.getBoundingClientRect();
835+
this.resize(width, height);
836+
}
820837
}
821838

822839
// ============================================================================
823840
// PRIVATE SETUP METHODS
824841
// ============================================================================
825842

826-
private async setupPixiApplication(width: number, height: number): Promise<void> {
843+
private async setupPixiApplication(
844+
width: number,
845+
height: number,
846+
): Promise<{ mainTimelineHeight: number }> {
827847
const ticker = PIXI.Ticker.shared;
828848
ticker.autoStart = false;
829849
ticker.stop();
@@ -832,12 +852,10 @@ export class FlameChart<E extends EventNode = EventNode> {
832852
sysTicker.autoStart = false;
833853
sysTicker.stop();
834854

835-
// Calculate minimap, metric strip, and main timeline heights
855+
// Calculate minimap, metric strip heights for fixed-height elements
836856
// Metric strip starts collapsed, so use collapsed height for initial layout
837857
const minimapHeight = calculateMinimapHeight(height);
838858
const metricStripHeight = METRIC_STRIP_COLLAPSED_HEIGHT;
839-
const totalOverheadHeight = minimapHeight + MINIMAP_GAP + metricStripHeight + METRIC_STRIP_GAP;
840-
const mainTimelineHeight = height - totalOverheadHeight;
841859

842860
// Create wrapper container with flexbox layout
843861
this.wrapper = document.createElement('div');
@@ -860,7 +878,7 @@ export class FlameChart<E extends EventNode = EventNode> {
860878
this.metricStripGapDiv = document.createElement('div');
861879
this.metricStripGapDiv.style.cssText = `height:${METRIC_STRIP_GAP}px;width:100%;flex-shrink:0;background:transparent`;
862880

863-
// Main timeline container (fills remaining space)
881+
// Main timeline container (fills remaining space via flex:1)
864882
const mainDiv = document.createElement('div');
865883
mainDiv.style.cssText = 'flex:1;width:100%;min-height:0';
866884

@@ -876,9 +894,21 @@ export class FlameChart<E extends EventNode = EventNode> {
876894
this.container.appendChild(this.wrapper);
877895
}
878896

897+
// Calculate main timeline height by subtracting fixed component heights.
898+
// At init time, all components are visible (metric strip hides later via setHeatStripTimeSeries).
899+
const mainTimelineHeight =
900+
height - minimapHeight - MINIMAP_GAP - metricStripHeight - METRIC_STRIP_GAP;
901+
902+
if (mainTimelineHeight <= 0) {
903+
throw new TimelineError(
904+
TimelineErrorCode.INVALID_CONTAINER,
905+
'Container height too small for timeline layout',
906+
);
907+
}
908+
879909
// Minimap app is created by MinimapOrchestrator in setupMinimap()
880910

881-
// Create main timeline app
911+
// Create main timeline app with measured height
882912
this.app = new PIXI.Application();
883913
await this.app.init({
884914
width,
@@ -893,6 +923,8 @@ export class FlameChart<E extends EventNode = EventNode> {
893923
this.app.ticker.stop();
894924
this.app.stage.eventMode = 'none';
895925
mainDiv.appendChild(this.app.canvas);
926+
927+
return { mainTimelineHeight };
896928
}
897929

898930
private setupCoordinateSystem(): void {

‎log-viewer/src/features/timeline/optimised/interaction/TimelineResizeHandler.ts‎

Lines changed: 25 additions & 67 deletions
Original file line numberDiff line numberDiff line change
@@ -26,62 +26,51 @@ export class TimelineResizeHandler {
2626
/**
2727
* @param containerRef - The container element to observe for resize
2828
* @param renderer - The resizable component to notify on resize
29-
* @param initialWidth - Initial width used by init (pass to ensure consistency)
30-
* @param initialHeight - Initial height used by init (pass to ensure consistency)
3129
*/
32-
constructor(
33-
containerRef: HTMLElement,
34-
renderer: IResizable,
35-
initialWidth?: number,
36-
initialHeight?: number,
37-
) {
30+
constructor(containerRef: HTMLElement, renderer: IResizable) {
3831
this.containerRef = containerRef;
3932
this.renderer = renderer;
4033

41-
// Pre-populate with the SAME dimensions that init() used.
42-
// This prevents double render on init: FlameChart.init() calls requestRender(),
43-
// and ResizeObserver fires immediately on observe() with the same dimensions.
44-
//
45-
// IMPORTANT: We must use the same dimensions that init() used to create the viewport,
46-
// not re-read from the container. DOM manipulation during init (adding canvases)
47-
// can cause layout shifts that change container dimensions between when init()
48-
// reads them and when this constructor runs.
49-
50-
// Fallback to reading from container (legacy behavior)
51-
const { width, height } =
52-
initialWidth && initialHeight
53-
? { width: initialWidth, height: initialHeight }
54-
: containerRef.getBoundingClientRect();
55-
this.lastResizeWidth = Math.round(width);
56-
this.lastResizeHeight = Math.round(height);
34+
// Dimensions will be populated when setupResizeObserver() is called.
35+
// This is deferred until after first render to avoid double render on init.
36+
this.lastResizeWidth = 0;
37+
this.lastResizeHeight = 0;
5738
}
5839

5940
public setupResizeObserver(): void {
6041
if (!this.containerRef) {
6142
return;
6243
}
6344

64-
// PERF: Skip first callback unconditionally.
65-
// ResizeObserver fires immediately on observe(), and even with dimension
66-
// pre-population, DOM manipulation during init can cause layout shifts.
67-
// Skipping the first callback eliminates ~100ms duplicate render.
68-
let skipFirstCallback = true;
45+
// Read current dimensions as baseline (after layout is finalized from first render).
46+
// This ensures ResizeObserver only triggers for actual subsequent resizes.
47+
const { width, height } = this.containerRef.getBoundingClientRect();
48+
this.lastResizeWidth = Math.round(width);
49+
this.lastResizeHeight = Math.round(height);
6950

7051
this.resizeObserver = new ResizeObserver(() => {
71-
if (skipFirstCallback) {
72-
skipFirstCallback = false;
73-
return;
52+
// Check dimensions immediately - handles initial callback naturally
53+
// If dimensions match what init() used, skip (no redundant render)
54+
// If dimensions changed (layout shift during init), handle it
55+
const { width, height } = this.containerRef.getBoundingClientRect();
56+
const roundedWidth = Math.round(width);
57+
const roundedHeight = Math.round(height);
58+
59+
if (roundedWidth === this.lastResizeWidth && roundedHeight === this.lastResizeHeight) {
60+
return; // Skip if unchanged (covers initial callback case)
7461
}
7562

76-
// Debounce resize handling to prevent flickering
77-
// Clear any existing frame request
63+
// Update dimensions before debounce to prevent rapid duplicate checks
64+
this.lastResizeWidth = roundedWidth;
65+
this.lastResizeHeight = roundedHeight;
66+
67+
// Debounce actual resize handling to prevent flickering
7868
if (this.resizeDebounceFrameId !== null) {
7969
cancelAnimationFrame(this.resizeDebounceFrameId);
8070
}
8171

82-
// Schedule resize handling on next frame
8372
this.resizeDebounceFrameId = requestAnimationFrame(() => {
84-
this.handleResize();
73+
this.renderer?.resize(roundedWidth, roundedHeight);
8574
this.resizeDebounceFrameId = null;
8675
});
8776
});
@@ -102,35 +91,4 @@ export class TimelineResizeHandler {
10291
this.resizeObserver = null;
10392
}
10493
}
105-
106-
/**
107-
* Handle container resize efficiently without full re-initialization.
108-
* Preserves viewport zoom/pan state.
109-
*/
110-
private handleResize(): void {
111-
if (!this.containerRef || !this.renderer) {
112-
return;
113-
}
114-
115-
const { width, height } = this.containerRef.getBoundingClientRect();
116-
if (width <= 0 || height <= 0) {
117-
return;
118-
}
119-
120-
// Round to prevent sub-pixel resize thrashing
121-
const roundedWidth = Math.round(width);
122-
const roundedHeight = Math.round(height);
123-
124-
// Skip if dimensions haven't actually changed (prevents duplicate calls)
125-
if (roundedWidth === this.lastResizeWidth && roundedHeight === this.lastResizeHeight) {
126-
return;
127-
}
128-
129-
// Update last resize dimensions
130-
this.lastResizeWidth = roundedWidth;
131-
this.lastResizeHeight = roundedHeight;
132-
133-
// Use efficient resize method that preserves state
134-
this.renderer.resize(roundedWidth, roundedHeight);
135-
}
13694
}

0 commit comments

Comments
 (0)