Skip to content

A block measured every child's bounding box twice - #1232

Open
redbluevn wants to merge 2 commits into
DomCR:masterfrom
redbluevn:moredwg/pr80-block-box-once
Open

A block measured every child's bounding box twice#1232
redbluevn wants to merge 2 commits into
DomCR:masterfrom
redbluevn:moredwg/pr80-block-box-once

Conversation

@redbluevn

Copy link
Copy Markdown
foreach (var item in this.Entities)
{
    if (item.GetBoundingBox().Extent == BoundingBoxExtent.Infinite && ignoreInfinite)
    {
        continue;
    }

    box = box.Merge(item.GetBoundingBox());
}

item.GetBoundingBox() is called once to test the extent and again to merge it.

That is twice per entity at one level — but blocks nest. Insert.GetBoundingBox() calls Block.GetBoundingBox(), so a block whose children are inserts of blocks whose children are inserts costs 2^depth evaluations of the same boxes. Some of those evaluations are not cheap: Arc and Circle sample 256 points, Ellipse 100, Spline up to 256.

Measurement

An architectural drawing of 188,000 entities, warm file cache, three runs each, reading the file and computing every model-space entity's box:

before   8s   8s   8s
after    5s   5s   4s

That is whole-run wall clock including parsing a 19 MB DWG, so the share actually spent measuring boxes fell by considerably more than the 40% the total shows.

Behaviour

Unchanged. The same box is computed, once, and used for both the test and the merge.

dotnet test: 2313 passed / 17 failed — identical to master at 592d70a on this machine, as expected for a change that adds no tests and alters no results.

BlockRecord.GetBoundingBox called item.GetBoundingBox() once to test whether the
extent was infinite and again to merge it. That is twice per entity at one
level - and blocks nest, so a block whose children are inserts of blocks whose
children are inserts costs 2^depth evaluations of the same boxes.

Measured on an architectural drawing of 188,000 entities, warm cache, three runs
each: 8s before, then 5s/5s/4s. That is whole-run wall clock including parsing a
19 MB file, so the share actually spent measuring boxes fell by considerably
more than the 40% the total shows.

No behaviour change: the same box is computed, once.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Comment thread src/ACadSharp/Tables/BlockRecord.cs Outdated
if (item.GetBoundingBox().Extent == BoundingBoxExtent.Infinite
&& ignoreInfinite)
//Once per entity. Asking twice - once to test the extent, once to merge - doubles the
//work at every level of nesting, so a block three deep was measured eight times over.

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

remove the comment

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Done in 965dfe1 — comment removed.

@DomCR DomCR added the bug Something isn't working label Aug 31, 2026
Asked for in review of DomCR#1232.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Ceo4HJNunc2XfA9PnLCpQK
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants