Skip to content

Replace O(n²) bubble chart overlap check with O(n log n) parent/child hierarchy algorithm#644

Draft
Copilot wants to merge 2 commits into
mainfrom
copilot/fix-bug-643
Draft

Replace O(n²) bubble chart overlap check with O(n log n) parent/child hierarchy algorithm#644
Copilot wants to merge 2 commits into
mainfrom
copilot/fix-bug-643

Conversation

Copilot AI commented Mar 26, 2026

Copy link
Copy Markdown
  • Understand the codebase and identify all issues
  • Create BubbleMarker struct extending SpecMarker with per-marker state
  • Eliminate separate parentOf/nextOfPar/nextSib/lastChild/children vectors
  • Fix angle storage: normalize to parent's first child, use ascending order
  • Fix binary search to use ascending order (fixes root bug causing "long fail" test failure)
  • Use std::size_t for indices and std::optional<std::size_t> for possibly-invalid indices
  • Fix nextOfPar[b0] = n0 early assignment (move after addChild)
  • Investigate and remove !candidate1->overlaps(markers[0].circle())
  • Add test case with fixed seed hitting original "TODO bubblechart generation failed" branch
  • Update tests to check all circles are placed (radius == 0 only acceptable for originally zero/negative areas)

💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.

@schaumb schaumb closed this Mar 26, 2026
Copilot stopped work on behalf of schaumb due to an error March 26, 2026 07:57
@schaumb schaumb reopened this Mar 26, 2026
Copilot AI requested a review from schaumb March 26, 2026 09:06
Copilot stopped work on behalf of schaumb due to an error March 26, 2026 09:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants