4 Commits

Author	SHA1	Message	Date
Copilot	aab90e1fbc	Fixes #4463 - Adds LineCanvas.GetRegion() - 10x performance regression in RenderLineCanvas (#4464) ... * Initial plan * Revert xunit maxParallelThreads to default to fix performance issue Co-authored-by: tig <585482+tig@users.noreply.github.com> * Fix 10x performance regression in RenderLineCanvas Found via binary search of 51 commits that commit 027018368 introduced the slowdown by calling context.AddDrawnRectangle() for every cell in the LineCanvas (thousands of times per frame). Fixed by reporting the entire LineCanvas.Bounds once instead of per-cell. Test results: - Before: 1m 33s (10x slower than v2_release) - After: 9s (same as v2_release) Fixes the performance issue in AllViews_Draw_Does_Not_Layout test. Co-authored-by: tig <585482+tig@users.noreply.github.com> * Add comment clarifying LineCanvas coordinate system Addresses code review feedback about coordinate system conversion. LineCanvas.Bounds is already in screen-relative coordinates since cells are stored with screen positions (used with Driver.Move). Co-authored-by: tig <585482+tig@users.noreply.github.com> * Build non-rectangular Region from LineCanvas cells efficiently Instead of reporting entire LineCanvas.Bounds (which includes empty space), efficiently build a Region that encompasses only the actual line cells. Algorithm groups contiguous horizontal cells into spans per row, avoiding per-cell overhead while accurately representing the non-rectangular shape. Performance maintained: 10s (same as v2_release) All tests pass: 5695 drawing tests, 200 LineCanvas tests, 33 transparency tests Co-authored-by: tig <585482+tig@users.noreply.github.com> * Address code review feedback - improve robustness - Remove redundant empty check (caller already verifies) - Add safety check for empty row groups to prevent IndexOutOfRangeException - Update documentation to clarify precondition Tests still pass with 9s performance. Co-authored-by: tig <585482+tig@users.noreply.github.com> * Optimize BuildRegionFromLineCells - eliminate redundant sort Remove redundant OrderBy since X values are already sorted from the initial OrderBy().ThenBy() operation. Also remove unreachable empty check since GroupBy guarantees non-empty groups. Performance maintained at 9s. Co-authored-by: tig <585482+tig@users.noreply.github.com> * Final code review fixes - improve comments and add defensive check - Clarify comment about sorting behavior - Add defensive empty check for edge cases - Performance maintained at 10s Co-authored-by: tig <585482+tig@users.noreply.github.com> * Move BuildRegionFromCells to LineCanvas class Moved the BuildRegionFromCells method from View.Drawing.cs to LineCanvas where it logically belongs as a public static method. Updated View.Drawing.cs to call Drawing.LineCanvas.BuildRegionFromCells. Performance maintained at 9s. All tests pass. Co-authored-by: tig <585482+tig@users.noreply.github.com> * Update documentation for BuildRegionFromCells - Clarify that method handles empty cellMap gracefully - Improve comment explaining the defensive empty check - Performance maintained at 10s Co-authored-by: tig <585482+tig@users.noreply.github.com> * Rename BuildRegionFromCells to GetRegion Renamed the method as requested to have a more concise name. Performance maintained at 10s. All tests pass. Co-authored-by: tig <585482+tig@users.noreply.github.com> * Add GetCellMapWithRegion optimization for single-pass iteration Implemented optimized method that builds both cell map and Region in a single pass through the canvas bounds, eliminating the need for separate GetCellMap() and GetRegion() calls. Benefits: - Reduces from 2 iterations to 1 (GetCellMap then GetRegion -> single pass) - Eliminates sorting overhead (O(n log n) -> O(n)) - X values already sorted due to inner loop order - Builds Region incrementally per row as cells are discovered Performance maintained at 10s. All tests pass: - AllViews_Draw_Does_Not_Layout: 10s - 5695 drawing tests passed - 200 LineCanvas tests passed - 33 transparency tests passed Co-authored-by: tig <585482+tig@users.noreply.github.com> * Add comprehensive unit tests for GetRegion and GetCellMapWithRegion Added 15 new unit tests covering: GetRegion tests: - Empty cell map handling - Single cell regions - Horizontal and vertical lines - L-shaped regions - Discontiguous cells creating separate spans - Intersecting lines GetCellMapWithRegion tests: - Empty canvas handling - Single horizontal/vertical lines - Intersecting lines with proper intersection handling - Complex shapes (boxes) - Consistency with separate GetCellMap + GetRegion calls - Negative coordinate handling - Exclusion region support All 215 LineCanvas tests pass. Performance maintained at 9s. Co-authored-by: tig <585482+tig@users.noreply.github.com> * Fix code style violations per CONTRIBUTING.md - Replace var with explicit types (Dictionary<Point, Cell?>, LineCanvas, Region, etc.) - Replace new TypeName() with target-typed new () where type is declared - Fix foreach loop to use explicit IGrouping<int, Point> type - Use StraightLine instead of var in foreach All style violations fixed per CONTRIBUTING.md coding conventions. Tests still pass: 215 LineCanvas tests, 9s performance maintained. Co-authored-by: tig <585482+tig@users.noreply.github.com> * Code cleanup. * Upgrades Transparent Scenario to demonstrate LineCanvas transparency. "dotnet" is drawn using LineCanvas. * Comment out debug assertions in View drawing logic The debug assertions in the `View` class, which validated the relationships between `Margin`, `Border`, `Padding`, and their parent objects, as well as their `SubViewNeedsDraw` and `NeedsDraw` states, have been commented out. These assertions were conditionally executed when the current object and its `SuperView` were not of type `Adornment`. The code has been retained as comments for potential future reference or debugging purposes, but the assertions are no longer active in the current implementation. --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: tig <585482+tig@users.noreply.github.com> Co-authored-by: Tig <tig@users.noreply.github.com>	2025-12-08 17:13:58 -07:00
Tig	3a8de25dce	Refactored NeedsDraw and SubViewNeedsDraw logic to improve clarity and control over redraw state. Introduced SetSubViewNeedsDrawDownHierarchy for better propagation of redraw flags. Updated Margin and Adornment classes to align with the new redraw management. ... Enhanced `View` class drawing logic to ensure proper ordering of margin and subview rendering, and introduced `DoDrawContent` for encapsulated content drawing. Improved comments and documentation for better maintainability. Updated tests to reflect the new redraw management methods. Made minor formatting changes and removed redundant code for consistency and readability.	2025-12-04 14:41:25 -07:00
Tig	641a0a599c	Optimize View drawing logic and update ClearViewport tests ... Refactored the `View` class to include a `NeedsDraw` check in multiple drawing methods, improving rendering efficiency. Adjusted `OnDrewText` and `DrewText` event handling for consistency. Removed unused code and redundant tests. Rewrote and reorganized `ClearViewportTests` for clarity and compatibility with the new `NeedsDraw` logic. Added new tests to validate `ClearViewport` behavior under various conditions, including transparent viewports, event cancellations, and content-only clearing. Updated namespaces for better alignment, disabled a noisy `ComboBoxTests` test, and improved code formatting and maintainability across files.	2025-12-03 16:30:22 -07:00
Copilot	6d53276be2	Fixes #4289 - Simplify Drawing/Color: unify named color handling under StandardColor and remove layered resolvers (#4432) ... * Initial plan * Delete AnsiColorNameResolver and MultiStandardColorNameResolver, add legacy 16-color names to StandardColor Co-authored-by: tig <585482+tig@users.noreply.github.com> * Refactor and enhance tests for Color, Region, and Lines Refactored `Color` struct by removing unused methods and simplifying logic. Updated namespaces for better organization. Enhanced test coverage for `Color`, `Region`, and `LineCanvas` with new test cases, parameterized tests, and edge case handling. Added `StraightLineExtensionsTests`, `StraightLineTests`, and `RegionClassTests` to validate behavior under various scenarios. Improved `MergeRectangles` stability and addressed crash patterns. Removed legacy features and unused code. Enhanced documentation and optimized performance in key methods. * Improve Color struct and StandardColors functionality Enhanced the Color struct to fully support the alpha channel for rendering intent while maintaining semantic color identity. Updated TryNameColor to ignore alpha when matching colors, ensuring transparency does not affect color resolution. Expanded XML documentation to clarify alpha channel usage and future alpha blending support. Improved drawing documentation to explain the lifecycle, deferred rendering, and color support, including 24-bit true color and legacy 16-color compatibility. Added a new section on transparency and its role in rendering. Revised StandardColors implementation to use modern C# features and ensure consistent ARGB mapping. Added comprehensive tests for StandardColors and Color, covering alpha handling, color parsing, thread safety, and aliased color resolution. Removed outdated tests relying on legacy behavior. Enhanced code readability, maintainability, and test coverage to ensure correctness and backward compatibility. * Code cleanup * Code cleanup * Fix warnings. Code cleanup * Add comprehensive unit tests for ColorStrings class Introduced a new test class `ColorStringsTests` under the `DrawingTests.ColorTests` namespace to validate the functionality of the `ColorStrings` class. Key changes include: - Added tests for `GetColorName` to verify behavior for standard and non-standard colors, ignoring alpha channels, and handling known colors. - Added tests for `GetStandardColorNames` to ensure the method returns a non-empty, alphabetically sorted collection containing all `StandardColor` enum values. - Implemented tests for `TryParseStandardColorName` to validate case-insensitive parsing, hex color support, handling invalid input, and `ReadOnlySpan<char>` compatibility. - Added tests for `TryParseNamedColor` to verify parsing of named and hex colors, handling of aliases, and `ReadOnlySpan<char>` support. - Added round-trip tests to ensure consistency between `GetColorName`, `TryParseNamedColor`, `GetStandardColorNames`, and `TryParseStandardColorName`. These tests ensure robust validation of color parsing and naming functionality. --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: tig <585482+tig@users.noreply.github.com> Co-authored-by: Tig <tig@users.noreply.github.com>	2025-12-03 11:09:02 -07:00