Add viewport intersection check to cull invisible GraphView edges#3619
Conversation
|
normally we require for these a recording of before and after the fix, also what is the issue that is fixing? |
Hi @wargio this resolves a todo mentioned in the codebase |
|
Hi @PremadeS could you pls let me know if there any changes needed or if this pr is good to merge |
PremadeS
left a comment
There was a problem hiding this comment.
Doesn't seem to be drawing all of the edges correctly
for example on the ls binary inside fcn.000065f0 function
Original: see the false/red and true/green "straight" arrows going to next blocks

This PR: those arrows are no longer present, even though the edges/arrows are in the visible range
Another one from a test binary of mine:
Original: see the true/green straight edge going from first block to next
This PR: It's no longer present
|
From that single addition there seems to be an ~83.45% decrease in time taken (or ~6.04 times faster) for rendering entire graph view with around 7014 edges where ~8 edges are currently visible (8 visible edges is a realistic scenario) on my machine That is assuming it draws all edges correctly 7014 edges in a graph view at once is highly unlikely in any realistic scenario, this was just to check the difference in performance if there happen to be ~7000 edges |
|
@PremadeS i reverted the changes the pr is ready for review |
PremadeS
left a comment
There was a problem hiding this comment.
Works fine, just need to update the comment :)
|
Hi @PremadeS I have updated the comment the pr is ready for review |
|
Hi @PremadeS is this pr good to merge? |
|
@andersendsa could you please rebase? sorry for not merging this sooner, was waiting for the 2.5 release |
|
Hi @PremadeS I have updated the base branch |
Your checklist for this pull request
Detailed description
Test plan (required)
Closing issues