-
Notifications
You must be signed in to change notification settings - Fork 8
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Feature: Small geometry in own sector #134
base: master
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks good to me, but as you say: hard to definitively test the impact
var smallNodes = nodes.Where(n => n.Diagonal < SplitDetailsThreshold).ToArray(); | ||
var largeNodes = nodes.Except(smallNodes).ToArray(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure if this has actual performance impact, but an inverse if check on largeNodes makes this check "O(n) instead of O(n^2) which it will be with the nodes.Except code.
var largeNodes = nodes.Where(n => n.Diagonal >= SplitDetailsThreshold).ToArray();
@@ -225,6 +209,76 @@ int depthToStartSplittingGeometry | |||
} | |||
} | |||
|
|||
private IEnumerable<InternalSector> HandleLastNodes( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Comment in code what this is intended to solve.
Something like:
This method is intended to avoid the problem that we allways fill leaf sectors to the brim with content. This means that we can have a sector with both large and tiny parts. If this is the case we some times want to avoid loading all the tiny parts until we are closer to the sector. blabalbablabla insert better text
} | ||
} | ||
|
||
private InternalSector CreateSector( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
Waiting for Spike #142 , as this (134) change will introduce more batching load and 142 should improve that especially for smaller sectors |
At the last splitting level, check if there are many small nodes and split the small ones to they're own sector.
Maybe split when it has 1000 nodes that are smaller than 0.1 meter?
Not sure how to test it properly.
Some stats from testing:
Huldra (started with 161 sectors):
Split smaller than 0.5: 196
Split smaller than 0.2: 224
Split smaller than 0.1: 226
0.1 with minimum 100 small nodes 217
0.1 with minimum 200: 215
0.1 with minimum 500: 204
0.1 with minimum 1000: 192
0.1 with minimum 2000: 168
0.1 with minimum 5000: 162
0.1 with minimum 10000: 161
TrollA (started with 1499):
Split smaller than 0.5: 2315
Split smaller than 0.2: 2314
Split smaller than 0.1: 2315
Split smaller than 0.05: 2311
Split smaller than 0.005: 2026
0.1 with minimum 1000: 1718