-
Notifications
You must be signed in to change notification settings - Fork 30
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Refactor/2318/migrate pre render service 2 (#2980)
- fix loading indicator isn't working #2977 - ref Migrate all Angular Components and Services #2318 (migrate codeMap.preRender.service) - explicit state actions on which to render - throttle rendering (reducing total initial calls to render of default dev map from 58 to 1) a very rough performance analysis through chrome's performance profile shows no real difference (uploading a 1.704kb and a 28.596kb file) - delete code for re-selecting building after map change as this didn't work and wasn't missed
- Loading branch information
1 parent
981a324
commit 3ee57c9
Showing
26 changed files
with
435 additions
and
717 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
73 changes: 73 additions & 0 deletions
73
...ts/autoFitCodeMapOnFileSelectionChange/autoFitCodeMapOnFileSelectionChange.effect.spec.ts
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,73 @@ | ||
| import { ApplicationInitStatus } from "@angular/core" | ||
| import { TestBed } from "@angular/core/testing" | ||
| import { Subject } from "rxjs" | ||
| import { mocked } from "ts-jest/utils" | ||
| import { ThreeOrbitControlsService } from "../../../ui/codeMap/threeViewer/threeOrbitControlsService" | ||
| import { EffectsModule } from "../../angular-redux/effects/effects.module" | ||
| import { Store } from "../../angular-redux/store" | ||
| import { visibleFileStatesSelector } from "../../selectors/visibleFileStates.selector" | ||
| import { resetCameraIfNewFileIsLoadedSelector } from "../../store/appSettings/resetCameraIfNewFileIsLoaded/resetCameraIfNewFileIsLoaded.selector" | ||
| import { RenderCodeMapEffect } from "../renderCodeMapEffect/renderCodeMap.effect" | ||
| import { AutoFitCodeMapOnFileSelectionChangeEffect } from "./autoFitCodeMapOnFileSelectionChange.effect" | ||
|
|
||
| jest.mock("../../store/appSettings/resetCameraIfNewFileIsLoaded/resetCameraIfNewFileIsLoaded.selector", () => ({ | ||
| resetCameraIfNewFileIsLoadedSelector: jest.fn() | ||
| })) | ||
| const mockedResetCameraIfNewFileIsLoadedSelector = mocked(resetCameraIfNewFileIsLoadedSelector) | ||
|
|
||
| describe("autoFitCodeMapOnFileSelectionChangeEffect", () => { | ||
| const mockedRenderCodeMap$ = new Subject() | ||
| const mockedVisibleFileStates$ = new Subject() | ||
| const mockedStore = { | ||
| select: (selector: unknown) => { | ||
| switch (selector) { | ||
| case visibleFileStatesSelector: | ||
| return mockedVisibleFileStates$ | ||
| default: | ||
| throw new Error("selector is not mocked") | ||
| } | ||
| }, | ||
| dispatch: jest.fn() | ||
| } | ||
|
|
||
| beforeEach(async () => { | ||
| ThreeOrbitControlsService.instance = { autoFitTo: jest.fn() } as unknown as ThreeOrbitControlsService | ||
|
|
||
| TestBed.configureTestingModule({ | ||
| imports: [EffectsModule.forRoot([AutoFitCodeMapOnFileSelectionChangeEffect])], | ||
| providers: [ | ||
| { provide: RenderCodeMapEffect, useValue: { renderCodeMap$: mockedRenderCodeMap$ } }, | ||
| { provide: Store, useValue: mockedStore } | ||
| ] | ||
| }) | ||
| await TestBed.inject(ApplicationInitStatus).donePromise | ||
| }) | ||
|
|
||
| it("should auto fit map after first render after selected files changed", () => { | ||
| mockedResetCameraIfNewFileIsLoadedSelector.mockImplementation(() => true) | ||
| mockedVisibleFileStates$.next("") | ||
| mockedVisibleFileStates$.next("") | ||
| expect(ThreeOrbitControlsService.instance.autoFitTo).not.toHaveBeenCalled() | ||
|
|
||
| mockedRenderCodeMap$.next("") | ||
| expect(ThreeOrbitControlsService.instance.autoFitTo).toHaveBeenCalledTimes(1) | ||
|
|
||
| mockedRenderCodeMap$.next("") | ||
| expect(ThreeOrbitControlsService.instance.autoFitTo).toHaveBeenCalledTimes(1) | ||
| }) | ||
|
|
||
| it("should skip first change", () => { | ||
| mockedResetCameraIfNewFileIsLoadedSelector.mockImplementation(() => true) | ||
| mockedVisibleFileStates$.next("") | ||
| mockedRenderCodeMap$.next("") | ||
| expect(ThreeOrbitControlsService.instance.autoFitTo).toHaveBeenCalledTimes(1) | ||
| }) | ||
|
|
||
| it("should do nothing when reset camera if new file is loaded is deactivated", () => { | ||
| mockedResetCameraIfNewFileIsLoadedSelector.mockImplementation(() => false) | ||
| mockedVisibleFileStates$.next("") | ||
| mockedVisibleFileStates$.next("") | ||
| mockedRenderCodeMap$.next("") | ||
| expect(ThreeOrbitControlsService.instance.autoFitTo).not.toHaveBeenCalled() | ||
| }) | ||
| }) |
32 changes: 32 additions & 0 deletions
32
...effects/autoFitCodeMapOnFileSelectionChange/autoFitCodeMapOnFileSelectionChange.effect.ts
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,32 @@ | ||
| import { Inject, Injectable } from "@angular/core" | ||
| import { concatMap, filter, skip, take, tap } from "rxjs" | ||
| import { ThreeOrbitControlsService } from "../../../ui/codeMap/threeViewer/threeOrbitControlsService" | ||
| import { createEffect } from "../../angular-redux/effects/createEffect" | ||
| import { State } from "../../angular-redux/state" | ||
| import { Store } from "../../angular-redux/store" | ||
| import { visibleFileStatesSelector } from "../../selectors/visibleFileStates.selector" | ||
| import { resetCameraIfNewFileIsLoadedSelector } from "../../store/appSettings/resetCameraIfNewFileIsLoaded/resetCameraIfNewFileIsLoaded.selector" | ||
| import { RenderCodeMapEffect } from "../renderCodeMapEffect/renderCodeMap.effect" | ||
|
|
||
| // don't inject AngularJS services, as AngularJS is not yet bootstrapped when Effects are bootstrapped | ||
| @Injectable() | ||
| export class AutoFitCodeMapOnFileSelectionChangeEffect { | ||
| constructor( | ||
| @Inject(Store) private store: Store, | ||
| @Inject(State) private state: State, | ||
| @Inject(RenderCodeMapEffect) private renderCodeMapEffect: RenderCodeMapEffect | ||
| ) {} | ||
|
|
||
| autoFitCodeMapOnFileSelectionChange$ = createEffect( | ||
| () => | ||
| this.store.select(visibleFileStatesSelector).pipe( | ||
| skip(1), // initial map load is already fitted | ||
| filter(() => resetCameraIfNewFileIsLoadedSelector(this.state.getValue())), | ||
| concatMap(() => this.renderCodeMapEffect.renderCodeMap$.pipe(take(1))), | ||
| tap(() => { | ||
| ThreeOrbitControlsService.instance?.autoFitTo() | ||
| }) | ||
| ), | ||
| { dispatch: false } | ||
| ) | ||
| } |
53 changes: 53 additions & 0 deletions
53
visualization/app/codeCharta/state/effects/renderCodeMapEffect/actionsRequiringRerender.ts
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,53 @@ | ||
| import { AmountOfEdgePreviewsActions } from "../../store/appSettings/amountOfEdgePreviews/amountOfEdgePreviews.actions" | ||
| import { AmountOfTopLabelsActions } from "../../store/appSettings/amountOfTopLabels/amountOfTopLabels.actions" | ||
| import { ColorLabelsActions } from "../../store/appSettings/colorLabels/colorLabels.actions" | ||
| import { EdgeHeightActions } from "../../store/appSettings/edgeHeight/edgeHeight.actions" | ||
| import { HideFlatBuildingsActions } from "../../store/appSettings/hideFlatBuildings/hideFlatBuildings.actions" | ||
| import { InvertAreaActions } from "../../store/appSettings/invertArea/invertArea.actions" | ||
| import { InvertHeightActions } from "../../store/appSettings/invertHeight/invertHeight.actions" | ||
| import { IsWhiteBackgroundActions } from "../../store/appSettings/isWhiteBackground/isWhiteBackground.actions" | ||
| import { LayoutAlgorithmActions } from "../../store/appSettings/layoutAlgorithm/layoutAlgorithm.actions" | ||
| import { MapColorsActions } from "../../store/appSettings/mapColors/mapColors.actions" | ||
| import { MaxTreeMapFilesActions } from "../../store/appSettings/maxTreeMapFiles/maxTreeMapFiles.actions" | ||
| import { ScalingActions } from "../../store/appSettings/scaling/scaling.actions" | ||
| import { SharpnessModeActions } from "../../store/appSettings/sharpnessMode/sharpnessMode.actions" | ||
| import { ShowMetricLabelNameValueActions } from "../../store/appSettings/showMetricLabelNameValue/showMetricLabelNameValue.actions" | ||
| import { ShowMetricLabelNodeNameActions } from "../../store/appSettings/showMetricLabelNodeName/showMetricLabelNodeName.actions" | ||
| import { ShowOnlyBuildingsWithEdgesActions } from "../../store/appSettings/showOnlyBuildingsWithEdges/showOnlyBuildingsWithEdges.actions" | ||
| import { AreaMetricActions } from "../../store/dynamicSettings/areaMetric/areaMetric.actions" | ||
| import { ColorMetricActions } from "../../store/dynamicSettings/colorMetric/colorMetric.actions" | ||
| import { ColorModeActions } from "../../store/dynamicSettings/colorMode/colorMode.actions" | ||
| import { ColorRangeActions } from "../../store/dynamicSettings/colorRange/colorRange.actions" | ||
| import { EdgeMetricActions } from "../../store/dynamicSettings/edgeMetric/edgeMetric.actions" | ||
| import { FocusedNodePathActions } from "../../store/dynamicSettings/focusedNodePath/focusedNodePath.actions" | ||
| import { HeightMetricActions } from "../../store/dynamicSettings/heightMetric/heightMetric.actions" | ||
| import { MarginActions } from "../../store/dynamicSettings/margin/margin.actions" | ||
| import { SearchPatternActions } from "../../store/dynamicSettings/searchPattern/searchPattern.actions" | ||
|
|
||
| export const actionsRequiringRerender = [ | ||
| ColorLabelsActions, | ||
| MapColorsActions, | ||
| ShowMetricLabelNodeNameActions, | ||
| ShowMetricLabelNameValueActions, | ||
| IsWhiteBackgroundActions, | ||
| InvertAreaActions, | ||
| InvertHeightActions, | ||
| HideFlatBuildingsActions, | ||
| ScalingActions, | ||
| EdgeHeightActions, | ||
| AmountOfEdgePreviewsActions, | ||
| AmountOfTopLabelsActions, | ||
| LayoutAlgorithmActions, | ||
| MaxTreeMapFilesActions, | ||
| SharpnessModeActions, | ||
| ColorModeActions, | ||
| EdgeMetricActions, | ||
| ColorRangeActions, | ||
| MarginActions, | ||
| SearchPatternActions, | ||
| FocusedNodePathActions, | ||
| HeightMetricActions, | ||
| AreaMetricActions, | ||
| ColorMetricActions, | ||
| ShowOnlyBuildingsWithEdgesActions | ||
| ] |
86 changes: 86 additions & 0 deletions
86
visualization/app/codeCharta/state/effects/renderCodeMapEffect/renderCodeMap.effect.spec.ts
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,86 @@ | ||
| import { ApplicationInitStatus } from "@angular/core" | ||
| import { TestBed } from "@angular/core/testing" | ||
| import { Action } from "redux" | ||
| import { BehaviorSubject, Subject } from "rxjs" | ||
| import { Vector3 } from "three" | ||
| import { CodeMapRenderService } from "../../../ui/codeMap/codeMap.render.service" | ||
| import { ThreeRendererService } from "../../../ui/codeMap/threeViewer/threeRendererService" | ||
| import { UploadFilesService } from "../../../ui/toolBar/uploadFilesButton/uploadFiles.service" | ||
| import { wait } from "../../../util/testUtils/wait" | ||
| import { EffectsModule } from "../../angular-redux/effects/effects.module" | ||
| import { Store } from "../../angular-redux/store" | ||
| import { accumulatedDataSelector } from "../../selectors/accumulatedData/accumulatedData.selector" | ||
| import { setInvertArea } from "../../store/appSettings/invertArea/invertArea.actions" | ||
| import { setIsLoadingFile } from "../../store/appSettings/isLoadingFile/isLoadingFile.actions" | ||
| import { setIsLoadingMap } from "../../store/appSettings/isLoadingMap/isLoadingMap.actions" | ||
| import { setScaling } from "../../store/appSettings/scaling/scaling.actions" | ||
| import { maxFPS, RenderCodeMapEffect } from "./renderCodeMap.effect" | ||
|
|
||
| describe("renderCodeMapEffect", () => { | ||
| let mockedStore | ||
|
|
||
| beforeEach(async () => { | ||
| mockedStore = { | ||
| select: (selector: unknown) => { | ||
| switch (selector) { | ||
| case accumulatedDataSelector: | ||
| return new BehaviorSubject({ unifiedMapNode: {} }) | ||
| default: | ||
| throw new Error("selector is not mocked") | ||
| } | ||
| }, | ||
| dispatch: jest.fn() | ||
| } | ||
|
|
||
| CodeMapRenderService.instance = { render: jest.fn(), scaleMap: jest.fn() } as unknown as CodeMapRenderService | ||
| ThreeRendererService.instance = { render: jest.fn() } as unknown as ThreeRendererService | ||
| EffectsModule.actions$ = new Subject<Action>() | ||
|
|
||
| TestBed.configureTestingModule({ | ||
| imports: [EffectsModule.forRoot([RenderCodeMapEffect])], | ||
| providers: [ | ||
| { provide: Store, useValue: mockedStore }, | ||
| { provide: UploadFilesService, useValue: { isUploading: false } } | ||
| ] | ||
| }) | ||
| await TestBed.inject(ApplicationInitStatus).donePromise | ||
| }) | ||
|
|
||
| afterEach(() => { | ||
| EffectsModule.actions$.complete() | ||
| }) | ||
|
|
||
| it("should render once throttled after actions requiring rerender, but not scale map", async () => { | ||
| EffectsModule.actions$.next(setInvertArea(true)) | ||
| EffectsModule.actions$.next(setInvertArea(true)) | ||
| expect(CodeMapRenderService.instance.render).toHaveBeenCalledTimes(0) | ||
| expect(ThreeRendererService.instance.render).toHaveBeenCalledTimes(0) | ||
|
|
||
| await wait(maxFPS) | ||
| expect(CodeMapRenderService.instance.render).toHaveBeenCalledTimes(1) | ||
| expect(ThreeRendererService.instance.render).toHaveBeenCalledTimes(1) | ||
| expect(CodeMapRenderService.instance.scaleMap).not.toHaveBeenCalled() | ||
| }) | ||
|
|
||
| it("should scale map when scaling changes", async () => { | ||
| EffectsModule.actions$.next(setScaling(new Vector3(1, 1, 1))) | ||
| await wait(maxFPS) | ||
| expect(CodeMapRenderService.instance.scaleMap).toHaveBeenCalledTimes(1) | ||
| }) | ||
|
|
||
| it("should remove loading indicators after render", async () => { | ||
| EffectsModule.actions$.next(setInvertArea(true)) | ||
| await wait(maxFPS) | ||
| expect(mockedStore.dispatch).toHaveBeenCalledWith(setIsLoadingFile(false)) | ||
| expect(mockedStore.dispatch).toHaveBeenCalledWith(setIsLoadingMap(false)) | ||
| }) | ||
|
|
||
| it("should not remove loading indicators after render when a file is still being uploaded", async () => { | ||
| const uploadFileService = TestBed.inject(UploadFilesService) | ||
| uploadFileService.isUploading = true | ||
| EffectsModule.actions$.next(setInvertArea(true)) | ||
| await wait(maxFPS) | ||
| expect(mockedStore.dispatch).not.toHaveBeenCalledWith(setIsLoadingFile(false)) | ||
| expect(mockedStore.dispatch).not.toHaveBeenCalledWith(setIsLoadingMap(false)) | ||
| }) | ||
| }) |
Oops, something went wrong.