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 #31755
Conversation
|
Hey there @@NanthiniMahalingam! Thank you so much for your PR! Someone from the team will get assigned to your PR shortly and we'll get it reviewed. |
dotnet-policy-service
bot
added
the
partner/syncfusion
There was a problem hiding this comment.
Pull Request Overview
This PR introduces density-aware layout calculations in Grid to eliminate pixel rounding discrepancies that cause layout issues on Android devices. The key change is the adoption of a DensityValue struct that tracks both density-independent pixels (dp) and physical pixels to enable precise pixel-perfect distribution.
- Introduces DensityValue struct for pixel-perfect layout calculations
- Modifies Grid layout manager to use density-aware star distribution
- Fixes pixel calculation precision in Android's ToPixels conversion method
Reviewed Changes
Copilot reviewed 8 out of 54 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| src/Core/src/Layouts/DensityValue.cs | New struct for tracking dp and pixel values with precise distribution algorithm |
| src/Core/src/Layouts/GridLayoutManager.cs | Updated to use DensityValue throughout and implement density-aware star sizing |
| src/Core/src/Platform/Android/ContextExtensions.cs | Fixed pixel calculation to use width/height instead of right/bottom coordinates |
| src/Core/src/IViewWithWindow.cs | New interface to enable window access for density retrieval |
| src/Core/src/Platform/ElementExtensions.cs | Enhanced GetWindow method to support IViewWithWindow interface |
| src/Core/tests/UnitTests/Layouts/GridLayoutManagerDensityTest.cs | Comprehensive unit tests for DensityValue pixel distribution |
| src/Controls/tests/TestCases.HostApp/Issues/Issue28117.cs | Test case reproducing the border/label cropping issue |
| src/Controls/tests/TestCases.Shared.Tests/Tests/Issues/Issue28117.cs | UI test for validating the fix |
| } | ||
|
|
||
|
|
||
|
|
There was a problem hiding this comment.
[nitpick] Remove the unnecessary blank line at line 40 to improve code readability.
| } | ||
|
|
||
| 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.
The method signature change from 'static' to instance method is a breaking change to the internal API. Consider maintaining the static signature and passing density as a parameter instead.
|
|
||
| var scale = (fullTargetSize - definition.MinimumSize) / totaldiff; | ||
| var portion = scale * availableSpace; | ||
| definition.Size = definition.MinimumSize + portion; | ||
| } | ||
| } | ||
| /// <summary> | ||
| /// Gets the display density for density-aware calculations. | ||
| /// </summary> | ||
| /// <returns>The display density, or 1.0 if not available.</returns> | ||
| double GetDensity() |
There was a problem hiding this comment.
[nitpick] Remove the unnecessary blank line at line 990 to maintain consistent spacing with other methods.
| Size = new DensityValue(0.0); | ||
| MinimumSize = new DensityValue(0.0); |
There was a problem hiding this comment.
[nitpick] Consider extracting DensityValue.Zero as a static property to avoid repeated instantiation of zero values and improve code readability.
| Size = new DensityValue(0.0); | |
| MinimumSize = new DensityValue(0.0); | |
| Size = DensityValue.Zero; | |
| MinimumSize = DensityValue.Zero; |
|
|
||
| // Total weight = 8, with right-to-left distribution: | ||
| // portions[0]=3: floor(333 * 3/8) = floor(124.875) = 124 | ||
| // portions[1]=2: floor(333 * 2/8) = floor(83.25) = 83 |
There was a problem hiding this comment.
Remove the trailing space after '83' to maintain consistent formatting.
| // portions[1]=2: floor(333 * 2/8) = floor(83.25) = 83 | |
| // portions[1]=2: floor(333 * 2/8) = floor(83.25) = 83 |
NanthiniMahalingam
force-pushed
the
issue-fix-28117
branch
from
984dbf6 to
6737f23
Compare
1 participant
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
#30017
Closed PR #30340 due to unwanted changes introduced during rebasing, so I am creating this new PR.
Output
Android platform