Refactor wall opening attachment to support batch operations#46
Open
bhairaav wants to merge 1 commit into
Open
Refactor wall opening attachment to support batch operations#46bhairaav wants to merge 1 commit into
bhairaav wants to merge 1 commit into
Conversation
…batch - SingleWall.detachOpening now disposes stale BooleanResult and re-runs resolveOpenings so multi-opening cuts stay consistent; previously the array shrank but the old cut mesh lingered. - SingleWall.resolveOpenings no longer returns early from the whole function when the 2D opening set is empty, so the 3D pass always runs. - Opening update callbacks on both SingleWall and PolyWall now call resolveOpenings instead of setOPGeometry, so editing an opening no longer tears down and rebuilds the host wall base geometry. - Attach listeners are tracked per ogid and removed on detach to stop accumulating stale handlers across attach/detach cycles. - Add attachOpenings / attachDoors / attachWindows batch APIs on both walls so callers can amortise the kernel boolean into a single resolve. - Drop stray runtime console.log calls in door, window, and poly-wall. Tracked remaining kernel gaps in docs/wall-openings-spec-debt.md.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description/Context/What Does Your PR Do
This PR refactors the opening attachment pipeline in
SingleWallandPolyWallto:Introduce batch attachment methods (
attachDoors,attachWindows,attachOpenings) that register multiple openings and resolve geometry only once, instead of once per opening. This significantly improves performance when attaching many openings at once.Centralize opening registration logic via a new private
registerOpening()method that:Map<string, () => void>Fix opening detachment to properly clean up event listeners and dispose of resolved geometry resources via a new
disposeResolvedEntry()helper.Refactor
resolveOpenings()to be more robust:Extract door/window opening logic in
PolyWallintoextractDoorOpening()andextractWindowOpening()helpers to reduce duplication.Add kernel spec debt documentation (
wall-openings-spec-debt.md) outlining gaps in the OpenGeometry kernel that affect correctness of multi-opening boolean operations, with prioritized recommendations for kernel maintainers.Remove debug console.log statements from
Door,Window, andPolyWall.Steps To Run/Review for Other Devs
registerOpening()private method to understand the listener lifecycle management patternattachOpenings([...])now batches resolution (singleresolveOpenings()call vs. N calls)detachOpening()properly cleans up listeners and disposes geometryProduction Log Message
[ ] - Internal refactor: wall opening attachment now supports efficient batch operations and properly manages event listener lifecycle.
https://claude.ai/code/session_01Fkhu5s2YSqr2wDZk4y8CRH