Performance Optimizations and Viewport Feature #135
Conversation
LeandroTreu
commented
Mar 22, 2024
LeandroTreu
commented
- Various rendering and mousehandler optimizations that increase FPS and decrease CPU utilization
- Implements feature request that binds viewport to graph bounds (no out of bounds panning)
src/renderer/renderer.ts
Outdated
| this.draw_async(); | ||
| // console.log(SDFGRenderer.rendered_elements_count) |
There was a problem hiding this comment.
| // console.log(SDFGRenderer.rendered_elements_count) |
src/renderer/renderer_elements.ts
Outdated
| continue; | ||
| } | ||
|
|
||
| block.draw(renderer, ctx, mousePos); | ||
| block.debug_draw(renderer, ctx); | ||
| // SDFGRenderer.rendered_elements_count++; |
There was a problem hiding this comment.
| // SDFGRenderer.rendered_elements_count++; |
There was a problem hiding this comment.
Please remove the commented out versions elsewhere too
src/renderer/renderer.ts
Outdated
| @@ -150,6 +150,7 @@ export class SDFGRenderer extends EventEmitter { | |||
| protected bgcolor: string | null = null; | |||
| protected visible_rect: SimpleRect | null = null; | |||
| protected static cssProps: { [key: string]: string } = {}; | |||
| public static rendered_elements_count: number = 0; | |||
There was a problem hiding this comment.
Where's this used, or what for?
There was a problem hiding this comment.
Debug helper variable that I forgot to remove again! I counted the amount of elements drawn to screen to see if there is correlation between that and low FPS. There was no correlation and I had to search for more specific causes (already fixed with later commits).
src/renderer/renderer_elements.ts
Outdated
| @@ -2568,6 +2740,7 @@ export function drawSDFG( | |||
| const ppp = cManager.points_per_pixel(); | |||
| const visibleRect = renderer.get_visible_rect() ?? undefined; | |||
|
|
|||
| SDFGRenderer.rendered_elements_count = 0; | |||
There was a problem hiding this comment.
Is this a vestigial too or does this still serve a function?
There was a problem hiding this comment.
No function other than debug. I will remove it again.
|
Two questions:
Nitpicky comment: please spell check your comments in the code. I saw “timout” and “its” (that should be it’s”) |
I would be OK with leaving it in if we add a warning behind it or something similar. The issue is currently that if it's enabled, you are able to navigate huge graphs without any problems, but the moment you turn it on you can crash your browser if it's not equipped to handle the immediate resource demand. That's what we would like to avoid. |
|
Yes exactly, as @phschaad said, turning the setting off will turn off all optimisations and will make navigation impossible on huge graphs. As I only removed the user setting in the settings menu, the boolean for this switch is still present and exporting with "Save SDFG as PDF" still works as before. BUT this feature is also currently broken (also on current master branch) for huge graphs, at least the ones I'm using for benchmarking, and I will take a look at why in the coming days. Before my next pull request I will make sure that all image exporting features work as intended as before. |
@tbennun I checked my changes again and I cannot find the code where I changed something that has to do with what you described. Can you maybe let me know which commit/code parts you are referring to? I can't recall that I have removed any code that has to do with edge bounds checking. |
…veStateHiding setting again with a warning
