Skip to content

Commit

Permalink
Merge "ui: rearchitect area select to be simpler and fix several bugs…
Browse files Browse the repository at this point in the history 
…" into main
  • Loading branch information
@LalitMaganti
LalitMaganti authored and Gerrit Code Review committed Nov 1, 2024
2 parents bc31f98 + 4d0bb20 commit 0c01c75
Show file tree
Hide file tree
Showing 3 changed files with 92 additions and 73 deletions.
6 changes: 0 additions & 6 deletions ui/src/core/timeline.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,11 +22,6 @@ import {Timeline} from '../public/timeline';
import {timestampFormat, TimestampFormat} from './timestamp_format';
import {TraceInfo} from '../public/trace_info';

interface Range {
start?: number;
end?: number;
}

const MIN_DURATION = 10;

/**
Expand Down Expand Up @@ -82,7 +77,6 @@ export class TimelineImpl implements Timeline {
}

// This is used to calculate the tracks within a Y range for area selection.
areaY: Range = {};
private _selectedArea?: Area;

constructor(private readonly traceInfo: TraceInfo) {
Expand Down
95 changes: 33 additions & 62 deletions ui/src/frontend/panel_container.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,11 +26,7 @@ import {
import {raf} from '../core/raf_scheduler';
import {SimpleResizeObserver} from '../base/resize_observer';
import {canvasClip} from '../base/canvas_utils';
import {
SELECTION_STROKE_COLOR,
TOPBAR_HEIGHT,
TRACK_SHELL_WIDTH,
} from './css_constants';
import {SELECTION_STROKE_COLOR, TRACK_SHELL_WIDTH} from './css_constants';
import {Bounds2D, Size2D, VerticalBounds} from '../base/geom';
import {VirtualCanvas} from './virtual_canvas';
import {DisposableStack} from '../base/disposable_stack';
Expand Down Expand Up @@ -66,6 +62,8 @@ export type PanelOrGroup = Panel | PanelGroup;
export interface PanelContainerAttrs extends TraceImplAttrs {
panels: PanelOrGroup[];
className?: string;
selectedYRange: VerticalBounds | undefined;

onPanelStackResize?: (width: number, height: number) => void;

// Called after all panels have been rendered to the canvas, to give the
Expand All @@ -87,6 +85,7 @@ interface PanelInfo {
width: number;
clientX: number;
clientY: number;
absY: number;
}

export interface RenderedPanelInfo {
Expand All @@ -98,11 +97,7 @@ export class PanelContainer
implements m.ClassComponent<PanelContainerAttrs>, PerfStatsSource
{
private readonly trace: TraceImpl;

// These values are updated with proper values in oncreate.
// Y position of the panel container w.r.t. the client
private panelContainerTop = 0;
private panelContainerHeight = 0;
private attrs: PanelContainerAttrs;

// Updated every render cycle in the view() hook
private panelById = new Map<string, Panel>();
Expand All @@ -125,8 +120,9 @@ export class PanelContainer
private readonly PANEL_STACK_REF = 'panel-stack';

constructor({attrs}: m.CVnode<PanelContainerAttrs>) {
this.attrs = attrs;
this.trace = attrs.trace;
const onRedraw = () => this.renderCanvas(attrs);
const onRedraw = () => this.renderCanvas();
raf.addRedrawCallback(onRedraw);
this.trash.defer(() => {
raf.removeRedrawCallback(onRedraw);
Expand Down Expand Up @@ -155,8 +151,8 @@ export class PanelContainer
if (
realPosX + pos.width >= minX &&
realPosX <= maxX &&
pos.clientY + pos.height >= minY &&
pos.clientY <= maxY &&
pos.absY + pos.height >= minY &&
pos.absY <= maxY &&
pos.panel.selectable
) {
panels.push(pos.panel);
Expand All @@ -168,27 +164,15 @@ export class PanelContainer
// This finds the tracks covered by the in-progress area selection. When
// editing areaY is not set, so this will not be used.
handleAreaSelection() {
const {selectedYRange} = this.attrs;
const area = this.trace.timeline.selectedArea;
if (
area === undefined ||
this.trace.timeline.areaY.start === undefined ||
this.trace.timeline.areaY.end === undefined ||
selectedYRange === undefined ||
this.panelInfos.length === 0
) {
return;
}
// Only get panels from the current panel container if the selection began
// in this container.
const panelContainerTop = this.panelInfos[0].clientY;
const panelContainerBottom =
this.panelInfos[this.panelInfos.length - 1].clientY +
this.panelInfos[this.panelInfos.length - 1].height;
if (
this.trace.timeline.areaY.start + TOPBAR_HEIGHT < panelContainerTop ||
this.trace.timeline.areaY.start + TOPBAR_HEIGHT > panelContainerBottom
) {
return;
}

// TODO(stevegolton): We shouldn't know anything about visible time scale
// right now, that's a job for our parent, but we can put one together so we
Expand All @@ -204,9 +188,10 @@ export class PanelContainer
const panels = this.getPanelsInRegion(
visibleTimeScale.timeToPx(area.start),
visibleTimeScale.timeToPx(area.end),
this.trace.timeline.areaY.start + TOPBAR_HEIGHT,
this.trace.timeline.areaY.end + TOPBAR_HEIGHT,
selectedYRange.top,
selectedYRange.bottom,
);

// Get the track ids from the panels.
const trackUris: string[] = [];
for (const panel of panels) {
Expand Down Expand Up @@ -255,7 +240,7 @@ export class PanelContainer
});

virtualCanvas.setLayoutShiftListener(() => {
this.renderCanvas(vnode.attrs);
this.renderCanvas();
});

this.onupdate(vnode);
Expand Down Expand Up @@ -314,6 +299,7 @@ export class PanelContainer
}

view({attrs}: m.CVnode<PanelContainerAttrs>) {
this.attrs = attrs;
this.panelById.clear();
const children = attrs.panels.map((panel, index) =>
this.renderTree(panel, `${index}`),
Expand All @@ -338,12 +324,10 @@ export class PanelContainer
private readPanelRectsFromDom(dom: Element): void {
this.panelInfos = [];

const panel = dom.querySelectorAll('.pf-panel');
const panels = assertExists(findRef(dom, this.PANEL_STACK_REF));
const domRect = panels.getBoundingClientRect();
this.panelContainerTop = domRect.y;
this.panelContainerHeight = domRect.height;

dom.querySelectorAll('.pf-panel').forEach((panelElement) => {
const {top} = panels.getBoundingClientRect();
panel.forEach((panelElement) => {
const panelHTMLElement = toHTMLElement(panelElement);
const panelId = assertExists(panelHTMLElement.dataset.panelId);
const panel = assertExists(this.panelById.get(panelId));
Expand All @@ -356,12 +340,13 @@ export class PanelContainer
width: rect.width,
clientX: rect.x,
clientY: rect.y,
absY: rect.y - top,
panel,
});
});
}

private renderCanvas(attrs: PanelContainerAttrs) {
private renderCanvas() {
if (!this.ctx) return;
if (!this.virtualCanvas) return;

Expand All @@ -378,8 +363,7 @@ export class PanelContainer

this.handleAreaSelection();

const totalRenderedPanels = this.renderPanels(ctx, vc, attrs);

const totalRenderedPanels = this.renderPanels(ctx, vc);
this.drawTopLayerOnCanvas(ctx, vc);

// Collect performance as the last thing we do.
Expand All @@ -394,9 +378,8 @@ export class PanelContainer
private renderPanels(
ctx: CanvasRenderingContext2D,
vc: VirtualCanvas,
attrs: PanelContainerAttrs,
): number {
attrs.renderUnderlay?.(ctx, vc.size);
this.attrs.renderUnderlay?.(ctx, vc.size);

let panelTop = 0;
let totalOnCanvas = 0;
Expand Down Expand Up @@ -449,7 +432,7 @@ export class PanelContainer
panelTop += panelHeight;
}

attrs.renderOverlay?.(ctx, vc.size, renderedPanels);
this.attrs.renderOverlay?.(ctx, vc.size, renderedPanels);

return totalOnCanvas;
}
Expand All @@ -460,41 +443,32 @@ export class PanelContainer
ctx: CanvasRenderingContext2D,
vc: VirtualCanvas,
): void {
const {selectedYRange} = this.attrs;
const area = this.trace.timeline.selectedArea;
if (
area === undefined ||
this.trace.timeline.areaY.start === undefined ||
this.trace.timeline.areaY.end === undefined
) {
if (area === undefined || selectedYRange === undefined) {
return;
}
if (this.panelInfos.length === 0 || area.trackUris.length === 0) {
return;
}
if (this.panelInfos.length === 0 || area.trackUris.length === 0) return;

// Find the minY and maxY of the selected tracks in this panel container.
let selectedTracksMinY = this.panelContainerHeight + this.panelContainerTop;
let selectedTracksMaxY = this.panelContainerTop;
let trackFromCurrentContainerSelected = false;
let selectedTracksMinY = selectedYRange.top;
let selectedTracksMaxY = selectedYRange.bottom;
for (let i = 0; i < this.panelInfos.length; i++) {
const trackUri = this.panelInfos[i].trackNode?.uri;
if (trackUri && area.trackUris.includes(trackUri)) {
trackFromCurrentContainerSelected = true;
selectedTracksMinY = Math.min(
selectedTracksMinY,
this.panelInfos[i].clientY,
this.panelInfos[i].absY,
);
selectedTracksMaxY = Math.max(
selectedTracksMaxY,
this.panelInfos[i].clientY + this.panelInfos[i].height,
this.panelInfos[i].absY + this.panelInfos[i].height,
);
}
}

// No box should be drawn if there are no selected tracks in the current
// container.
if (!trackFromCurrentContainerSelected) {
return;
}

// TODO(stevegolton): We shouldn't know anything about visible time scale
// right now, that's a job for our parent, but we can put one together so we
// don't have to refactor this entire bit right now...
Expand All @@ -506,9 +480,6 @@ export class PanelContainer

const startX = visibleTimeScale.timeToPx(area.start);
const endX = visibleTimeScale.timeToPx(area.end);
// To align with where to draw on the canvas subtract the first panel Y.
selectedTracksMinY -= this.panelContainerTop;
selectedTracksMaxY -= this.panelContainerTop;
ctx.save();
ctx.strokeStyle = SELECTION_STROKE_COLOR;
ctx.lineWidth = 1;
Expand Down
64 changes: 59 additions & 5 deletions ui/src/frontend/viewer_page.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ import m from 'mithril';
import {removeFalsyValues} from '../base/array_utils';
import {canvasClip, canvasSave} from '../base/canvas_utils';
import {findRef, toHTMLElement} from '../base/dom_utils';
import {Size2D} from '../base/geom';
import {Size2D, VerticalBounds} from '../base/geom';
import {assertExists} from '../base/logging';
import {clamp} from '../base/math_utils';
import {Time, TimeSpan} from '../base/time';
Expand Down Expand Up @@ -82,6 +82,12 @@ function onTimeRangeBoundary(
return null;
}

interface SelectedContainer {
readonly containerClass: string;
readonly dragStartAbsY: number;
readonly dragEndAbsY: number;
}

/**
* Top-most level component for the viewer page. Holds tracks, brush timeline,
* panels, and everything else that's part of the main trace viewer page.
Expand All @@ -97,6 +103,7 @@ export class ViewerPage implements m.ClassComponent<PageWithTraceAttrs> {
private notesPanel: NotesPanel;
private tickmarkPanel: TickmarkPanel;
private timelineWidthPx?: number;
private selectedContainer?: SelectedContainer;

private readonly PAN_ZOOM_CONTENT_REF = 'pan-and-zoom-content';

Expand All @@ -113,6 +120,7 @@ export class ViewerPage implements m.ClassComponent<PageWithTraceAttrs> {
const panZoomElRaw = findRef(dom, this.PAN_ZOOM_CONTENT_REF);
const panZoomEl = toHTMLElement(assertExists(panZoomElRaw));

const {top: panTop} = panZoomEl.getBoundingClientRect();
this.zoomContent = new PanAndZoomHandler({
element: panZoomEl,
onPanned: (pannedPx: number) => {
Expand Down Expand Up @@ -215,15 +223,47 @@ export class ViewerPage implements m.ClassComponent<PageWithTraceAttrs> {
timescale.pxToHpTime(startPx).toTime('floor'),
timescale.pxToHpTime(endPx).toTime('ceil'),
);
timeline.areaY.start = dragStartY;
timeline.areaY.end = currentY;

const absStartY = dragStartY + panTop;
const absCurrentY = currentY + panTop;
if (this.selectedContainer === undefined) {
for (const c of dom.querySelectorAll('.pf-panel-container')) {
const {top, bottom} = c.getBoundingClientRect();
if (top <= absStartY && absCurrentY <= bottom) {
const stack = assertExists(c.querySelector('.pf-panel-stack'));
const stackTop = stack.getBoundingClientRect().top;
this.selectedContainer = {
containerClass: Array.from(c.classList).filter(
(x) => x !== 'pf-panel-container',
)[0],
dragStartAbsY: -stackTop + absStartY,
dragEndAbsY: -stackTop + absCurrentY,
};
break;
}
}
} else {
const c = assertExists(
dom.querySelector(`.${this.selectedContainer.containerClass}`),
);
const {top, bottom} = c.getBoundingClientRect();
const boundedCurrentY = Math.min(
Math.max(top, absCurrentY),
bottom,
);
const stack = assertExists(c.querySelector('.pf-panel-stack'));
const stackTop = stack.getBoundingClientRect().top;
this.selectedContainer = {
...this.selectedContainer,
dragEndAbsY: -stackTop + boundedCurrentY,
};
}
publishShowPanningHint();
}
raf.scheduleRedraw();
},
endSelection: (edit: boolean) => {
attrs.trace.timeline.areaY.start = undefined;
attrs.trace.timeline.areaY.end = undefined;
this.selectedContainer = undefined;
const area = attrs.trace.timeline.selectedArea;
// If we are editing we need to pass the current id through to ensure
// the marked area with that id is also updated.
Expand Down Expand Up @@ -279,6 +319,7 @@ export class ViewerPage implements m.ClassComponent<PageWithTraceAttrs> {
this.notesPanel,
this.tickmarkPanel,
]),
selectedYRange: this.getYRange('header-panel-container'),
}),
m('.scrollbar-spacer-vertical'),
),
Expand Down Expand Up @@ -310,6 +351,7 @@ export class ViewerPage implements m.ClassComponent<PageWithTraceAttrs> {
renderUnderlay: (ctx, size) => renderUnderlay(attrs.trace, ctx, size),
renderOverlay: (ctx, size, panels) =>
renderOverlay(attrs.trace, ctx, size, panels),
selectedYRange: this.getYRange('pinned-panel-container'),
}),
m(PanelContainer, {
trace: attrs.trace,
Expand All @@ -322,6 +364,7 @@ export class ViewerPage implements m.ClassComponent<PageWithTraceAttrs> {
renderUnderlay: (ctx, size) => renderUnderlay(attrs.trace, ctx, size),
renderOverlay: (ctx, size, panels) =>
renderOverlay(attrs.trace, ctx, size, panels),
selectedYRange: this.getYRange('scrolling-panel-container'),
}),
),
m(TabPanel, {
Expand All @@ -332,6 +375,17 @@ export class ViewerPage implements m.ClassComponent<PageWithTraceAttrs> {
attrs.trace.tracks.flushOldTracks();
return result;
}

private getYRange(cls: string): VerticalBounds | undefined {
if (this.selectedContainer?.containerClass !== cls) {
return undefined;
}
const {dragStartAbsY, dragEndAbsY} = this.selectedContainer;
return {
top: Math.min(dragStartAbsY, dragEndAbsY),
bottom: Math.max(dragStartAbsY, dragEndAbsY),
};
}
}

function renderUnderlay(
Expand Down

0 comments on commit 0c01c75

Please sign in to comment.