Skip to content

Feature: Small geometry in own sector #134

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.

Sign up for GitHub

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

Open
vegasten wants to merge 6 commits into
base: master
Choose a base branch
from
Open

Feature: Small geometry in own sector #134

Changes from 4 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
9ac8d07
Tmp
vegasten May 2, 2023
9bd7c95
Refactor
vegasten May 2, 2023
f71d328
Don't split if everything has diagonal equal to zero
vegasten May 2, 2023
e84b475
Add minimum number of required small nodes before splitting
vegasten May 9, 2023
be9e668
Cleanup after review
vegasten May 10, 2023
502387e
Fix typo
vegasten May 11, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
114 changes: 84 additions & 30 deletions CadRevealComposer/Operations/SectorSplitting/SectorSplitterOctree.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@ public class SectorSplitterOctree : ISectorSplitter
private const float MinDiagonalSizeAtDepth_1 = 7; // arbitrary value for min size at depth 1
private const float MinDiagonalSizeAtDepth_2 = 4; // arbitrary value for min size at depth 2
private const float MinDiagonalSizeAtDepth_3 = 1.5f; // arbitrary value for min size at depth 3
private const float SplitDetailsThreshold = 0.1f; // arbitrary value for splitting out details from last nodes
private const int MinimumNumberOfSmallPartsBeforeSplitting = 1000;

public IEnumerable<InternalSector> SplitIntoSectors(APrimitive[] allGeometries)
{
Expand Down Expand Up @@ -85,9 +87,9 @@ private IEnumerable<InternalSector> SplitIntoSectorsRecursive(
int depthToStartSplittingGeometry
)
{
/* Recursively divides space into eight voxels of about equal size (each dimension X,Y,Z is divided in half).
/*
* Recursively divides space into eight voxels of about equal size (each dimension X,Y,Z is divided in half).
* Note: Voxels might have partial overlap, to place nodes that is between two sectors without duplicating the data.
* Important: Geometries are grouped by NodeId and the group as a whole is placed into the same voxel (that encloses all the geometries in the group).
*/

if (nodes.Length == 0)
Expand Down Expand Up @@ -121,23 +123,12 @@ int depthToStartSplittingGeometry

if (!subVoxelNodes.Any())
{
var sectorId = (uint)sectorIdGenerator.GetNextId();
var path = $"{parentPath}/{sectorId}";
var geometries = nodes.SelectMany(n => n.Geometries).ToArray();
var minDiagonal = nodes.Min(x => x.Diagonal);
var maxDiagonal = nodes.Max(x => x.Diagonal);
var lastSectors = HandleLastNodes(nodes, actualDepth, parentSectorId, parentPath, sectorIdGenerator);

yield return new InternalSector(
sectorId,
parentSectorId,
actualDepth,
path,
minDiagonal,
maxDiagonal,
geometries,
subtreeBoundingBox,
subtreeBoundingBox
);
foreach (var sector in lastSectors)
{
yield return sector;
}
}
else
{
Expand All @@ -150,24 +141,17 @@ int depthToStartSplittingGeometry
if (geometries.Any() || subVoxelNodes.Any())
{
var sectorId = (uint)sectorIdGenerator.GetNextId();
var path = $"{parentPath}/{sectorId}";
var geometryBb = geometries.CalculateBoundingBox();

var minDiagonal = mainVoxelNodes.Any() ? mainVoxelNodes.Min(x => x.Diagonal) : 0;
var maxDiagonal = mainVoxelNodes.Any() ? mainVoxelNodes.Max(x => x.Diagonal) : 0;
yield return new InternalSector(
yield return CreateSector(
mainVoxelNodes,
sectorId,
parentSectorId,
parentPath,
actualDepth,
path,
minDiagonal,
maxDiagonal,
geometries,
subtreeBoundingBox,
geometryBb
subtreeBoundingBox
);

parentPathForChildren = path;
parentPathForChildren = $"{parentPath}/{sectorId}";
parentSectorIdForChildren = sectorId;
}

Expand Down Expand Up @@ -225,6 +209,76 @@ int depthToStartSplittingGeometry
}
}

private IEnumerable<InternalSector> HandleLastNodes(
Copy link
Collaborator

@Strepto Strepto May 10, 2023

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

Node[] nodes,
int depth,
uint? parentSectorId,
string parentPath,
SequentialIdGenerator sectorIdGenerator
)
{
var sectorId = (uint)sectorIdGenerator.GetNextId();

var smallNodes = nodes.Where(n => n.Diagonal < SplitDetailsThreshold).ToArray();
var largeNodes = nodes.Except(smallNodes).ToArray();
Copy link
Collaborator

@Strepto Strepto May 10, 2023 •
edited
Loading

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();


var subtreeBoundingBox = nodes.CalculateBoundingBox();

if (
largeNodes.Length > 0
&& smallNodes.Length > MinimumNumberOfSmallPartsBeforeSplitting
&& smallNodes.Any(n => n.Diagonal > 0) // There can be nodes with diagonal = 0, no point in splitting if they're all 0
)
{
yield return CreateSector(largeNodes, sectorId, parentSectorId, parentPath, depth, subtreeBoundingBox);

var smallNodesSectorId = (uint)sectorIdGenerator.GetNextId();
var smallNodesParentPath = $"{parentPath}/{sectorId}";

yield return CreateSector(
smallNodes,
smallNodesSectorId,
sectorId,
smallNodesParentPath,
depth + 1,
smallNodes.CalculateBoundingBox()
);
}
else
{
yield return CreateSector(nodes, sectorId, parentSectorId, parentPath, depth, subtreeBoundingBox);
}
}

private InternalSector CreateSector(
Copy link
Collaborator

@Strepto Strepto May 10, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

Node[] nodes,
uint sectorId,
uint? parentSectorId,
string parentPath,
int depth,
BoundingBox subtreeBoundingBox
)
{
var path = $"{parentPath}/{sectorId}";

var minDiagonal = nodes.Any() ? nodes.Min(n => n.Diagonal) : 0;
var maxDiagonal = nodes.Any() ? nodes.Max(n => n.Diagonal) : 0;
var geometries = nodes.SelectMany(n => n.Geometries).ToArray();
var geometryBoundingBox = geometries.CalculateBoundingBox();

return new InternalSector(
sectorId,
parentSectorId,
depth,
path,
minDiagonal,
maxDiagonal,
geometries,
subtreeBoundingBox,
geometryBoundingBox
);
}

private static InternalSector CreateRootSector(uint sectorId, string path, BoundingBox subtreeBoundingBox)
{
return new InternalSector(sectorId, null, 0, path, 0, 0, Array.Empty<APrimitive>(), subtreeBoundingBox, null);
Expand Down