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

[Windows] Fix Border corners clipping issue #14403

Merged
merged 23 commits into from
Jul 28, 2023
Merged

Conversation

jsuarezruiz
Copy link
Contributor

@jsuarezruiz jsuarezruiz commented Apr 5, 2023

Description of Change

Fix Border corners clipping mistake on Windows. Checking x I realized that the changes applied in #10964 in Android and iOS were not in Windows.

We was clipping using the Border shape. That is correct in many cases but not always. Using a RoundRectangle, the Corner Radius value is not the same in the inner or outer. We want in this case user the inner value.
image

Before
border-changes-before
Notice the internal white space between the content and the border.

After
border-changes-after

Fix #9129

@jsuarezruiz jsuarezruiz added t/bug Something isn't working area-drawing Shapes, Borders, Shadows, Graphics, BoxView, custom drawing area-controls-border Border labels Apr 5, 2023
@ghost ghost added the legacy-area-controls Label, Button, CheckBox, Slider, Stepper, Switch, Picker, Entry, Editor label Apr 5, 2023
@jsuarezruiz jsuarezruiz requested a review from hartez April 10, 2023 15:15
@jsuarezruiz
Copy link
Contributor Author

@hartez Could I have your feedback here?. Thanks!

hartez
hartez previously requested changes May 10, 2023
src/Core/src/Platform/Windows/ContentPanel.cs Outdated Show resolved Hide resolved
@jsuarezruiz jsuarezruiz requested a review from hartez May 31, 2023 08:58
src/Core/src/Platform/Windows/ContentPanel.cs Outdated Show resolved Hide resolved
UpdateClip(borderShape);
}

internal void UpdateBorderStroke(IBorderStroke borderStroke)
Copy link
Member

Choose a reason for hiding this comment

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

I think this can be public on net8 no @hartez ? or else the Obsolete comment will not make sense since this is internal users can't call it

src/Core/src/PublicAPI/net-windows/PublicAPI.Shipped.txt Outdated Show resolved Hide resolved
@jsuarezruiz jsuarezruiz requested a review from rmarinho June 22, 2023 08:22
Copy link
Member

@rmarinho rmarinho left a comment

Choose a reason for hiding this comment

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

I was testing the gallery and seems some calculations are still wrong when we have content inside, we can see this using the gallery

-Go to the Border Playground

  • Set Border Shape as Rectangle
  • Set a solid Border
  • Increase the Border Width to around 15
  • You can see the text gets clipped
Screenshot 2023-06-22 at 16 53 24

here's how it looks on Android, seems ok:

Screenshot_1687448718

@jsuarezruiz jsuarezruiz marked this pull request as draft June 27, 2023 15:03
@jsuarezruiz
Copy link
Contributor Author

I was testing the gallery and seems some calculations are still wrong when we have content inside, we can see this using the gallery

-Go to the Border Playground

  • Set Border Shape as Rectangle
  • Set a solid Border
  • Increase the Border Width to around 15
  • You can see the text gets clipped
Screenshot 2023-06-22 at 16 53 24 here's how it looks on Android, seems ok:

Screenshot_1687448718

Reviewed and you are right. Without this PR changes can still reproduce it, so you have found a different issue. Will try to solve the issue in this same PR, and if require a bug amount of changes will open a new issue with a different associated PR.

@jsuarezruiz jsuarezruiz requested a review from rmarinho July 13, 2023 10:40
@jsuarezruiz
Copy link
Contributor Author

Applied changes to fix the issue detected by Rui:
fix-border-sizing

@jsuarezruiz
Copy link
Contributor Author

Added device tests:
image

@jsuarezruiz jsuarezruiz marked this pull request as ready for review July 18, 2023 09:35
@samhouts samhouts added this to the .NET 8 GA milestone Jul 26, 2023
@rmarinho rmarinho merged commit 12c39bb into main Jul 28, 2023
@rmarinho rmarinho deleted the housekeeping-border-corners branch July 28, 2023 23:22
rmarinho added a commit that referenced this pull request Aug 3, 2023
* [create-pull-request] automated change (#16413)

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>

* [Windows] Fix Border corners clipping issue (#14403)

* Fix Border corners clipping mistake on Windows

* Updated impl

* Created method to avoid duplicated code

* Revert uneccesary changes

* Changes in clipping sizing

* Fix Rui issue

* Added device tests

* Remove unnecessary changes

---------

Co-authored-by: Javier Suárez <[email protected]>

* Add ViewHandler Mapper docs (#16296)

Co-authored-by: Juan Diego Herrera <[email protected]>

* [create-pull-request] automated change (#16427)

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>

* Fix device tests to tun on arm64 (#16408)

* Build on arm64

* magic

* Only install on Arm64 machines

* cleanup demands on the pipeline and add windows parameter

* Print system arch

* Fix getting OSArchitecture

---------

Co-authored-by: Rui Marinho <[email protected]>

* [uitests] Fix appium script (#16451)

* Bump node version

* Add more info to install script

* Try clean modules before install

* Uninstall before install

* fix

* Cleanup yaml

* Write output of drivers

* Try remove plugins before uninstall

* [iOS] Fix Editor placeholder issues  (#15883)

* Fix iOS Editor placeholder size, characterspacing and fontsize issues

* Added Device Test

* Invalidate CharacterSpacing updating the placeholder

* Updated Impl

* [Essentials] Make PublisherName public on NET8 (#16454)

* Fix EntryCellRenderer to use fromhandler for text changes (#16458)

* [compatibility] Skip test on iOS (#16452)

* [iOS] Re-enable editor tests (#16365)

* Re-enable editor tests on iOS

* Fix merge problem

* [build] Remove the old step for vs insertion on release (#16457)

* Reinstate IContentView and ILayout methods (#16411)

* Reinstate IContentView and ILayout methods
Fixes #16166

* Limit fake methods to NET Standard 2.0

* Reverse the polarity

* Simplify

* Work around compatibility implementations of ICV/ICPL methods on ScrollView

* Use correct interface for Frame

* [net8.0] Don't move to use arm yet here

* [net8.0] Make UpdateBackground public (#16453)

* [api] Make UpdateBackground public

* Keep extensions internal

* Fix issue setting ContentPage gradient background on iOS/Catalyst (#15832)

* Fix provisioning script

* [provisioning] Set Xcode path correctly for Xamarin Hosting

---------

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: Javier Suárez <[email protected]>
Co-authored-by: Javier Suárez <[email protected]>
Co-authored-by: Matthew Leibowitz <[email protected]>
Co-authored-by: Juan Diego Herrera <[email protected]>
Co-authored-by: Shane Neuville <[email protected]>
Co-authored-by: E.Z. Hart <[email protected]>
@github-actions github-actions bot locked and limited conversation to collaborators Dec 12, 2023
@Eilon Eilon removed the legacy-area-controls Label, Button, CheckBox, Slider, Stepper, Switch, Picker, Entry, Editor label May 10, 2024
@samhouts samhouts added the fixed-in-8.0.0-rc.1.9171 Look for this fix in 8.0.0-rc.1.9171 label Aug 2, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-controls-border Border area-drawing Shapes, Borders, Shadows, Graphics, BoxView, custom drawing fixed-in-8.0.0-rc.1.9171 Look for this fix in 8.0.0-rc.1.9171 platform/windows 🪟 t/bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Clipping an Image with a Border has a gap on Windows
6 participants