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

[iOS] Border often has wrong initial bounds #23494

Closed
borrmann opened this issue Jul 9, 2024 · 15 comments
Closed

[iOS] Border often has wrong initial bounds #23494

borrmann opened this issue Jul 9, 2024 · 15 comments
Labels
area-controls-border Border i/regression This issue described a confirmed regression on a currently supported version p/2 Work that is important, but is currently not scheduled for release platform/iOS 🍎 s/triaged Issue has been reviewed s/try-latest-version Please try to reproduce the potential issue on the latest public version s/verified Verified / Reproducible Issue ready for Engineering Triage t/bug Something isn't working

Comments

@borrmann
Copy link
Contributor

borrmann commented Jul 9, 2024

Description

reopening issue #20381

Steps to Reproduce

No response

Link to public reproduction project repository

https://github.com/borrmann/BorderDrawingIssue

Version with bug

8.0.61 SR6.1

Is this a regression from previous behavior?

Yes, this used to work in .NET MAUI

Last version that worked well

8.0.14

Affected platforms

iOS

Affected platform versions

No response

Did you find any workaround?

no

Relevant log output

No response

@borrmann borrmann added the t/bug Something isn't working label Jul 9, 2024
Copy link
Contributor

github-actions bot commented Jul 9, 2024

Hi I'm an AI powered bot that finds similar issues based off the issue title.

Please view the issues below to see if they solve your problem, and if the issue describes your problem please consider closing this one and thumbs upping the other issue to help us prioritize it. Thank you!

Open similar issues:

Closed similar issues:

Note: You can give me feedback by thumbs upping or thumbs downing this comment.

@Zhanglirong-Winnie Zhanglirong-Winnie added s/verified Verified / Reproducible Issue ready for Engineering Triage s/triaged Issue has been reviewed labels Jul 9, 2024
@Zhanglirong-Winnie
Copy link

This issue has been verified using Visual Studio 17.11.0 Preview 2.1(8.0.61 & 8.0.40). Can repro on iOS platform.
2

@PureWeen PureWeen added area-controls-border Border potential-regression This issue described a possible regression on a currently supported version., verification pending p/3 Work that is nice to have labels Jul 9, 2024
@PureWeen PureWeen added this to the Backlog milestone Jul 9, 2024
@jonmdev
Copy link

jonmdev commented Jul 10, 2024

What you are seeing is a combination of these two bugs:

#18204

#23070

You can solve the animation which is the main problem with the NotAnimatedBorderHandler implementation in the first bug thread.

The other has no solution yet.

@albyrock87
Copy link
Contributor

@borrmann the animation issue fix has been released in the latest SR

@IeuanWalker
Copy link

@albyrock87 I'm experiencing this in the latest version, 8.0.70

@jonmdev
Copy link

jonmdev commented Jul 16, 2024

@albyrock87 re: the animation bug I think you should have used the approach I wrote here rather than just removing it from one layer:

using Microsoft.Maui.Handlers;
using Microsoft.Maui.Platform;

#if IOS
using UIKit;
using ContentView = Microsoft.Maui.Platform.ContentView;
using Foundation;
using CoreAnimation;
#endif 

namespace iOSBorderShadowAnimationBug {

    //TEMP BUG FIX FOR: https://github.com/dotnet/maui/issues/18204
    public class NotAnimatedBorderHandler : BorderHandler {

#if IOS
        private class BorderContentView : ContentView {
            public override void LayoutSubviews() {

                Layer.RemoveAllAnimations();

                if (Layer.Sublayers != null) {

                    for (int i=0; i < Layer.Sublayers.Count(); i++) {
                        Layer.Sublayers[i].RemoveAllAnimations();
                    }
                }
                
                base.LayoutSubviews();
            }

        }

        protected override ContentView CreatePlatformView() {
            _ = VirtualView ?? throw new InvalidOperationException($"{nameof(VirtualView)} must be set to create a {nameof(ContentView)}");
            _ = MauiContext ?? throw new InvalidOperationException($"{nameof(MauiContext)} cannot be null");

            return new BorderContentView {
                CrossPlatformLayout = VirtualView
            };
        }
#endif
    }
}

I don't know much about iOS animations and my hat is off to you for coming up with the solution, but this removes animations from everything in case it is a multilayer element. I would be suprprised if anyone had any more animations after that.

Edit: Oh never mind I see you did do something similar to that as well here: https://github.com/dotnet/maui/pull/23156/files#diff-fe6d138a657c96dcb93e2a6bbc458c3fec125cf61417747a9029e971b06cd08a

Not sure why it's not working then.

@maonaoda
Copy link
Contributor

maonaoda commented Jul 16, 2024

@jonmdev

@albyrock87 re: the animation bug I think you should have used the approach I wrote here rather than just removing it from one layer:

using Microsoft.Maui.Handlers;
using Microsoft.Maui.Platform;

#if IOS
using UIKit;
using ContentView = Microsoft.Maui.Platform.ContentView;
using Foundation;
using CoreAnimation;
#endif 

namespace iOSBorderShadowAnimationBug {

    //TEMP BUG FIX FOR: https://github.com/dotnet/maui/issues/18204
    public class NotAnimatedBorderHandler : BorderHandler {

#if IOS
        private class BorderContentView : ContentView {
            public override void LayoutSubviews() {

                Layer.RemoveAllAnimations();

                if (Layer.Sublayers != null) {

                    for (int i=0; i < Layer.Sublayers.Count(); i++) {
                        Layer.Sublayers[i].RemoveAllAnimations();
                    }
                }
                
                base.LayoutSubviews();
            }

        }

        protected override ContentView CreatePlatformView() {
            _ = VirtualView ?? throw new InvalidOperationException($"{nameof(VirtualView)} must be set to create a {nameof(ContentView)}");
            _ = MauiContext ?? throw new InvalidOperationException($"{nameof(MauiContext)} cannot be null");

            return new BorderContentView {
                CrossPlatformLayout = VirtualView
            };
        }
#endif
    }
}

I don't know much about iOS animations and my hat is off to you for coming up with the solution, but this removes animations from everything in case it is a multilayer element. I would be suprprised if anyone had any more animations after that.

Edit: Oh never mind I see you did do something similar to that as well here: https://github.com/dotnet/maui/pull/23156/files#diff-fe6d138a657c96dcb93e2a6bbc458c3fec125cf61417747a9029e971b06cd08a

Not sure why it's not working then.

Nice ideal, It fixed #23620
This means that #23156 is ​​still insufficient.

@maonaoda
Copy link
Contributor

@jonmdev
As a temporary workaround, I suggest directly replacing the BorderHandler`s creation method like this

BorderHandler.PlatformViewFactory = (h) => new NoneAnimatedBorderContentView() { CrossPlatformLayout = h.VirtualView };

@albyrock87
Copy link
Contributor

@jonmdev my understanding is that the remaining 1 frame issue is due to the change on IsVisible.

I think it might be due to the order of changes.
Probably the visibility of the layer is changing before applying the changes to the background.

@maonaoda
Copy link
Contributor

Yes, that is IsVisible like #23620

@jonmdev
Copy link

jonmdev commented Jul 17, 2024

@jonmdev my understanding is that the remaining 1 frame issue is due to the change on IsVisible.

I think it might be due to the order of changes. Probably the visibility of the layer is changing before applying the changes to the background.

Unfortunately it occurs not just when isVisible is toggled. It happens constantly in my real app all over the place. Using IsVisible is just the easiest way I found to reproduce it.

@PureWeen
Copy link
Member

PureWeen commented Aug 9, 2024

@Zhanglirong-Winnie can you verify which .NET 8 version of MAUI this broke on

@PureWeen PureWeen added p/2 Work that is important, but is currently not scheduled for release and removed p/3 Work that is nice to have labels Aug 9, 2024
@Zhanglirong-Winnie
Copy link

@Zhanglirong-Winnie can you verify which .NET 8 version of MAUI this broke on

Hi, @PureWeen
This issue started to occur with 8.0.20, and worked fine with 8.0.14.

@Zhanglirong-Winnie Zhanglirong-Winnie added i/regression This issue described a confirmed regression on a currently supported version and removed potential-regression This issue described a possible regression on a currently supported version., verification pending labels Aug 12, 2024
@karthikraja-arumugam
Copy link

The issue is not replicated with 8.0.90, can you test with latest version?

@karthikraja-arumugam karthikraja-arumugam added the s/try-latest-version Please try to reproduce the potential issue on the latest public version label Sep 19, 2024
Copy link
Contributor

Hi @borrmann. We have added the "s/try-latest-version" label to this issue, which indicates that we'd like you to try and reproduce this issue on the latest available public version. This can happen because we think that this issue was fixed in a version that has just been released, or the information provided by you indicates that you might be working with an older version.

You can install the latest version by installing the latest Visual Studio (Preview) with the .NET MAUI workload installed. If the issue still persists, please let us know with any additional details and ideally a reproduction project provided through a GitHub repository.

This issue will be closed automatically in 7 days if we do not hear back from you by then - please feel free to re-open it if you come back to this issue after that time.

@dotnet-policy-service dotnet-policy-service bot removed this from the Backlog milestone Sep 27, 2024
@github-actions github-actions bot locked and limited conversation to collaborators Oct 27, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-controls-border Border i/regression This issue described a confirmed regression on a currently supported version p/2 Work that is important, but is currently not scheduled for release platform/iOS 🍎 s/triaged Issue has been reviewed s/try-latest-version Please try to reproduce the potential issue on the latest public version s/verified Verified / Reproducible Issue ready for Engineering Triage t/bug Something isn't working
Projects
None yet
Development

No branches or pull requests

8 participants