Feature/1728/on demand rendering #1806
Conversation
|
Seems like the prettier check does not check the exit code properly. Some files have the different line ending again :) |
There was a problem hiding this comment.
Mainly LGTM. It does feel like we could easily miss something though and we have to be very careful to really rerender everything properly. Thus, I would like to have others closely review this, especially by trying it out in all kinds of situations.
Going forward is also more difficult with this solution as we have to pay attention to call update manually. It would be good to rework the internals in a way that this is all done for us automatically.
visualization/app/codeCharta/ui/codeMap/threeViewer/threeStatsService.ts
Outdated
Show resolved
Hide resolved
visualization/app/codeCharta/ui/codeMap/threeViewer/threeStatsService.ts
Show resolved
Hide resolved
visualization/app/codeCharta/ui/codeMap/threeViewer/threeStatsService.ts
Outdated
Show resolved
Hide resolved
visualization/app/codeCharta/ui/codeMap/threeViewer/threeStatsService.spec.ts
Outdated
Show resolved
Hide resolved
visualization/app/codeCharta/ui/codeMap/threeViewer/threeStatsService.spec.ts
Outdated
Show resolved
Hide resolved
| @@ -99,6 +99,8 @@ export class CodeMapPreRenderService | |||
| ) { | |||
| this.debounceRendering() | |||
| this.debounceTracking(actionType) | |||
| } else { | |||
| this.codeMapRenderService.update() | |||
There was a problem hiding this comment.
Should this update really be called here? I would expect it to be triggered identical to this.debounceRendering() from the debounce time and never cause any update otherwise?
There was a problem hiding this comment.
I think debouncerendering does set a new map mesh etc, while this.codeMapRenderService.update() is only re drawing the frame.
There was a problem hiding this comment.
But do we need to redraw anything in case there's no update to the map? Why would we trigger the function otherwise?
visualization/app/codeCharta/ui/codeMap/codeMap.component.spec.ts
Outdated
Show resolved
Hide resolved
|
I tested it locally and it works :) |
| const sharpnessTests = [ | ||
| { func: "stopAnimate" }, | ||
| { func: "destroy" }, | ||
| { func: "autoFitTo" }, | ||
| { func: "animate" }, | ||
| { func: "animateStats" } | ||
| ] | ||
| beforeEach(() => { | ||
| codeMapController.onSharpnessModeChanged() | ||
| }) | ||
|
|
||
| for (const test of sharpnessTests) { | ||
| it(`should call threeViewerService ${test.func}`, () => { | ||
| expect(threeViewerService[`${test.func}`]).toHaveBeenCalled() | ||
| }) | ||
| } |
There was a problem hiding this comment.
I personally would have used a single it and used a for loop inside that, but that's just my personal style and not blocking.
|
[CodeCharta Analysis] Kudos, SonarCloud Quality Gate passed!
|
|
[CodeCharta Visualization] Kudos, SonarCloud Quality Gate passed! |
ondemand rendering
Issue: #1728
Description
Descriptive pull request text, answering:
Screenshots or gifs
without gpu on demand
with gpu on demand
