Improve initial display selection and naming.
Displays without a known name property should be labelled "Unknown Display".
The initial display selected should default to one with non-display rects, if possible.
Fixes: 327325638
Test: npm run test:presubmit:quiet
Change-Id: I135dd48ddb3d40e266f27d7692db4f3faab14f9c
diff --git a/tools/winscope/src/parsers/surface_flinger/computations/rects_computation.ts b/tools/winscope/src/parsers/surface_flinger/computations/rects_computation.ts
index 94b0e76..762cefc 100644
--- a/tools/winscope/src/parsers/surface_flinger/computations/rects_computation.ts
+++ b/tools/winscope/src/parsers/surface_flinger/computations/rects_computation.ts
@@ -30,7 +30,8 @@
const layerStack = assertDefined(
display.getChildByName('layerStack'),
).getValue();
- let displayName = display.getChildByName('name')?.getValue() ?? '';
+ let displayName =
+ display.getChildByName('name')?.getValue() ?? 'Unknown Display';
const id = assertDefined(display.getChildByName('id')).getValue();
const existingNameCount = nameCounts.get(displayName);
diff --git a/tools/winscope/src/parsers/surface_flinger/computations/rects_computation_test.ts b/tools/winscope/src/parsers/surface_flinger/computations/rects_computation_test.ts
index 54c23a8..6528aab 100644
--- a/tools/winscope/src/parsers/surface_flinger/computations/rects_computation_test.ts
+++ b/tools/winscope/src/parsers/surface_flinger/computations/rects_computation_test.ts
@@ -268,6 +268,44 @@
expect(hierarchyRoot.getRects()).toEqual(expectedDisplayRects);
});
+ it('makes display rects with unknown name', () => {
+ const hierarchyRoot = new HierarchyTreeBuilder()
+ .setId('LayerTraceEntry')
+ .setName('root')
+ .setProperties({
+ displays: [
+ {
+ id: 1,
+ layerStack: 0,
+ size: {w: 5, h: 5},
+ transform: Transform.EMPTY,
+ },
+ ],
+ })
+ .build();
+
+ const expectedDisplayRects = [
+ new TraceRectBuilder()
+ .setX(0)
+ .setY(0)
+ .setWidth(5)
+ .setHeight(5)
+ .setId('Display - 1')
+ .setName('Unknown Display')
+ .setCornerRadius(0)
+ .setTransform(Transform.EMPTY.matrix)
+ .setDepth(0)
+ .setGroupId(0)
+ .setIsVisible(false)
+ .setIsDisplay(true)
+ .setIsVirtual(false)
+ .build(),
+ ];
+
+ computation.setRoot(hierarchyRoot).executeInPlace();
+ expect(hierarchyRoot.getRects()).toEqual(expectedDisplayRects);
+ });
+
it('handles z-order paths with different lengths', () => {
const hierarchyRoot = new HierarchyTreeBuilder()
.setId('LayerTraceEntry')
diff --git a/tools/winscope/src/viewers/components/rects/rects_component.ts b/tools/winscope/src/viewers/components/rects/rects_component.ts
index 538cd3d..bb04d73 100644
--- a/tools/winscope/src/viewers/components/rects/rects_component.ts
+++ b/tools/winscope/src/viewers/components/rects/rects_component.ts
@@ -21,6 +21,7 @@
Input,
OnDestroy,
OnInit,
+ SimpleChange,
SimpleChanges,
} from '@angular/core';
import {assertDefined} from 'common/assert_utils';
@@ -290,7 +291,10 @@
this.updateControlsFromStore();
}
- this.currentDisplay = this.internalDisplays[0] ?? undefined;
+ this.currentDisplay =
+ this.internalDisplays.length > 0
+ ? this.getFirstDisplayWithRectsOrFirstDisplay(this.internalDisplays)
+ : undefined;
this.mapper3d.setCurrentGroupId(this.currentDisplay?.groupId ?? 0);
this.mapper3d.increaseZoomFactor(this.zoomFactor - 1);
this.drawLargeRectsAndLabels();
@@ -320,7 +324,7 @@
}
}
if (simpleChanges['displays']) {
- this.onDisplaysChange(simpleChanges['displays'].currentValue);
+ this.onDisplaysChange(simpleChanges['displays']);
if (this.isStackBased) {
this.internalGroupIds = new Set(
this.internalDisplays.map((display) => display.groupId),
@@ -337,13 +341,21 @@
this.resizeObserver?.disconnect();
}
- onDisplaysChange(displays: DisplayIdentifier[]) {
+ onDisplaysChange(change: SimpleChange) {
+ const displays = change.currentValue;
this.internalDisplays = displays;
if (displays.length === 0) {
return;
}
+ if (change.firstChange) {
+ this.updateCurrentDisplay(
+ this.getFirstDisplayWithRectsOrFirstDisplay(this.internalDisplays),
+ );
+ return;
+ }
+
if (!this.stackSelected) {
const curr = this.internalDisplays.find(
(display) => display.displayId === this.currentDisplay?.displayId,
@@ -354,11 +366,13 @@
}
}
- const firstDisplayWithCurrentGroupId = this.internalDisplays.find(
+ const displaysWithCurrentGroupId = this.internalDisplays.filter(
(display) => display.groupId === this.mapper3d.getCurrentGroupId(),
);
- if (!firstDisplayWithCurrentGroupId) {
- this.updateCurrentDisplay(this.internalDisplays[0]);
+ if (displaysWithCurrentGroupId.length === 0) {
+ this.updateCurrentDisplay(
+ this.getFirstDisplayWithRectsOrFirstDisplay(this.internalDisplays),
+ );
return;
}
@@ -366,7 +380,9 @@
(display) => display.displayId === this.currentDisplay?.displayId,
);
if (!displayWithCurrentDisplayId) {
- this.updateCurrentDisplay(firstDisplayWithCurrentGroupId);
+ this.updateCurrentDisplay(
+ this.getFirstDisplayWithRectsOrFirstDisplay(displaysWithCurrentGroupId),
+ );
return;
}
@@ -467,7 +483,9 @@
displaysWithGroupId.length > 0 &&
!displaysWithGroupId.includes(this.currentDisplay)
) {
- this.updateCurrentDisplay(displaysWithGroupId[0]);
+ this.updateCurrentDisplay(
+ this.getFirstDisplayWithRectsOrFirstDisplay(displaysWithGroupId),
+ );
}
}
@@ -543,6 +561,18 @@
return this.currentDisplay.groupId === groupId ? 'primary' : 'secondary';
}
+ private getFirstDisplayWithRectsOrFirstDisplay(
+ displays: DisplayIdentifier[],
+ ): DisplayIdentifier {
+ return (
+ displays.find((display) =>
+ this.internalRects.some(
+ (rect) => !rect.isDisplay && rect.groupId === display.groupId,
+ ),
+ ) ?? assertDefined(displays.at(0))
+ );
+ }
+
private updateCurrentDisplay(display: DisplayIdentifier) {
this.currentDisplay = display;
this.mapper3d.setCurrentGroupId(display.groupId);
diff --git a/tools/winscope/src/viewers/components/rects/rects_component_test.ts b/tools/winscope/src/viewers/components/rects/rects_component_test.ts
index fe4cecf..21f04d9 100644
--- a/tools/winscope/src/viewers/components/rects/rects_component_test.ts
+++ b/tools/winscope/src/viewers/components/rects/rects_component_test.ts
@@ -55,7 +55,6 @@
fixture = TestBed.createComponent(TestHostComponent);
component = fixture.componentInstance;
htmlElement = fixture.nativeElement;
- fixture.detectChanges();
});
it('can be created', () => {
@@ -78,31 +77,10 @@
});
it('draws scene when input data changes', async () => {
+ fixture.detectChanges();
spyOn(Canvas.prototype, 'draw').and.callThrough();
- const inputRect = new UiRectBuilder()
- .setX(0)
- .setY(0)
- .setWidth(1)
- .setHeight(1)
- .setLabel('rectangle1')
- .setTransform({
- dsdx: 1,
- dsdy: 0,
- dtdx: 0,
- dtdy: 1,
- tx: 0,
- ty: 0,
- })
- .setIsVisible(true)
- .setIsDisplay(false)
- .setId('test-id-1234')
- .setGroupId(0)
- .setIsVirtual(false)
- .setIsClickable(false)
- .setCornerRadius(0)
- .setDepth(0)
- .build();
+ const inputRect = makeRectWithGroupId(0);
expect(Canvas.prototype.draw).toHaveBeenCalledTimes(0);
component.rects = [inputRect];
@@ -114,6 +92,7 @@
});
it('draws scene when rotation slider changes', () => {
+ fixture.detectChanges();
spyOn(Canvas.prototype, 'draw').and.callThrough();
const slider = assertDefined(htmlElement.querySelector('.slider-rotation'));
@@ -124,6 +103,7 @@
});
it('draws scene when spacing slider changes', () => {
+ fixture.detectChanges();
spyOn(Canvas.prototype, 'draw').and.callThrough();
const slider = assertDefined(htmlElement.querySelector('.slider-spacing'));
@@ -134,6 +114,7 @@
});
it('unfocuses spacing slider on click', () => {
+ fixture.detectChanges();
const spacingSlider = assertDefined(
htmlElement.querySelector('.slider-spacing'),
);
@@ -141,6 +122,7 @@
});
it('unfocuses rotation slider on click', () => {
+ fixture.detectChanges();
const rotationSlider = assertDefined(
htmlElement.querySelector('.slider-rotation'),
);
@@ -406,21 +388,56 @@
});
it('uses stored rects view settings', () => {
- expect(component.rectsComponent.getZSpacingFactor()).toEqual(1);
- component.rectsComponent.onSeparationSliderChange(0.06);
fixture.detectChanges();
- expect(component.rectsComponent.getZSpacingFactor()).toEqual(0.06);
+ const rectsComponent = assertDefined(component.rectsComponent);
+ expect(rectsComponent.getZSpacingFactor()).toEqual(1);
+ rectsComponent.onSeparationSliderChange(0.06);
+ fixture.detectChanges();
+ expect(rectsComponent.getZSpacingFactor()).toEqual(0.06);
- expect(component.rectsComponent.getShowOnlyVisibleMode()).toBeFalse();
+ expect(rectsComponent.getShowOnlyVisibleMode()).toBeFalse();
findAndClickCheckbox('.top-view-controls .show-only-visible input');
- expect(component.rectsComponent.getShowOnlyVisibleMode()).toBeTrue();
+ expect(rectsComponent.getShowOnlyVisibleMode()).toBeTrue();
const newFixture = TestBed.createComponent(TestHostComponent);
- const newComponent = newFixture.componentInstance;
newFixture.detectChanges();
+ const newRectsComponent = assertDefined(
+ newFixture.componentInstance.rectsComponent,
+ );
+ expect(newRectsComponent.getZSpacingFactor()).toEqual(0.06);
+ expect(newRectsComponent.getShowOnlyVisibleMode()).toBeTrue();
+ });
- expect(newComponent.rectsComponent.getZSpacingFactor()).toEqual(0.06);
- expect(newComponent.rectsComponent.getShowOnlyVisibleMode()).toBeTrue();
+ it('defaults initial selection to first display with non-display rects and groupId 0', () => {
+ const inputRect = makeRectWithGroupId(0);
+ component.rects = [inputRect];
+ component.displays = [
+ {displayId: 10, groupId: 1, name: 'Display 0'},
+ {displayId: 20, groupId: 0, name: 'Display 1'},
+ ];
+ fixture.detectChanges();
+
+ checkButtons(
+ ['Display 0', 'Display 1'],
+ ['secondary', 'primary'],
+ '.display-name-buttons',
+ );
+ });
+
+ it('defaults initial selection to first display with non-display rects and groupId non-zero', () => {
+ const inputRect = makeRectWithGroupId(1);
+ component.rects = [inputRect];
+ component.displays = [
+ {displayId: 10, groupId: 0, name: 'Display 0'},
+ {displayId: 20, groupId: 1, name: 'Display 1'},
+ ];
+ fixture.detectChanges();
+
+ checkButtons(
+ ['Display 0', 'Display 1'],
+ ['secondary', 'primary'],
+ '.display-name-buttons',
+ );
});
function checkButtons(
@@ -461,14 +478,41 @@
}
function checkSliderUnfocusesOnClick(slider: Element, expectedValue: number) {
+ const rectsComponent = assertDefined(component.rectsComponent);
slider.dispatchEvent(new MouseEvent('mousedown'));
- expect(component.rectsComponent.getZSpacingFactor()).toEqual(expectedValue);
+ expect(rectsComponent.getZSpacingFactor()).toEqual(expectedValue);
htmlElement.dispatchEvent(
new KeyboardEvent('keydown', {key: 'ArrowRight'}),
);
- expect(component.rectsComponent.getZSpacingFactor()).toEqual(expectedValue);
+ expect(rectsComponent.getZSpacingFactor()).toEqual(expectedValue);
htmlElement.dispatchEvent(new KeyboardEvent('keydown', {key: 'ArrowLeft'}));
- expect(component.rectsComponent.getZSpacingFactor()).toEqual(expectedValue);
+ expect(rectsComponent.getZSpacingFactor()).toEqual(expectedValue);
+ }
+
+ function makeRectWithGroupId(groupId: number): UiRect {
+ return new UiRectBuilder()
+ .setX(0)
+ .setY(0)
+ .setWidth(1)
+ .setHeight(1)
+ .setLabel('rectangle1')
+ .setTransform({
+ dsdx: 1,
+ dsdy: 0,
+ dtdx: 0,
+ dtdy: 1,
+ tx: 0,
+ ty: 0,
+ })
+ .setIsVisible(true)
+ .setIsDisplay(false)
+ .setId('test-id-1234')
+ .setGroupId(groupId)
+ .setIsVirtual(false)
+ .setIsClickable(false)
+ .setCornerRadius(0)
+ .setDepth(0)
+ .build();
}
@Component({
@@ -489,6 +533,6 @@
isStackBased = false;
@ViewChild(RectsComponent)
- rectsComponent!: RectsComponent;
+ rectsComponent: RectsComponent | undefined;
}
});
