Skip to content

Commit fb4587b

Browse files
Merge pull request #741 from lukecotter/feat-tl-marker-color-updates
fix: use true alpha transparency for timeline marker colors
2 parents 93f9d43 + d2fd0c3 commit fb4587b

8 files changed

Lines changed: 42 additions & 104 deletions

File tree

log-viewer/src/features/timeline/__tests__/markers.test.ts

Lines changed: 19 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ class MockSprite {
1919
public width = 0;
2020
public height = 0;
2121
public tint = 0xffffff;
22+
public alpha = 1;
2223
public visible = true;
2324
public parent: unknown = null;
2425
public _zIndex = 0;
@@ -67,22 +68,11 @@ jest.mock('pixi.js', () => {
6768
});
6869

6970
import * as PIXI from 'pixi.js';
70-
import { blendWithBackground } from '../optimised/BucketColorResolver.js';
7171
import { TimelineMarkerRenderer } from '../optimised/markers/TimelineMarkerRenderer.js';
7272
import { TimelineViewport } from '../optimised/TimelineViewport.js';
7373
import type { TimelineMarker } from '../types/flamechart.types.js';
7474
import { MARKER_ALPHA, MARKER_COLORS } from '../types/flamechart.types.js';
7575

76-
/**
77-
* Pre-blended marker colors for testing (matches TimelineMarkerRenderer).
78-
* These are computed once to match the expected render output.
79-
*/
80-
const MARKER_COLORS_BLENDED = {
81-
error: blendWithBackground(MARKER_COLORS.error, MARKER_ALPHA),
82-
skip: blendWithBackground(MARKER_COLORS.skip, MARKER_ALPHA),
83-
unexpected: blendWithBackground(MARKER_COLORS.unexpected, MARKER_ALPHA),
84-
};
85-
8676
// Mock PIXI.Container
8777
class MockContainer {
8878
private children: unknown[] = [];
@@ -125,7 +115,7 @@ describe('TimelineMarkerRenderer', () => {
125115
});
126116

127117
describe('T013: Color Accuracy Verification', () => {
128-
it('should render error markers with pre-blended color via sprite tint', () => {
118+
it('should render error markers with raw color and alpha via sprite tint', () => {
129119
const markers: TimelineMarker[] = [
130120
{ id: 'marker-error', type: 'error', startTime: 100_000, summary: 'Test error' },
131121
];
@@ -137,14 +127,15 @@ describe('TimelineMarkerRenderer', () => {
137127
);
138128
renderer.render();
139129

140-
// Find the sprite with error color
130+
// Find the sprite with error color and alpha
141131
const errorSprites = createdMockSprites.filter(
142-
(s) => s.visible && s.tint === MARKER_COLORS_BLENDED.error,
132+
(s) => s.visible && s.tint === MARKER_COLORS.error,
143133
);
144134
expect(errorSprites.length).toBeGreaterThanOrEqual(1);
135+
expect(errorSprites[0]!.alpha).toBe(MARKER_ALPHA);
145136
});
146137

147-
it('should render skip markers with pre-blended color via sprite tint', () => {
138+
it('should render skip markers with raw color and alpha via sprite tint', () => {
148139
const markers: TimelineMarker[] = [
149140
{ id: 'marker-skip', type: 'skip', startTime: 100_000, summary: 'Test skip' },
150141
];
@@ -156,14 +147,15 @@ describe('TimelineMarkerRenderer', () => {
156147
);
157148
renderer.render();
158149

159-
// Find the sprite with skip color
150+
// Find the sprite with skip color and alpha
160151
const skipSprites = createdMockSprites.filter(
161-
(s) => s.visible && s.tint === MARKER_COLORS_BLENDED.skip,
152+
(s) => s.visible && s.tint === MARKER_COLORS.skip,
162153
);
163154
expect(skipSprites.length).toBeGreaterThanOrEqual(1);
155+
expect(skipSprites[0]!.alpha).toBe(MARKER_ALPHA);
164156
});
165157

166-
it('should render unexpected markers with pre-blended color via sprite tint', () => {
158+
it('should render unexpected markers with raw color and alpha via sprite tint', () => {
167159
const markers: TimelineMarker[] = [
168160
{
169161
id: 'marker-unexpected',
@@ -180,11 +172,12 @@ describe('TimelineMarkerRenderer', () => {
180172
);
181173
renderer.render();
182174

183-
// Find the sprite with unexpected color
175+
// Find the sprite with unexpected color and alpha
184176
const unexpectedSprites = createdMockSprites.filter(
185-
(s) => s.visible && s.tint === MARKER_COLORS_BLENDED.unexpected,
177+
(s) => s.visible && s.tint === MARKER_COLORS.unexpected,
186178
);
187179
expect(unexpectedSprites.length).toBeGreaterThanOrEqual(1);
180+
expect(unexpectedSprites[0]!.alpha).toBe(MARKER_ALPHA);
188181
});
189182
});
190183

@@ -300,7 +293,7 @@ describe('TimelineMarkerRenderer', () => {
300293
// First marker should be culled (ends at 200_000 < viewport start 250_000)
301294
// Second marker should be visible (200_000 to 1_000_000 overlaps 250_000)
302295
expect(visibleSprites.length).toBe(1);
303-
expect(visibleSprites[0]!.tint).toBe(MARKER_COLORS_BLENDED.skip);
296+
expect(visibleSprites[0]!.tint).toBe(MARKER_COLORS.skip);
304297
});
305298

306299
it('should cull markers entirely after viewport', () => {
@@ -412,11 +405,11 @@ describe('TimelineMarkerRenderer', () => {
412405
expect(visibleSprites.length).toBe(3);
413406

414407
// First sprite (skip at 100_000) should be leftmost
415-
expect(visibleSprites[0]!.tint).toBe(MARKER_COLORS_BLENDED.skip);
408+
expect(visibleSprites[0]!.tint).toBe(MARKER_COLORS.skip);
416409
// Second sprite (unexpected at 200_000)
417-
expect(visibleSprites[1]!.tint).toBe(MARKER_COLORS_BLENDED.unexpected);
410+
expect(visibleSprites[1]!.tint).toBe(MARKER_COLORS.unexpected);
418411
// Third sprite (error at 300_000)
419-
expect(visibleSprites[2]!.tint).toBe(MARKER_COLORS_BLENDED.error);
412+
expect(visibleSprites[2]!.tint).toBe(MARKER_COLORS.error);
420413
});
421414
});
422415

@@ -580,8 +573,8 @@ describe('TimelineMarkerRenderer', () => {
580573
expect(visibleSprites.length).toBe(2);
581574

582575
// Verify the new markers are rendered with correct tints
583-
const skipSprites = visibleSprites.filter((s) => s.tint === MARKER_COLORS_BLENDED.skip);
584-
const errorSprites = visibleSprites.filter((s) => s.tint === MARKER_COLORS_BLENDED.error);
576+
const skipSprites = visibleSprites.filter((s) => s.tint === MARKER_COLORS.skip);
577+
const errorSprites = visibleSprites.filter((s) => s.tint === MARKER_COLORS.error);
585578
expect(skipSprites.length).toBe(1);
586579
expect(errorSprites.length).toBe(1);
587580
});

log-viewer/src/features/timeline/optimised/RectangleShader.ts

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -41,8 +41,8 @@ void main() {
4141
/**
4242
* WebGL fragment shader (GLSL 300 ES)
4343
*
44-
* Simply outputs the interpolated vertex color.
45-
* Colors are pre-blended with background, so no alpha blending needed.
44+
* Outputs premultiplied alpha color for correct blending with PixiJS 8's
45+
* default blend mode (GL_ONE, GL_ONE_MINUS_SRC_ALPHA).
4646
*/
4747
const glslFragment = /* glsl */ `#version 300 es
4848
precision highp float;
@@ -51,7 +51,7 @@ in vec4 vColor;
5151
out vec4 fragColor;
5252
5353
void main() {
54-
fragColor = vColor;
54+
fragColor = vec4(vColor.rgb * vColor.a, vColor.a);
5555
}
5656
`;
5757

@@ -83,7 +83,7 @@ fn main(input: VertexInput) -> VertexOutput {
8383
/**
8484
* WebGPU fragment shader (WGSL)
8585
*
86-
* Same functionality as GLSL version for WebGPU renderer.
86+
* Outputs premultiplied alpha color for correct blending.
8787
*/
8888
const wgslFragment = /* wgsl */ `
8989
struct FragmentInput {
@@ -92,7 +92,7 @@ struct FragmentInput {
9292
9393
@fragment
9494
fn main(input: FragmentInput) -> @location(0) vec4f {
95-
return input.vColor;
95+
return vec4f(input.vColor.rgb * input.vColor.a, input.vColor.a);
9696
}
9797
`;
9898

log-viewer/src/features/timeline/optimised/markers/MarkerProcessor.ts

Lines changed: 2 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -10,22 +10,8 @@
1010
* used by both MeshMarkerRenderer and TimelineMarkerRenderer.
1111
*/
1212

13-
import type { MarkerType, TimelineMarker } from '../../types/flamechart.types.js';
14-
import { MARKER_ALPHA, MARKER_COLORS, SEVERITY_RANK } from '../../types/flamechart.types.js';
15-
import { blendWithBackground } from '../BucketColorResolver.js';
16-
17-
/**
18-
* Pre-blended opaque marker colors (MARKER_COLORS blended at MARKER_ALPHA opacity).
19-
* Computed once at module load time for performance.
20-
*
21-
* Using pre-blended colors avoids runtime alpha compositing on the GPU,
22-
* which is more efficient for static opacity values.
23-
*/
24-
export const MARKER_COLORS_BLENDED: Record<MarkerType, number> = {
25-
error: blendWithBackground(MARKER_COLORS.error, MARKER_ALPHA),
26-
skip: blendWithBackground(MARKER_COLORS.skip, MARKER_ALPHA),
27-
unexpected: blendWithBackground(MARKER_COLORS.unexpected, MARKER_ALPHA),
28-
};
13+
import type { TimelineMarker } from '../../types/flamechart.types.js';
14+
import { SEVERITY_RANK } from '../../types/flamechart.types.js';
2915

3016
/**
3117
* Sort markers by startTime, then by severity (higher severity first for stacking).

log-viewer/src/features/timeline/optimised/markers/MeshMarkerRenderer.ts

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -12,16 +12,17 @@
1212
* - Single Mesh draw call for all markers
1313
* - Direct buffer updates (no scene graph overhead)
1414
* - Clip-space coordinates (no uniform binding overhead)
15-
* - Pre-blended opaque colors (no alpha blending)
15+
* - True alpha transparency for theme-adaptive colors
1616
*/
1717

1818
import { Container, Geometry, Mesh, Shader } from 'pixi.js';
1919
import type { TimelineMarker } from '../../types/flamechart.types.js';
20+
import { MARKER_ALPHA, MARKER_COLORS } from '../../types/flamechart.types.js';
2021
import { RectangleGeometry, type ViewportTransform } from '../RectangleGeometry.js';
2122
import { createRectangleShader } from '../RectangleShader.js';
2223
import type { TimelineViewport } from '../TimelineViewport.js';
2324
import { hitTestMarkers, type MarkerIndicator } from './MarkerHitTest.js';
24-
import { MARKER_COLORS_BLENDED, sortMarkersByTimeAndSeverity } from './MarkerProcessor.js';
25+
import { sortMarkersByTimeAndSeverity } from './MarkerProcessor.js';
2526

2627
/**
2728
* Renders marker indicators as semi-transparent vertical bands using Mesh.
@@ -136,14 +137,13 @@ export class MeshMarkerRenderer {
136137
continue;
137138
}
138139

139-
// Create indicator record with pre-blended opaque color
140140
const indicator: MarkerIndicator = {
141141
marker,
142142
resolvedEndTime,
143143
screenStartX: worldStartX,
144144
screenEndX: worldEndX,
145145
screenWidth: worldWidth,
146-
color: MARKER_COLORS_BLENDED[marker.type],
146+
color: MARKER_COLORS[marker.type],
147147
isVisible: true,
148148
};
149149

@@ -193,6 +193,7 @@ export class MeshMarkerRenderer {
193193
viewportState.displayHeight,
194194
indicator.color,
195195
viewportTransform,
196+
MARKER_ALPHA,
196197
);
197198
rectIndex++;
198199
}

log-viewer/src/features/timeline/optimised/markers/TimelineMarkerRenderer.ts

Lines changed: 4 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -11,27 +11,16 @@
1111
* Performance optimizations:
1212
* - Uses SpritePool with shared 1x1 white texture for automatic GPU batching
1313
* - Sprites are pooled and reused (no GC overhead after warmup)
14-
* - Color applied via sprite.tint (pre-blended opaque colors)
14+
* - Color applied via sprite.tint with true alpha transparency
1515
*/
1616

1717
import type { Container } from 'pixi.js';
18-
import type { MarkerType, TimelineMarker } from '../../types/flamechart.types.js';
18+
import type { TimelineMarker } from '../../types/flamechart.types.js';
1919
import { MARKER_ALPHA, MARKER_COLORS, SEVERITY_RANK } from '../../types/flamechart.types.js';
20-
import { blendWithBackground } from '../BucketColorResolver.js';
2120
import { SpritePool } from '../SpritePool.js';
2221
import type { TimelineViewport } from '../TimelineViewport.js';
2322
import { hitTestMarkers, type MarkerIndicator } from './MarkerHitTest.js';
2423

25-
/**
26-
* Pre-blended opaque marker colors (MARKER_COLORS blended at MARKER_ALPHA opacity).
27-
* Computed once at module load time for performance.
28-
*/
29-
const MARKER_COLORS_BLENDED: Record<MarkerType, number> = {
30-
error: blendWithBackground(MARKER_COLORS.error, MARKER_ALPHA),
31-
skip: blendWithBackground(MARKER_COLORS.skip, MARKER_ALPHA),
32-
unexpected: blendWithBackground(MARKER_COLORS.unexpected, MARKER_ALPHA),
33-
};
34-
3524
/**
3625
* Renders marker indicators as semi-transparent vertical bands using sprites.
3726
*
@@ -130,14 +119,13 @@ export class TimelineMarkerRenderer {
130119
continue;
131120
}
132121

133-
// Create indicator record with pre-blended opaque color
134122
const indicator: MarkerIndicator = {
135123
marker,
136124
resolvedEndTime,
137125
screenStartX: worldStartX,
138126
screenEndX: worldEndX,
139127
screenWidth: worldWidth,
140-
color: MARKER_COLORS_BLENDED[marker.type],
128+
color: MARKER_COLORS[marker.type],
141129
isVisible: true,
142130
};
143131

@@ -161,6 +149,7 @@ export class TimelineMarkerRenderer {
161149
sprite.width = gappedWidth;
162150
sprite.height = viewportState.displayHeight;
163151
sprite.tint = indicator.color;
152+
sprite.alpha = MARKER_ALPHA;
164153
}
165154
}
166155

log-viewer/src/features/timeline/optimised/metric-strip/MetricStripRenderer.ts

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -34,15 +34,14 @@ import type {
3434
TimelineMarker,
3535
ViewportState,
3636
} from '../../types/flamechart.types.js';
37+
import { MARKER_ALPHA, MARKER_COLORS } from '../../types/flamechart.types.js';
3738
import {
3839
BREACH_AREA_OPACITY,
3940
DANGER_ZONE_OPACITY,
4041
getMetricStripColors,
4142
getTrafficLightColor,
4243
METRIC_STRIP_HEIGHT,
4344
METRIC_STRIP_LINE_WIDTHS,
44-
METRIC_STRIP_MARKER_COLORS_BLENDED,
45-
METRIC_STRIP_MARKER_OPACITY,
4645
METRIC_STRIP_THRESHOLDS,
4746
METRIC_STRIP_TOGGLE_WIDTH,
4847
METRIC_STRIP_Y_MAX_PERCENT,
@@ -402,7 +401,7 @@ export class MetricStripRenderer {
402401
continue;
403402
}
404403

405-
const color = METRIC_STRIP_MARKER_COLORS_BLENDED[marker.type];
404+
const color = MARKER_COLORS[marker.type];
406405
if (color === undefined) {
407406
continue;
408407
}
@@ -413,7 +412,7 @@ export class MetricStripRenderer {
413412

414413
if (gappedWidth > 0) {
415414
g.rect(gappedStartX, 0, gappedWidth, this.height);
416-
g.fill({ color, alpha: METRIC_STRIP_MARKER_OPACITY });
415+
g.fill({ color, alpha: MARKER_ALPHA });
417416
}
418417
}
419418
}

log-viewer/src/features/timeline/optimised/metric-strip/metric-strip-colors.ts

Lines changed: 0 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -9,8 +9,6 @@
99
* Colors are designed to be distinguishable on both light and dark backgrounds.
1010
*/
1111

12-
import type { MarkerType } from '../../types/flamechart.types.js';
13-
1412
/**
1513
* MetricStrip color palette.
1614
*/
@@ -264,22 +262,6 @@ export const METRIC_STRIP_TIME_GRID_COLOR = 0x808080;
264262
*/
265263
export const METRIC_STRIP_TIME_GRID_OPACITY = 0.3;
266264

267-
/**
268-
* Marker colors pre-blended with background for metric strip background bands.
269-
* These match the minimap marker colors for visual consistency.
270-
*/
271-
export const METRIC_STRIP_MARKER_COLORS_BLENDED: Record<MarkerType, number> = {
272-
error: 0xff8080, // Light red
273-
skip: 0x1e80ff, // Light blue
274-
unexpected: 0x8080ff, // Light purple
275-
};
276-
277-
/**
278-
* Marker band opacity in metric strip.
279-
* Matches MARKER_ALPHA (0.2) from main timeline for visual consistency.
280-
*/
281-
export const METRIC_STRIP_MARKER_OPACITY = 0.2;
282-
283265
/**
284266
* Width of the expand/collapse toggle area on the left side.
285267
*/

0 commit comments

Comments
 (0)