Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Various fixes for Border on Android #15458

Merged
merged 1 commit into from Jul 14, 2023

Conversation

jstedfast
Copy link
Member

@jstedfast jstedfast commented Jun 5, 2023

Description of Change

  • Fixed Border.CrossPlatformArrange() logic
  • Fixed Android's ShapeExtensions.ToPlatform() to not inset the path
    bounds unless innerPath is true.

Issues Fixed

This is an attempt at fixing issue #15339

@jstedfast
Copy link
Member Author

Unfortunately this isn't enough to fix the issue :-(

There seems to be more problems influencing this bug. From what I can tell, something to do with Grid layout (due to star dimensions), but I can't figure that part out without a working debugger. Being stuck with only CWL's makes this incredibly painful.

@Eilon Eilon added the area/drawing ✏️ Shapes, Borders, Shadows, Graphics, custom drawing label Jun 6, 2023
@jstedfast
Copy link
Member Author

Just an updated comment: I no longer think it has to do with * dimensions because this happens even when hard-coding Grid dimensions.

@jstedfast jstedfast force-pushed the dev/jestedfa/grid-border-strokethickness-inset branch 4 times, most recently from c3c2da6 to f855faf Compare June 22, 2023 16:00
@jstedfast jstedfast changed the title Don't double-inset Border strokes Various fixes for Border on Android Jun 22, 2023
@jstedfast
Copy link
Member Author

Okay, so I think that assuming the unit test passes, this bug is now fixed. The tests verified locally as well as with using the following XAML in the Sandbox app:

diff --git a/src/Controls/samples/Controls.Sample.Sandbox/MainPage.xaml b/src/Controls/samples/Controls.Sample.Sandbox/MainPage.xaml
index e1762ab92..f2aff04df 100644
--- a/src/Controls/samples/Controls.Sample.Sandbox/MainPage.xaml
+++ b/src/Controls/samples/Controls.Sample.Sandbox/MainPage.xaml
@@ -3,4 +3,12 @@
     xmlns:x="http://schemas.microsoft.com/winfx/2009/xaml"
     x:Class="Maui.Controls.Sample.MainPage"
     xmlns:local="clr-namespace:Maui.Controls.Sample">
+
+    <Grid ColumnDefinitions="180,180" RowDefinitions="360,360" RowSpacing="0" ColumnSpacing="0">
+        <Border Grid.Row="0" Grid.Column="0" Background="Red" Stroke="Black" StrokeThickness="10" />
+        <Border Grid.Row="0" Grid.Column="1" Background="Orange" Stroke="Black" StrokeThickness="10" />
+        <Border Grid.Row="1" Grid.Column="0" Background="Green" Stroke="Black" StrokeThickness="10" />
+        <Border Grid.Row="1" Grid.Column="1" Background="Blue" Stroke="Black" StrokeThickness="10" />
+    </Grid>
+
 </ContentPage>
\ No newline at end of file

@jstedfast jstedfast force-pushed the dev/jestedfa/grid-border-strokethickness-inset branch 4 times, most recently from fd98eaf to 8af1f15 Compare June 29, 2023 15:12
* Fixed Border.CrossPlatformArrange() logic
* Fixed Android's ShapeExtensions.ToPlatform() to not inset the path
  bounds *unless* innerPath is true.

This is an attempt at fixing issue #15339
@jstedfast jstedfast force-pushed the dev/jestedfa/grid-border-strokethickness-inset branch from f33b663 to a33b76f Compare June 30, 2023 13:34
@@ -251,8 +251,8 @@ StrokeLineJoin switch

public Size CrossPlatformArrange(Graphics.Rect bounds)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you document CrossPlatformArrange? Other than that looks good!

@jstedfast jstedfast merged commit 8ed7af4 into main Jul 14, 2023
28 checks passed
@jstedfast jstedfast deleted the dev/jestedfa/grid-border-strokethickness-inset branch July 14, 2023 19:08
@MartyIX
Copy link
Collaborator

MartyIX commented Jul 15, 2023

@jstedfast Should #15339 be closed now?

@daltzctr
Copy link

daltzctr commented Aug 10, 2023

This seems to have broken several layouts on Windows when stroke thickness is set to larger values (5). Was this PR even tested on Windows?

@jstedfast
Copy link
Member Author

@daltzctr This couldn't have broken Windows because all of the changes were in Android-specific source files.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area/drawing ✏️ Shapes, Borders, Shadows, Graphics, custom drawing platform/android 🤖
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants