From 7f2e35900b3adee5e71d3eb4f5b3e6151cdf6155 Mon Sep 17 00:00:00 2001 From: Markus Stange Date: Mon, 1 May 2023 13:33:50 -0400 Subject: [PATCH 1/3] Supply cssToDeviceScale to drawCanvas. In the past, the components which draw in device space were using window.devicePixelRatio and relying on the assumption that this is the same scale that ChartCanvas was using when it set the canvas backing size and CSS size. This assumption is correct, but it still feels better to pass the scale along explicitly, rather than assuming something about the global state. I'm also adding some checks so that it's obvious to readers of the drawCanvas methods how the canvas scale is controlled. --- src/components/flame-graph/Canvas.js | 17 +++++++++++- src/components/js-tracer/Canvas.js | 40 +++++++++++++++++---------- src/components/marker-chart/Canvas.js | 23 +++++++++++++-- src/components/shared/chart/Canvas.js | 34 ++++++++++++++++++----- src/components/stack-chart/Canvas.js | 37 +++++++++++++++++-------- 5 files changed, 114 insertions(+), 37 deletions(-) diff --git a/src/components/flame-graph/Canvas.js b/src/components/flame-graph/Canvas.js index 0ccb59cc7e..f19c524ad4 100644 --- a/src/components/flame-graph/Canvas.js +++ b/src/components/flame-graph/Canvas.js @@ -43,6 +43,11 @@ import type { IndexIntoFlameGraphTiming, } from 'firefox-profiler/profile-logic/flame-graph'; +import type { + ChartCanvasScale, + ChartCanvasHoverInfo, +} from '../shared/chart/Canvas'; + import type { CallTree } from 'firefox-profiler/profile-logic/call-tree'; export type OwnProps = {| @@ -149,7 +154,8 @@ class FlameGraphCanvasImpl extends React.PureComponent { _drawCanvas = ( ctx: CanvasRenderingContext2D, - hoveredItem: HoveredStackTiming | null + scale: ChartCanvasScale, + hoverInfo: ChartCanvasHoverInfo ) => { const { thread, @@ -168,6 +174,15 @@ class FlameGraphCanvasImpl extends React.PureComponent { }, } = this.props; + const { hoveredItem } = hoverInfo; + + const { cssToUserScale } = scale; + if (cssToUserScale !== 1) { + throw new Error( + 'FlameGraphCanvasImpl sets scaleCtxToCssPixels={true}, so canvas user space units should be equal to CSS pixels.' + ); + } + // Ensure the text measurement tool is created, since this is the first time // this class has access to a ctx. if (!this._textMeasurement) { diff --git a/src/components/js-tracer/Canvas.js b/src/components/js-tracer/Canvas.js index 79bac30a8f..5642ed8d04 100644 --- a/src/components/js-tracer/Canvas.js +++ b/src/components/js-tracer/Canvas.js @@ -32,6 +32,11 @@ import type { JsTracerTiming, } from 'firefox-profiler/types'; +import type { + ChartCanvasScale, + ChartCanvasHoverInfo, +} from '../shared/chart/Canvas'; + import type { WrapFunctionInDispatch } from 'firefox-profiler/utils/connect'; type OwnProps = {| @@ -99,7 +104,8 @@ class JsTracerCanvasImpl extends React.PureComponent { */ drawCanvas = ( ctx: CanvasRenderingContext2D, - hoveredItem: IndexIntoJsTracerEvents | null + scale: ChartCanvasScale, + hoverInfo: ChartCanvasHoverInfo ) => { const { rowHeight, @@ -111,11 +117,17 @@ class JsTracerCanvasImpl extends React.PureComponent { containerHeight, }, } = this.props; + const { hoveredItem } = hoverInfo; - const { devicePixelRatio } = window; + const { cssToDeviceScale, cssToUserScale } = scale; + if (cssToDeviceScale !== cssToUserScale) { + throw new Error( + 'JsTracerCanvasImpl sets scaleCtxToCssPixels={false}, so canvas user space units should be equal to device pixels.' + ); + } // Set the font size before creating a text measurer. - ctx.font = `${FONT_SIZE * devicePixelRatio}px sans-serif`; + ctx.font = `${FONT_SIZE * cssToDeviceScale}px sans-serif`; const renderPass: RenderPass = { ctx, @@ -130,19 +142,19 @@ class JsTracerCanvasImpl extends React.PureComponent { ), devicePixels: { // Convert many of the common values provided by the Props into DevicePixels. - containerWidth: containerWidth * devicePixelRatio, + containerWidth: containerWidth * cssToDeviceScale, innerContainerWidth: (containerWidth - TIMELINE_MARGIN_LEFT - TIMELINE_MARGIN_RIGHT) * - devicePixelRatio, - containerHeight: containerHeight * devicePixelRatio, - textOffsetStart: TEXT_OFFSET_START * devicePixelRatio, - textOffsetTop: TEXT_OFFSET_TOP * devicePixelRatio, - rowHeight: rowHeight * devicePixelRatio, - viewportTop: viewportTop * devicePixelRatio, - timelineMarginLeft: TIMELINE_MARGIN_LEFT * devicePixelRatio, - timelineMarginRight: TIMELINE_MARGIN_RIGHT * devicePixelRatio, - oneCssPixel: devicePixelRatio, - rowLabelOffsetLeft: ROW_LABEL_OFFSET_LEFT * devicePixelRatio, + cssToDeviceScale, + containerHeight: containerHeight * cssToDeviceScale, + textOffsetStart: TEXT_OFFSET_START * cssToDeviceScale, + textOffsetTop: TEXT_OFFSET_TOP * cssToDeviceScale, + rowHeight: rowHeight * cssToDeviceScale, + viewportTop: viewportTop * cssToDeviceScale, + timelineMarginLeft: TIMELINE_MARGIN_LEFT * cssToDeviceScale, + timelineMarginRight: TIMELINE_MARGIN_RIGHT * cssToDeviceScale, + oneCssPixel: cssToDeviceScale, + rowLabelOffsetLeft: ROW_LABEL_OFFSET_LEFT * cssToDeviceScale, }, }; diff --git a/src/components/marker-chart/Canvas.js b/src/components/marker-chart/Canvas.js index 466085dfd2..b939d960c7 100644 --- a/src/components/marker-chart/Canvas.js +++ b/src/components/marker-chart/Canvas.js @@ -32,6 +32,11 @@ import type { } from 'firefox-profiler/types'; import { getStartEndRangeForMarker } from 'firefox-profiler/utils'; +import type { + ChartCanvasScale, + ChartCanvasHoverInfo, +} from '../shared/chart/Canvas'; + import type { WrapFunctionInDispatch } from 'firefox-profiler/utils/connect'; type MarkerDrawingInformation = {| @@ -99,9 +104,8 @@ class MarkerChartCanvasImpl extends React.PureComponent { drawCanvas = ( ctx: CanvasRenderingContext2D, - hoveredItems: HoveredMarkerChartItems | null, - prevHoveredItems: HoveredMarkerChartItems | null, - isHoveredOnlyDifferent: boolean + scale: ChartCanvasScale, + hoverInfo: ChartCanvasHoverInfo ) => { const { rowHeight, @@ -120,6 +124,12 @@ class MarkerChartCanvasImpl extends React.PureComponent { let prevHoveredMarker = null; let prevHoveredLabel = null; + const { + hoveredItem: hoveredItems, + prevHoveredItem: prevHoveredItems, + isHoveredOnlyDifferent, + } = hoverInfo; + if (hoveredItems) { hoveredMarker = hoveredItems.markerIndex; hoveredLabel = hoveredItems.rowIndexOfLabel; @@ -129,6 +139,13 @@ class MarkerChartCanvasImpl extends React.PureComponent { prevHoveredLabel = prevHoveredItems.rowIndexOfLabel; } + const { cssToUserScale } = scale; + if (cssToUserScale !== 1) { + throw new Error( + 'StackChartCanvasImpl sets scaleCtxToCssPixels={true}, so canvas user space units should be equal to CSS pixels.' + ); + } + // Convert CssPixels to Stack Depth const startRow = Math.floor(viewportTop / rowHeight); const endRow = Math.min( diff --git a/src/components/shared/chart/Canvas.js b/src/components/shared/chart/Canvas.js index 526da30383..753e350cc5 100644 --- a/src/components/shared/chart/Canvas.js +++ b/src/components/shared/chart/Canvas.js @@ -20,9 +20,8 @@ type Props = {| +getHoveredItemInfo: (HoveredItem) => React.Node, +drawCanvas: ( CanvasRenderingContext2D, - hoveredItem: HoveredItem | null, - prevHoveredItem: HoveredItem | null, - isHoveredOnlyDifferent: boolean + ChartCanvasScale: ChartCanvasScale, + ChartCanvasHoverInfo: ChartCanvasHoverInfo ) => void, +isDragging: boolean, // Applies ctx.scale() to the canvas to draw using CssPixels rather than DevicePixels. @@ -41,6 +40,19 @@ type State = { pageY: CssPixels, }; +export type ChartCanvasScale = { + // Always equal to devicePixelRatio + cssToDeviceScale: number, + // 1 if scaleCtxToCssPixels is true, otherwise equal to cssToDeviceScale + cssToUserScale: number, +}; + +export type ChartCanvasHoverInfo = { + hoveredItem: HoveredItem | null, + prevHoveredItem: HoveredItem | null, + isHoveredOnlyDifferent: boolean, +}; + import './Canvas.css'; /** @@ -160,15 +172,23 @@ export class ChartCanvas extends React.Component< isHoveredOnlyDifferent: boolean = false, prevHoveredItem: HoveredItem | null = null ) { - const { className, drawCanvas } = this.props; + const { className, drawCanvas, scaleCtxToCssPixels } = this.props; + const { hoveredItem } = this.state; if (this._canvas) { timeCode(`${className} render`, () => { this._prepCanvas(); + const scale = this._devicePixelRatio; drawCanvas( this._ctx, - this.state.hoveredItem, - prevHoveredItem, - isHoveredOnlyDifferent + { + cssToDeviceScale: scale, + cssToUserScale: scaleCtxToCssPixels ? 1 : scale, + }, + { + hoveredItem, + prevHoveredItem, + isHoveredOnlyDifferent, + } ); }); } diff --git a/src/components/stack-chart/Canvas.js b/src/components/stack-chart/Canvas.js index 6e6a82ac2f..e5defe505f 100644 --- a/src/components/stack-chart/Canvas.js +++ b/src/components/stack-chart/Canvas.js @@ -39,6 +39,11 @@ import type { Page, } from 'firefox-profiler/types'; +import type { + ChartCanvasScale, + ChartCanvasHoverInfo, +} from '../shared/chart/Canvas'; + import type { StackTimingDepth, IndexIntoStackTiming, @@ -143,7 +148,8 @@ class StackChartCanvasImpl extends React.PureComponent { */ _drawCanvas = ( ctx: CanvasRenderingContext2D, - hoveredItem: HoveredStackTiming | null + scale: ChartCanvasScale, + hoverInfo: ChartCanvasHoverInfo ) => { const { thread, @@ -165,16 +171,23 @@ class StackChartCanvasImpl extends React.PureComponent { viewportBottom, }, } = this.props; + const { hoveredItem } = hoverInfo; + const fastFillStyle = new FastFillStyle(ctx); - const { devicePixelRatio } = window; + const { cssToDeviceScale, cssToUserScale } = scale; + if (cssToDeviceScale !== cssToUserScale) { + throw new Error( + 'StackChartCanvasImpl sets scaleCtxToCssPixels={false}, so canvas user space units should be equal to device pixels.' + ); + } // Set the font size before creating a text measurer. - ctx.font = `${FONT_SIZE * devicePixelRatio}px sans-serif`; + ctx.font = `${FONT_SIZE * cssToDeviceScale}px sans-serif`; const textMeasurement = new TextMeasurement(ctx); - const devicePixelsWidth = containerWidth * devicePixelRatio; - const devicePixelsHeight = containerHeight * devicePixelRatio; + const devicePixelsWidth = containerWidth * cssToDeviceScale; + const devicePixelsHeight = containerHeight * cssToDeviceScale; fastFillStyle.set('#ffffff'); ctx.fillRect(0, 0, devicePixelsWidth, devicePixelsHeight); @@ -182,7 +195,7 @@ class StackChartCanvasImpl extends React.PureComponent { const rangeLength: Milliseconds = rangeEnd - rangeStart; const viewportLength: UnitIntervalOfProfileRange = viewportRight - viewportLeft; - const viewportDevicePixelsTop = viewportTop * devicePixelRatio; + const viewportDevicePixelsTop = viewportTop * cssToDeviceScale; // Convert CssPixels to Stack Depth const startDepth = Math.floor(viewportTop / stackFrameHeight); @@ -190,24 +203,24 @@ class StackChartCanvasImpl extends React.PureComponent { const innerContainerWidth = containerWidth - marginLeft - TIMELINE_MARGIN_RIGHT; - const innerDevicePixelsWidth = innerContainerWidth * devicePixelRatio; + const innerDevicePixelsWidth = innerContainerWidth * cssToDeviceScale; const pixelAtViewportPosition = ( viewportPosition: UnitIntervalOfProfileRange ): DevicePixels => - devicePixelRatio * + cssToDeviceScale * // The right hand side of this formula is all in CSS pixels. (marginLeft + ((viewportPosition - viewportLeft) * innerContainerWidth) / viewportLength); // Apply the device pixel ratio to various CssPixel constants. - const rowDevicePixelsHeight = ROW_CSS_PIXELS_HEIGHT * devicePixelRatio; - const oneCssPixelInDevicePixels = 1 * devicePixelRatio; + const rowDevicePixelsHeight = ROW_CSS_PIXELS_HEIGHT * cssToDeviceScale; + const oneCssPixelInDevicePixels = 1 * cssToDeviceScale; const textDevicePixelsOffsetStart = - TEXT_CSS_PIXELS_OFFSET_START * devicePixelRatio; + TEXT_CSS_PIXELS_OFFSET_START * cssToDeviceScale; const textDevicePixelsOffsetTop = - TEXT_CSS_PIXELS_OFFSET_TOP * devicePixelRatio; + TEXT_CSS_PIXELS_OFFSET_TOP * cssToDeviceScale; let categoryForUserTiming = categories.findIndex( (category) => category.name === 'JavaScript' ); From 8c44ab9866247269e77ce95068b774bfb07b98d3 Mon Sep 17 00:00:00 2001 From: Markus Stange Date: Mon, 1 May 2023 13:34:17 -0400 Subject: [PATCH 2/3] Cache TextMeasurer for stack chart / js tracer. --- src/components/js-tracer/Canvas.js | 19 ++++++- src/components/stack-chart/Canvas.js | 21 +++++++- .../__snapshots__/JsTracer.test.js.snap | 48 ----------------- .../__snapshots__/StackChart.test.js.snap | 52 ------------------- src/utils/text-measurement.js | 5 ++ 5 files changed, 41 insertions(+), 104 deletions(-) diff --git a/src/components/js-tracer/Canvas.js b/src/components/js-tracer/Canvas.js index 5642ed8d04..6f62c31033 100644 --- a/src/components/js-tracer/Canvas.js +++ b/src/components/js-tracer/Canvas.js @@ -97,6 +97,8 @@ class JsTracerCanvasImpl extends React.PureComponent { state = { hasFirstDraw: false, }; + _textMeasurement: null | TextMeasurement; + _textMeasurementCssToDeviceScale: number = 1; /** * This method is called by the ChartCanvas component whenever the canvas needs to @@ -126,12 +128,25 @@ class JsTracerCanvasImpl extends React.PureComponent { ); } - // Set the font size before creating a text measurer. + // Set the font before creating the text renderer. The font property resets + // automatically whenever the canvas size is changed, so we set it on every + // call. ctx.font = `${FONT_SIZE * cssToDeviceScale}px sans-serif`; + // Ensure the text measurement tool is created, since this is the first time + // this class has access to a ctx. We also need to recreate it when the scale + // changes because we are working with device coordinates. + if ( + !this._textMeasurement || + this._textMeasurementCssToDeviceScale !== cssToDeviceScale + ) { + this._textMeasurement = new TextMeasurement(ctx); + this._textMeasurementCssToDeviceScale = cssToDeviceScale; + } + const renderPass: RenderPass = { ctx, - textMeasurement: new TextMeasurement(ctx), + textMeasurement: this._textMeasurement, fastFillStyle: new FastFillStyle(ctx), // Define a start and end row, so that we only draw the events // that are vertically within view. diff --git a/src/components/stack-chart/Canvas.js b/src/components/stack-chart/Canvas.js index e5defe505f..b81a670bf8 100644 --- a/src/components/stack-chart/Canvas.js +++ b/src/components/stack-chart/Canvas.js @@ -94,6 +94,9 @@ const FONT_SIZE = 10; const BORDER_OPACITY = 0.4; class StackChartCanvasImpl extends React.PureComponent { + _textMeasurement: null | TextMeasurement; + _textMeasurementCssToDeviceScale: number = 1; + componentDidUpdate(prevProps) { // We want to scroll the selection into view when this component // is mounted, but using componentDidMount won't work here as the @@ -182,9 +185,23 @@ class StackChartCanvasImpl extends React.PureComponent { ); } - // Set the font size before creating a text measurer. + // Set the font before creating the text renderer. The font property resets + // automatically whenever the canvas size is changed, so we set it on every + // call. ctx.font = `${FONT_SIZE * cssToDeviceScale}px sans-serif`; - const textMeasurement = new TextMeasurement(ctx); + + // Ensure the text measurement tool is created, since this is the first time + // this class has access to a ctx. We also need to recreate it when the scale + // changes because we are working with device coordinates. + if ( + !this._textMeasurement || + this._textMeasurementCssToDeviceScale !== cssToDeviceScale + ) { + this._textMeasurement = new TextMeasurement(ctx); + this._textMeasurementCssToDeviceScale = cssToDeviceScale; + } + + const textMeasurement = this._textMeasurement; const devicePixelsWidth = containerWidth * cssToDeviceScale; const devicePixelsHeight = containerHeight * cssToDeviceScale; diff --git a/src/test/components/__snapshots__/JsTracer.test.js.snap b/src/test/components/__snapshots__/JsTracer.test.js.snap index 6bf5f31217..2752a5dab6 100644 --- a/src/test/components/__snapshots__/JsTracer.test.js.snap +++ b/src/test/components/__snapshots__/JsTracer.test.js.snap @@ -157,14 +157,6 @@ Array [ "set font", "10px sans-serif", ], - Array [ - "measureText", - "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ.()< /:-_", - ], - Array [ - "measureText", - "…", - ], Array [ "set fillStyle", "#ffffff", @@ -187,10 +179,6 @@ Array [ 200, 13, ], - Array [ - "measureText", - "https://mozilla.org", - ], Array [ "set fillStyle", "white", @@ -212,10 +200,6 @@ Array [ 179, 13, ], - Array [ - "measureText", - "Interpreter", - ], Array [ "set fillStyle", "white", @@ -244,10 +228,6 @@ Array [ 160, 13, ], - Array [ - "measureText", - "IonMonkey", - ], Array [ "set fillStyle", "white", @@ -294,10 +274,6 @@ Array [ "set fillStyle", "#000000", ], - Array [ - "measureText", - "Tracing Information", - ], Array [ "fillText", "Tracing Information", @@ -464,14 +440,6 @@ Array [ "set font", "10px sans-serif", ], - Array [ - "measureText", - "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ.()< /:-_", - ], - Array [ - "measureText", - "…", - ], Array [ "set fillStyle", "#ffffff", @@ -494,10 +462,6 @@ Array [ 200, 13, ], - Array [ - "measureText", - "https://mozilla.org", - ], Array [ "set fillStyle", "white", @@ -519,10 +483,6 @@ Array [ 179, 13, ], - Array [ - "measureText", - "Interpreter", - ], Array [ "set fillStyle", "white", @@ -551,10 +511,6 @@ Array [ 160, 13, ], - Array [ - "measureText", - "IonMonkey", - ], Array [ "set fillStyle", "white", @@ -601,10 +557,6 @@ Array [ "set fillStyle", "#000000", ], - Array [ - "measureText", - "Tracing Information", - ], Array [ "fillText", "Tracing Information", diff --git a/src/test/components/__snapshots__/StackChart.test.js.snap b/src/test/components/__snapshots__/StackChart.test.js.snap index 5bd0fc00a5..c53ccd82bb 100644 --- a/src/test/components/__snapshots__/StackChart.test.js.snap +++ b/src/test/components/__snapshots__/StackChart.test.js.snap @@ -179,14 +179,6 @@ Array [ "set font", "10px sans-serif", ], - Array [ - "measureText", - "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ.()< /:-_", - ], - Array [ - "measureText", - "…", - ], Array [ "set fillStyle", "#ffffff", @@ -284,10 +276,6 @@ Array [ 199.4, 15, ], - Array [ - "measureText", - "A", - ], Array [ "set fillStyle", "#000000", @@ -309,10 +297,6 @@ Array [ 199.4, 15, ], - Array [ - "measureText", - "B", - ], Array [ "set fillStyle", "#000000", @@ -334,10 +318,6 @@ Array [ 133.4, 15, ], - Array [ - "measureText", - "C", - ], Array [ "set fillStyle", "#000000", @@ -359,10 +339,6 @@ Array [ 65.4, 15, ], - Array [ - "measureText", - "H", - ], Array [ "set fillStyle", "#000000", @@ -384,10 +360,6 @@ Array [ 66.4, 15, ], - Array [ - "measureText", - "D", - ], Array [ "set fillStyle", "#000000", @@ -409,10 +381,6 @@ Array [ 66.4, 15, ], - Array [ - "measureText", - "F", - ], Array [ "set fillStyle", "#000000", @@ -434,10 +402,6 @@ Array [ 65.4, 15, ], - Array [ - "measureText", - "I", - ], Array [ "set fillStyle", "#000000", @@ -459,10 +423,6 @@ Array [ 66.4, 15, ], - Array [ - "measureText", - "E", - ], Array [ "set fillStyle", "#000000", @@ -484,10 +444,6 @@ Array [ 66.4, 15, ], - Array [ - "measureText", - "G", - ], Array [ "set fillStyle", "#000000", @@ -698,14 +654,6 @@ Array [ "set font", "10px sans-serif", ], - Array [ - "measureText", - "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ.()< /:-_", - ], - Array [ - "measureText", - "…", - ], Array [ "set fillStyle", "#ffffff", diff --git a/src/utils/text-measurement.js b/src/utils/text-measurement.js index 8b12d9f873..27e8c04371 100644 --- a/src/utils/text-measurement.js +++ b/src/utils/text-measurement.js @@ -8,6 +8,11 @@ * Measure the size of text for drawing within a 2d context. This will allow text * to be drawn in a constrained space. This class uses a variety of heuristics and * caching to make this process fast. + * + * All measurements are in user space coordinates of the context. When the + * context transform changes, these user space coordinates remain valid. They + * only become invalid when the context's font or font size changes. When this + * happens, a new TextMeasurement instance should be created. */ class TextMeasurement { _ctx: CanvasRenderingContext2D; From 2bf992ed4069277c5ba6b2e75425f0c9375774ef Mon Sep 17 00:00:00 2001 From: Markus Stange Date: Mon, 1 May 2023 14:41:23 -0400 Subject: [PATCH 3/3] Draw Flamegraph in device pixels. Fixes #4591. This changes how small boxes and gaps are handled. If a row consists of many adjacent narrow boxes, we will now fill that entire row with painted pixels. Before this patch, we would skip all narrow boxes and leave large parts of the rendering empty. --- src/components/flame-graph/Canvas.js | 128 ++++++++++++++---- .../__snapshots__/FlameGraph.test.js.snap | 59 ++++---- 2 files changed, 131 insertions(+), 56 deletions(-) diff --git a/src/components/flame-graph/Canvas.js b/src/components/flame-graph/Canvas.js index f19c524ad4..c6babf437e 100644 --- a/src/components/flame-graph/Canvas.js +++ b/src/components/flame-graph/Canvas.js @@ -26,6 +26,7 @@ import type { Thread, CategoryList, CssPixels, + DevicePixels, Milliseconds, CallNodeInfo, IndexIntoCallNodeTable, @@ -95,9 +96,32 @@ import './Canvas.css'; const ROW_HEIGHT = 16; const TEXT_OFFSET_START = 3; const TEXT_OFFSET_TOP = 11; +const FONT_SIZE = 10; + +/** + * Round the given value to integers, consistently rounding x.5 towards positive infinity. + * This is different from Math.round: Math.round rounds 0.5 to the right (to 1), and -0.5 + * to the left (to -1). + * snap should be preferred over Math.round for rounding coordinates which might + * be negative, so that there is no discontinuity when a box moves past zero. + */ +function snap(floatDeviceValue: DevicePixels): DevicePixels { + return Math.floor(floatDeviceValue + 0.5); +} + +/** + * Round the given value to a multiple of `integerFactor`. + */ +function snapValueToMultipleOf( + floatDeviceValue: DevicePixels, + integerFactor: number +): DevicePixels { + return snap(floatDeviceValue / integerFactor) * integerFactor; +} class FlameGraphCanvasImpl extends React.PureComponent { _textMeasurement: null | TextMeasurement; + _textMeasurementCssToDeviceScale: number = 1; componentDidUpdate(prevProps) { // If the stack depth changes (say, when changing the time range @@ -176,23 +200,41 @@ class FlameGraphCanvasImpl extends React.PureComponent { const { hoveredItem } = hoverInfo; - const { cssToUserScale } = scale; - if (cssToUserScale !== 1) { + const { cssToDeviceScale, cssToUserScale } = scale; + if (cssToDeviceScale !== cssToUserScale) { throw new Error( - 'FlameGraphCanvasImpl sets scaleCtxToCssPixels={true}, so canvas user space units should be equal to CSS pixels.' + 'FlameGraphCanvasImpl sets scaleCtxToCssPixels={false}, so canvas user space units should be equal to device pixels.' ); } + const deviceContainerWidth = containerWidth * cssToDeviceScale; + const deviceContainerHeight = containerHeight * cssToDeviceScale; + + // Set the font before creating the text renderer. The font property resets + // automatically whenever the canvas size is changed, so we set it on every + // call. + ctx.font = `${FONT_SIZE * cssToDeviceScale}px sans-serif`; + // Ensure the text measurement tool is created, since this is the first time - // this class has access to a ctx. - if (!this._textMeasurement) { + // this class has access to a ctx. We also need to recreate it when the scale + // changes because we are working with device coordinates. + if ( + !this._textMeasurement || + this._textMeasurementCssToDeviceScale !== cssToDeviceScale + ) { this._textMeasurement = new TextMeasurement(ctx); + this._textMeasurementCssToDeviceScale = cssToDeviceScale; } + const textMeasurement = this._textMeasurement; + const fastFillStyle = new FastFillStyle(ctx); + const deviceHorizontalPadding: DevicePixels = Math.round( + TEXT_OFFSET_START * cssToDeviceScale + ); fastFillStyle.set('#ffffff'); - ctx.fillRect(0, 0, containerWidth, containerHeight); + ctx.fillRect(0, 0, deviceContainerWidth, deviceContainerHeight); const startDepth = Math.floor( maxStackDepth - viewportBottom / stackFrameHeight @@ -200,6 +242,7 @@ class FlameGraphCanvasImpl extends React.PureComponent { const endDepth = Math.ceil(maxStackDepth - viewportTop / stackFrameHeight); // Only draw the stack frames that are vertically within view. + // The graph is drawn from bottom to top, in order of increasing depth. for (let depth = startDepth; depth < endDepth; depth++) { // Get the timing information for a row of stack frames. const stackTiming = flameGraphTiming[depth]; @@ -208,19 +251,45 @@ class FlameGraphCanvasImpl extends React.PureComponent { continue; } + const cssRowTop: CssPixels = + (maxStackDepth - depth - 1) * ROW_HEIGHT - viewportTop; + const cssRowBottom: CssPixels = + (maxStackDepth - depth) * ROW_HEIGHT - viewportTop; + const deviceRowTop: DevicePixels = snap(cssRowTop * cssToDeviceScale); + const deviceRowBottom: DevicePixels = + snap(cssRowBottom * cssToDeviceScale) - 1; + const deviceRowHeight: DevicePixels = deviceRowBottom - deviceRowTop; + + const deviceTextTop = + deviceRowTop + snap(TEXT_OFFSET_TOP * cssToDeviceScale); + for (let i = 0; i < stackTiming.length; i++) { - const startTime = stackTiming.start[i]; - const endTime = stackTiming.end[i]; + // For each box, snap the left and right edges to the nearest multiple + // of two device pixels. If both edges snap to the same value, the box + // becomes empty and is not drawn. + // + // Boxes which remain are at least two device pixels wide. We create a + // translucent gap the end of each box by shifting the right edge to the + // left by 0.8 device pixels, so that this gap pixel column is filled to + // 20%. + + const boxLeftFraction = stackTiming.start[i]; + const boxRightFraction = stackTiming.end[i]; + const deviceBoxLeftUnsnapped = boxLeftFraction * deviceContainerWidth; + const deviceBoxRightUnsnapped = boxRightFraction * deviceContainerWidth; + + const deviceBoxLeft: DevicePixels = snapValueToMultipleOf( + deviceBoxLeftUnsnapped, + 2 + ); + const deviceBoxRight: DevicePixels = + snapValueToMultipleOf(deviceBoxRightUnsnapped, 2) - 0.8; - const w: CssPixels = (endTime - startTime) * containerWidth; - if (w < 2) { - // Skip sending draw calls for sufficiently small boxes. + const deviceBoxWidth: DevicePixels = deviceBoxRight - deviceBoxLeft; + if (deviceBoxWidth <= 0) { + // Skip drawing boxes which snapped away to nothing. continue; } - const x: CssPixels = startTime * containerWidth; - const y: CssPixels = - (maxStackDepth - depth - 1) * ROW_HEIGHT - viewportTop; - const h: CssPixels = ROW_HEIGHT - 1; const callNodeIndex = stackTiming.callNode[i]; const isSelected = selectedCallNodeIndex === callNodeIndex; @@ -242,25 +311,32 @@ class FlameGraphCanvasImpl extends React.PureComponent { : colorStyles.unselectedFillStyle; fastFillStyle.set(background); - // Draw rect at an offset to ensure spacing between blocks. - ctx.fillRect(x + 1, y, w - 1, h); - - // TODO - L10N RTL. - // Constrain the x coordinate to the leftmost area. - const x2: CssPixels = Math.max(x, 0) + TEXT_OFFSET_START; - const w2: CssPixels = Math.max(0, w - (x2 - x)); - if (w2 > textMeasurement.minWidth) { + ctx.fillRect( + deviceBoxLeft, + deviceRowTop, + deviceBoxWidth, + deviceRowHeight + ); + + const deviceTextLeft: DevicePixels = + deviceBoxLeft + deviceHorizontalPadding; + const deviceTextWidth: DevicePixels = deviceBoxRight - deviceTextLeft; + if (deviceTextWidth > textMeasurement.minWidth) { const funcIndex = callNodeTable.func[callNodeIndex]; const funcName = thread.stringTable.getString( thread.funcTable.name[funcIndex] ); - const fittedText = textMeasurement.getFittedText(funcName, w2); + const fittedText = textMeasurement.getFittedText( + funcName, + deviceTextWidth + ); if (fittedText) { const foreground = isHighlighted ? colorStyles.selectedTextColor : '#000'; fastFillStyle.set(foreground); - ctx.fillText(fittedText, x2, y + TEXT_OFFSET_TOP); + // TODO - L10N RTL. + ctx.fillText(fittedText, deviceTextLeft, deviceTextTop); } } } @@ -428,7 +504,7 @@ class FlameGraphCanvasImpl extends React.PureComponent { containerWidth={containerWidth} containerHeight={containerHeight} isDragging={isDragging} - scaleCtxToCssPixels={true} + scaleCtxToCssPixels={false} onDoubleClickItem={this._onDoubleClick} getHoveredItemInfo={this._getHoveredStackInfo} drawCanvas={this._drawCanvas} diff --git a/src/test/components/__snapshots__/FlameGraph.test.js.snap b/src/test/components/__snapshots__/FlameGraph.test.js.snap index d3a55003cb..85d7b50d83 100644 --- a/src/test/components/__snapshots__/FlameGraph.test.js.snap +++ b/src/test/components/__snapshots__/FlameGraph.test.js.snap @@ -459,9 +459,8 @@ exports[`FlameGraph matches the snapshot 1`] = ` exports[`FlameGraph matches the snapshot 2`] = ` Array [ Array [ - "scale", - 1, - 1, + "set font", + "10px sans-serif", ], Array [ "measureText", @@ -488,9 +487,9 @@ Array [ ], Array [ "fillRect", - 1, + 0, 284, - 199, + 199.2, 15, ], Array [ @@ -513,9 +512,9 @@ Array [ ], Array [ "fillRect", - 1, + 0, 268, - 199, + 199.2, 15, ], Array [ @@ -538,9 +537,9 @@ Array [ ], Array [ "fillRect", - 1, + 0, 252, - 132.33333333333331, + 133.2, 15, ], Array [ @@ -563,9 +562,9 @@ Array [ ], Array [ "fillRect", - 134.33333333333331, + 134, 252, - 65.66666666666667, + 65.19999999999999, 15, ], Array [ @@ -579,7 +578,7 @@ Array [ Array [ "fillText", "H", - 136.33333333333331, + 137, 263, ], Array [ @@ -588,9 +587,9 @@ Array [ ], Array [ "fillRect", - 1, + 0, 236, - 65.66666666666666, + 65.2, 15, ], Array [ @@ -613,9 +612,9 @@ Array [ ], Array [ "fillRect", - 67.66666666666666, + 66, 236, - 65.66666666666666, + 67.19999999999999, 15, ], Array [ @@ -629,7 +628,7 @@ Array [ Array [ "fillText", "F", - 69.66666666666666, + 69, 247, ], Array [ @@ -638,9 +637,9 @@ Array [ ], Array [ "fillRect", - 134.33333333333331, + 134, 236, - 65.66666666666667, + 65.19999999999999, 15, ], Array [ @@ -654,7 +653,7 @@ Array [ Array [ "fillText", "I", - 136.33333333333331, + 137, 247, ], Array [ @@ -663,9 +662,9 @@ Array [ ], Array [ "fillRect", - 1, + 0, 220, - 65.66666666666666, + 65.2, 15, ], Array [ @@ -688,9 +687,9 @@ Array [ ], Array [ "fillRect", - 67.66666666666666, + 66, 220, - 65.66666666666666, + 67.19999999999999, 15, ], Array [ @@ -704,7 +703,7 @@ Array [ Array [ "fillText", "G", - 69.66666666666666, + 69, 231, ], Array [ @@ -713,9 +712,9 @@ Array [ ], Array [ "fillRect", - 67.66666666666666, + 66, 204, - 65.66666666666666, + 67.19999999999999, 15, ], Array [ @@ -729,7 +728,7 @@ Array [ Array [ "fillText", "J", - 69.66666666666666, + 69, 215, ], ] @@ -739,7 +738,7 @@ exports[`FlameGraph shows a tooltip with the resource information with categorie