-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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
[net7.0] Fixed Android's StreamImageSourceService.LoadDrawableAsync() #16640
Conversation
/rebase |
469875f
to
08d3c0f
Compare
/azp run MAUI-DeviceTests-public |
Azure Pipelines successfully started running 1 pipeline(s). |
|
src/Core/AndroidNative/maui/src/main/java/com/microsoft/maui/glide/MauiCustomViewTarget.java
Show resolved
Hide resolved
/azp run |
Azure Pipelines successfully started running 2 pipeline(s). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ImageSetFromStreamRenders is failing
Transition back to the calling thread context (which should theoretically be the main thread). Fixes issue #14052
9481d79
to
c247bc0
Compare
/azp run MAUI-DeviceTests-public |
@mattleibow nope :( Looks like the error is in some screenshotting code and the width/height are -1 or something. Probably something needs to be fixed in the unit test or in the testing infrastructure, but I'm not sure what. |
Hi @github-actions[bot]. We have added the "s/pr-needs-author-input" label to this issue, which indicates that we have an open question/action for you before we can take further action. This PRwill be closed automatically in 14 days if we do not hear back from you by then - please feel free to re-open it if you come back to this PR after that time. |
SetupBuilder(); | ||
var layout = new VerticalStackLayout() | ||
{ | ||
HeightRequest = 100, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@mattleibow @jstedfast I modified this test to just set the height/width because I don't think the remeasuring of the layout is relevant to the test here.
I also don't think this test completely accurately embodies the original issue. I rolled back the changes from this PR and this test still passes without throwing a UIThread exception
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I haven't been able to reproduce the original issue outside the sample repository.
I started a branch here to attempt reproducing
https://github.com/dotnet/maui/tree/test_for_Issue14052 but haven't had any luck yet.
I did test this PR directly against the repro and was able to reproduce the crash and then verified this PR fixes the crash.
This fix has been on NET8 since preview5 so I feel alright to merge this Backport
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
D'oh, this Width/HeightRequest change makes total sense. I was assuming the issue was that the screenshot was happening before layout had finished or something and so the width/height of the element wasn't yet known (hence the -1 values).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
FWIW, I think the test only ever randomly failed - I don't think it failed every run.
/azp run MAUI-DeviceTests-public |
Azure Pipelines successfully started running 1 pipeline(s). |
Backport of #14109 to net7.0
/cc @PureWeen @jstedfast