Skip to content
This repository has been archived by the owner on May 1, 2024. It is now read-only.

Shapes on iOS fail to render if the bounds of the view is set prior to renderer creation #13284

Merged
merged 5 commits into from
Jan 15, 2021

Conversation

PureWeen
Copy link
Contributor

@PureWeen PureWeen commented Jan 5, 2021

Description of Change

The ShapeRenderer was setup to UpdateSize only when the width/height changed. So in cases where the width/height were already set the native shape would never draw. This issue was present on shell because on shell when you switch Flyout Items all of the renderers get removed and will get re-added when navigating back. Because the xplat size of the Ellipse was already set then on the second navigation the native size would never get set. This issue probably manifests itself in other cases as well. For example, if someone sets the bounds of a shape before attaching a renderer it won't draw

Issues Resolved

Platforms Affected

  • iOS
  • Android

Testing Procedure

PR Checklist

  • Targets the correct branch
  • Tests are passing (or failures are unrelated)

@PureWeen PureWeen added this to To do in vNext+1 (5.0.0) via automation Jan 5, 2021
@PureWeen PureWeen added this to the 5.0.0 milestone Jan 5, 2021
@PureWeen PureWeen added the blocker Issue blocks next stable release. Prioritize fixing and reviewing this issue. label Jan 5, 2021
@rmarinho rmarinho moved this from To do to In Review in vNext+1 (5.0.0) Jan 5, 2021
@PureWeen PureWeen added the DO-NOT-MERGE-!!! 🛑 This is in progress and needs to be updated before it can be merged. label Jan 7, 2021
@samhouts samhouts added p/iOS 🍎 a/radiobutton 🔘 a/shell 🐚 i/high Completely doesn't work, crashes, or is unusably slow, has no obvious workaround; occurs less often t/bug 🐛 a/shapes labels Jan 8, 2021
@jsuarezruiz
Copy link
Contributor

I just have a question, what is the reason for the DO-NOT-MERGE!!! label?

@PureWeen
Copy link
Contributor Author

@jsuarezruiz DNM got added once I noticed your PR here #13167 that solves the same issue differently. So, wanted to make sure we got the best of both PRs merged together and then merged into 5.0.0

@jsuarezruiz
Copy link
Contributor

jsuarezruiz commented Jan 15, 2021

We had several issues and PRs with a similar fix although with different examples, etc. I have reviewed all related PRs and issues and have grouped them into this PR.
Added the fix for Android too. In this way, this PR is an important fix that affects a large number of issues (marked as blocker).
Here are screenshots of the added examples:
99787445-0b17f780-2b20-11eb-8ef8-9054f130303e
96897202-a620a180-148e-11eb-9555-edad7c2bf36c
91295958-23d07580-e79c-11ea-9af5-b285a6064506
102372868-ee0c0280-3fbf-11eb-86c3-24f0b798ae4e

@durandt
Copy link

durandt commented Feb 16, 2021

Adding this just for reference in case other people stumble on this issue:

This PR also fixes a rendering crash on Android when a shape with a radial gradient would be added to a view outside of the current view hierarchy.

Error
Java.Lang.IllegalArgumentException: radius must be > 0

Fix
Upgrade to latest Xamarin.Forms 5.0.0 or apply the changes in ShapeRenderer from this changeset.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.