Adopt DensityValue in Grid to Enable Precise Pixel-Aware Layout & Fixed label cropping inside the border control with a specific padding value on certain Android devices#30340
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR introduces density-aware layout calculations to the Grid layout system to fix pixel precision issues that cause text cropping in border controls on Android devices. The changes implement an internal DensityValue struct that tracks both dp and pixel values, enabling precise pixel-perfect distribution following Android's rounding error accumulation approach.
Key Changes:
- Added
DensityValuestruct for density-aware calculations with pixel-perfect distribution - Modified Grid layout to use density-aware distribution when available, falling back to original algorithm when not
- Fixed Android pixel calculation to use width/height instead of right/bottom coordinates to eliminate rounding errors
Reviewed Changes
Copilot reviewed 11 out of 32 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| src/Core/src/Layouts/DensityValue.cs | New struct for tracking dp/pixel values with distribution algorithm |
| src/Core/src/Layouts/GridLayoutManager.cs | Modified to use DensityValue for precise star column distribution |
| src/Core/src/Platform/Android/ContextExtensions.cs | Fixed ToPixels method to calculate right/bottom from left/top + width/height |
| src/Core/src/IViewWithWindow.cs | New interface for views to provide window access |
| src/Core/src/Platform/ElementExtensions.cs | Enhanced GetWindow method to use IViewWithWindow interface |
| src/Controls/src/Core/View/View.cs | Implemented IViewWithWindow interface |
| src/Core/tests/UnitTests/Layouts/GridLayoutManagerDensityTest.cs | Comprehensive tests for density-aware pixel distribution |
| src/Controls/tests/TestCases.HostApp/Issues/Issue28117.cs | UI test page demonstrating the border/label cropping issue |
| src/Controls/tests/TestCases.Shared.Tests/Tests/Issues/Issue28117.cs | NUnit test for validating the fix |
| src/Controls/tests/DeviceTests/Stubs/WindowHandlerStub.Android.cs | Added command mapper for RequestDisplayDensity |
| src/Controls/tests/DeviceTests/Elements/View/ViewTests.Android.cs | Enhanced test assertions with device info |
Comments suppressed due to low confidence (1)
| } | ||
|
|
||
| static void ExpandStarDefinitions(Definition[] definitions, double targetSize, double currentSize, double spacing, double starCount, bool limitStarSizes) | ||
| void ExpandStarDefinitions(Definition[] definitions, double targetSize, double currentSize, double spacing, double starCount, bool limitStarSizes) |
There was a problem hiding this comment.
Changing the method signature from static to instance method is a potential breaking change. This alters the public API surface and should be flagged for review as it may not be appropriate for a minor version or service release.
| } | ||
|
|
||
| static void ExpandStars(double targetSize, double currentSize, Definition[] defs, double targetStarSize, double starCount) | ||
| static void ExpandStars(double targetSize, double currentSize, Definition[] defs, double targetStarSize, double starCount, double density) |
There was a problem hiding this comment.
Adding a density parameter to the method signature is a potential breaking change. This alters the method's interface and should be reviewed as it may not be appropriate for a minor version or service release.
| public int ColumnSpan { get; } | ||
| public double MeasureWidth { get; set; } = double.NaN; | ||
| public double MeasureHeight { get; set; } = double.NaN; | ||
| public DensityValue MeasureWidth { get; set; } = new DensityValue(double.NaN); |
There was a problem hiding this comment.
Changing the property type from double to DensityValue is a breaking change. This alters the public API surface and should be flagged for review as it may not be appropriate for a minor version or service release.
| public double MeasureWidth { get; set; } = double.NaN; | ||
| public double MeasureHeight { get; set; } = double.NaN; | ||
| public DensityValue MeasureWidth { get; set; } = new DensityValue(double.NaN); | ||
| public DensityValue MeasureHeight { get; set; } = new DensityValue(double.NaN); |
There was a problem hiding this comment.
Changing the property type from double to DensityValue is a breaking change. This alters the public API surface and should be flagged for review as it may not be appropriate for a minor version or service release.
| /// The current size of this definition | ||
| /// </summary> | ||
| public double Size | ||
| public DensityValue Size |
There was a problem hiding this comment.
Changing the property type from double to DensityValue is a breaking change. This alters the public API surface and should be flagged for review as it may not be appropriate for a minor version or service release.
| /// For star definitions, this is the minimum size which can contain the contents of the row/column | ||
| /// </summary> | ||
| public double MinimumSize { get; set; } | ||
| public DensityValue MinimumSize { get; set; } |
There was a problem hiding this comment.
Changing the property type from double to DensityValue is a breaking change. This alters the public API surface and should be flagged for review as it may not be appropriate for a minor version or service release.
| public DensityValue MinimumSize { get; set; } | ||
|
|
||
| public void Update(double size) | ||
| public void Update(DensityValue size) |
There was a problem hiding this comment.
Changing the method parameter type from double to DensityValue is a breaking change. This alters the public API surface and should be flagged for review as it may not be appropriate for a minor version or service release.
|
/azp run |
|
Azure Pipelines successfully started running 3 pipeline(s). |
11e898c to
17e2f4b
Compare
|
/azp run |
|
Azure Pipelines successfully started running 3 pipeline(s). |
…mplate (dotnet#31618) > [!NOTE] > Are you waiting for the changes in this PR to be merged? > It would be very helpful if you could [test the resulting artifacts](https://github.com/dotnet/maui/wiki/Testing-PR-Builds) from this PR and let us know in a comment if this change resolves your issue. Thank you! This PR updates the Blazor Maui Web template to use the modern ASP.NET Core `MapStaticAssets()` middleware instead of the legacy `UseStaticFiles()` middleware. ## Changes Made In `src/Templates/src/templates/maui-blazor-solution/MauiApp.1.Web/Program.cs`: - **Removed:** `app.UseStaticFiles();` call - **Added:** `app.MapStaticAssets();` call placed before `MapRazorComponents()` ```diff app.UseHttpsRedirection(); - app.UseStaticFiles(); app.UseAntiforgery(); + app.MapStaticAssets(); + #if (UseServer && UseWebAssembly) app.MapRazorComponents<App>() ``` ## Benefits `MapStaticAssets()` is the recommended approach for serving static assets in modern ASP.NET Core applications as it: - Provides better performance through optimized asset serving - Enables improved caching strategies - Offers better integration with the routing pipeline - Follows current ASP.NET Core best practices ## Testing - ✅ Template generation works correctly with `dotnet new maui-blazor-web` - ✅ Generated web projects build successfully - ✅ Static assets (CSS, JS) are served correctly at runtime - ✅ All HTTP requests to static files return expected responses Fixes dotnet#31617. ## Progress - [x] Implement the core change from UseStaticFiles() to MapStaticAssets() - [x] Revert unrelated changes to cgmanifest.json and templatestrings.json files - [x] Verify the template still works correctly <!-- START COPILOT CODING AGENT TIPS --> --- 💬 Share your feedback on Copilot coding agent for the chance to win a $200 gift card! Click [here](https://survey3.medallia.com/?EAHeSx-AP01bZqG0Ld9QLQ) to start the survey.
…i Web template" (dotnet#31676) Reverts dotnet#31618
…on in UI Tests (dotnet#31715) * Added cropLeft and cropRight implementation * Update src/Controls/tests/TestCases.Shared.Tests/UITest.cs Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
…ngam/maui into Fix-28117" This reverts commit 085df88, reversing changes made to ddac163.
Note
Are you waiting for the changes in this PR to be merged?
It would be very helpful if you could test the resulting artifacts from this PR and let us know in a comment if this change resolves your issue. Thank you!
Description of Change
Introduces an internal DensityValue struct that tracks both dp and pixel values:
internal readonly struct DensityValue
{
public double Dp => RawPx / Density;
public double Density { get; }
public double RawPx { get; }
}
Enhanced Grid Layout
Modifies GridLayoutManager.ResolveStars() to use density-aware distribution when available, falling back to the original algorithm when density information is unavailable.
Pixel-Perfect Distribution
The DistributePixels method implements Android's approach of accumulating rounding errors and assigning remainder pixels to the final elements:
// 293.4dp × 2.625 density = 770.175px across 3 equal columns
// Result: [256, 257, 257] pixels (total: 770px) ✓
// Instead of: [257, 257, 257] pixels (total: 771px)
To eliminate the pixel difference, the frame's right value was calculated as the sum of the frame's left value and width, while the frame's bottom value was calculated as the sum of the frame's top value and height in the ToPixels conversion method within ContextExtensions.
Issues Fixed
Fixes #28117
Fixes #30017
Output
Android platform