-
Notifications
You must be signed in to change notification settings - Fork 1.9k
[Android] Fix for Resize method returns an image that has already been disposed #29964
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
base: main
Are you sure you want to change the base?
[Android] Fix for Resize method returns an image that has already been disposed #29964
Conversation
… the test case file accordingly.
Hey there @@SyedAbdulAzeemSF4852! Thank you so much for your PR! Someone from the team will get assigned to your PR shortly and we'll get it reviewed. |
/azp run MAUI-UITests-public |
Azure Pipelines successfully started running 1 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.
Pull Request Overview
This PR ensures the Android Resize
method returns a valid, non-disposed image by cloning the internal bitmap when disposal is requested.
- Clone the internal bitmap in
PlatformBitmapExportContext.Bitmap
instead of returning the original. - Added a UI test in
TestCases.Shared.Tests
to verify theResize
method. - Implemented a host app page in
TestCases.HostApp
and embedded the sample image resource.
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
File | Description |
---|---|
src/Graphics/src/Graphics/Platforms/Android/PlatformBitmapExportContext.cs | Updated the Bitmap getter to return a copied instance when _disposeBitmap is true |
src/Controls/tests/TestCases.Shared.Tests/Tests/Issues/Issue29961.cs | Added a UI test to verify the resized image is not disposed |
src/Controls/tests/TestCases.HostApp/Issues/Issue29961.cs | Created a host app page and button for exercising the resize behavior |
src/Controls/tests/TestCases.HostApp/Controls.TestCases.HostApp.csproj | Embedded the royals.png image used by the host app test |
Comments suppressed due to low confidence (1)
src/Graphics/src/Graphics/Platforms/Android/PlatformBitmapExportContext.cs:40
- [nitpick] Cloning the bitmap on every access can be expensive. Consider caching the copied instance or providing an explicit
CloneBitmap()
method to avoid repeated allocations whenBitmap
is accessed multiple times.
public Bitmap Bitmap => _disposeBitmap ? _bitmap.Copy(_bitmap.GetConfig(), false) : _bitmap;
{ | ||
Button button = new Button | ||
{ | ||
AutomationId = $"Issue21886_1ResizeBtn", |
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.
The AutomationId is reused from Issue21886 and may conflict with other tests. Please update it to a unique identifier (e.g., Issue29961_ResizeBtn
) to ensure uniqueness.
Copilot uses AI. Check for mistakes.
@@ -0,0 +1,27 @@ | |||
#if TEST_FAILS_ON_WINDOWS // Issue Link - https://github.com/dotnet/maui/issues/16767 |
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.
The test is conditionally compiled only for Windows (TEST_FAILS_ON_WINDOWS
), which prevents it from executing on Android. Remove or adjust the compilation symbol so the test runs on all relevant platforms.
Copilot uses AI. Check for mistakes.
{ | ||
Button button = new Button | ||
{ | ||
AutomationId = $"Issue21886_1ResizeBtn", |
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.
This AutomationId looks like duplicated, could you use the issue identifier number?
Issue29961_ResizeBtn
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.
@jsuarezruiz , Updated the AutomationId based on the suggestion.
public void VerifyResizeMethodReturnsValidImage() | ||
{ | ||
App.WaitForElement("ConvertedImageStatusLabel"); | ||
App.Tap("Issue21886_1ResizeBtn"); |
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.
Issue29961_ResizeBtn
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.
@jsuarezruiz , Updated the AutomationId based on the suggestion.
Note
Are you waiting for the changes in this PR to be merged?
It would be very helpful if you could test the resulting artifacts from this PR and let us know in a comment if this change resolves your issue. Thank you!
Issue Details
Root Cause
Description of Change
Reference
maui/src/Graphics/src/Graphics/Platforms/iOS/PlatformBitmapExportContext.cs
Lines 41 to 49 in acdb6f2
Issues Fixed
Fixes #29961
Validated the behaviour in the following platforms
Output
Before.mov
After.mov