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

[Android] Fix gif animation initial state #14295

Merged
merged 42 commits into from
Jan 26, 2024
Merged

[Android] Fix gif animation initial state #14295

merged 42 commits into from
Jan 26, 2024

Conversation

jsuarezruiz
Copy link
Contributor

@jsuarezruiz jsuarezruiz commented Mar 30, 2023

Description of Change

Fix gif animation initial state.

To validate the changes, launch the .NET MAUI Gallery and navigate to the Image samples or, use the existing related tests previously ignored and now passing.

fix-7019
image

Issues Fixed

Fixes #7019

@jsuarezruiz jsuarezruiz added t/bug Something isn't working platform/android 🤖 area-controls-image Image control labels Mar 30, 2023
@ghost ghost added the legacy-area-controls Label, Button, CheckBox, Slider, Stepper, Switch, Picker, Entry, Editor label Mar 30, 2023
mandel-macaque
mandel-macaque previously approved these changes Mar 30, 2023
@github-actions
Copy link
Contributor

Thank you for your pull request. We are auto-formatting your source code to follow our code guidelines.

@jsuarezruiz jsuarezruiz removed the stale Indicates a stale issue/pr and will be closed soon label Dec 12, 2023
@samhouts samhouts added the stale Indicates a stale issue/pr and will be closed soon label Jan 8, 2024
@jsuarezruiz jsuarezruiz removed the stale Indicates a stale issue/pr and will be closed soon label Jan 25, 2024
@@ -48,8 +48,13 @@ public static void MapBackground(IImageHandler handler, IImage image)
public static void MapSource(IImageHandler handler, IImage image) =>
MapSourceAsync(handler, image).FireAndForget(handler);

public static Task MapSourceAsync(IImageHandler handler, IImage image) =>
handler.SourceLoader.UpdateImageSourceAsync();
public static Task MapSourceAsync(IImageHandler handler, IImage image)
Copy link
Member

Choose a reason for hiding this comment

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

Should we just make this an async Task and then await each? Not sure if this may have issues with the ContinueWith... Are we sure the threads will be correct when the source finishes?

Copy link
Member

@mattleibow mattleibow left a comment

Choose a reason for hiding this comment

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

I also noticed this issue on Windows as it also is the case of the image itself, not the view, that is doing the animating. Should we fix that as well?

Also, when copying the ContinueWith code on Windows I got a crash about threads. Should we just await the tasks instead of ContinueWith.

Also, how is the device test passing on Windows if it is actually broken...

@mattleibow
Copy link
Member

I see windows does have this: https://github.com/dotnet/maui/blob/main/src/Core/src/Platform/Windows/ImageSourcePartExtensions.cs#L46

Should we consolidate in either the image setter or the handler? I like the handler because that is more extensible/custmisable? @PureWeen

@mattleibow
Copy link
Member

Hmmm, maybe windows is separate since it reads gif as NOT animated:

image

@mattleibow
Copy link
Member

Let me try force playing...

@mattleibow
Copy link
Member

mattleibow commented Jan 25, 2024

Looking at the code, Android does go onto another thread and somehow it still works... We need to await I think to avoid potential threading issues. We can fix Windows separately. Even calling Stop directly does not do anything.

@mattleibow
Copy link
Member

This is why it passes on Windows... They are not even there!
image

OK, Windows is a totally separate thing with all sorts of issues.

@mattleibow mattleibow dismissed their stale review January 26, 2024 02:04

Windows is separate

@mattleibow mattleibow merged commit dbf7aad into main Jan 26, 2024
47 checks passed
@mattleibow mattleibow deleted the fix-7019 branch January 26, 2024 02:05
@mattleibow
Copy link
Member

I have fixed this issue on Windows in #20169 and also enavledenabled the tests in #20167

rmarinho added a commit that referenced this pull request Jan 29, 2024
* Add Obsolete tag for old IndexOf (#20068)

* Split the InputTransparency tests (#19925)

* Bump Xamarin.UITest to 4.3.4 (#20067)

* Bump Xamarin.UITest to 4.3.4

* Update NUnit

* Restore dotnet tool (#20106)

* Do not use underscores in the ApplicationId (#19377)

* Do not use underscores in the ApplicationId

Underscores are not supported on Windows

* Update DotnetInternal.cs

* [WinUI] Add workaround for Connectivity check on Win10  (#19261)

* Implemented a Win10 work-around for connectivity thread issue

Reimplement `ConnectivityImplementation.ConnectionProfiles` to use native .net core APIs

* Remove prop bag ID

* Replace network availability changed event with native .net API

* Remove explicit file include

* Fix file naming

* Implemented a Win10 work-around for connectivity thread issue

Reimplement `ConnectivityImplementation.ConnectionProfiles` to use native .net core APIs

* Remove prop bag ID

* Replace network availability changed event with native .net API

* Remove explicit file include

* Fix file naming

* * Revert change to remove `Windows.Networking.Connectivity.NetworkInformation.NetworkStatusChanged`

* Update Connectivity.uwp.cs

---------

Co-authored-by: Mike Corsaro <mikecorsaro@microsoft.com>
Co-authored-by: Matthew Leibowitz <mattleibow@live.com>

* [Windows] Adjust recycle check in ItemContentControl (#20079)

* Adjust recycle check in `ItemContentControl` to use Content property

* Fix confusing line

* Rectify the scopes in the builder (#19932)

* Update System.Drawing.Common (#20122)

* Enable building WASDK Self-Contained packaged apps (#20019)

* Translucent and Transparent NavigationBar on iOS (#19204)

* Changes for enabling translucent navigation bar ios

* Add UITest for NavigationPage Safe Area Translucence

* remove UIKit

* Move UITest from gallery to Issues

* push a new page for UITests, consolidate code, and save shadowimage

* Changes for enabling translucent navigation bar ios

* Add UITest for NavigationPage Safe Area Translucence

* remove UIKit

* Move UITest from gallery to Issues

* make the extension method

* use the background color alpha for translucent

* use same alpha logic for pre ios 15

* accidently added testing comments

* add more UITests for the different NavigationPage and Flyout Page scenarios

* use additionalsafeareainsets for the secondary toolbar

* missing new line

* only run the secondary toolbar offset when we have a secondary toolbar

* use existing childViewControllers

* remove the shadow stuff and simplify the expression with null

* style and fixes from merge conflicts

* standardAppearance and scrolledgeappearance should be kept in

* changes after talking with Shane

* change shell behavior to be more like navrenderer with preiOS13

* move code if we are transparent pre13 shell

* remove new lines

* add screenshot tests

* be able to remove and add secondarybar additionalsafeareas

* reset the xaml on the Sandboc MainPage (#20082)

* [Android] Fix gif animation initial state (#14295)

* Fix gifs initial animation state on Android

* Device tests

* Auto-format source code

* Updated sample

* Updated device tests

* Refactor code

* Update src/Core/src/Handlers/Image/ImageHandler.Android.cs

Co-authored-by: Matthew Leibowitz <mattleibow@live.com>

* Remove unnecessary change

* Fix build errors

* Merge branch 'main' into fix-7019

* Fix merge mistake

---------

Co-authored-by: GitHub Actions Autoformatter <autoformat@example.com>
Co-authored-by: Javier Suárez <6755973+jsuarezruiz@users.noreply.github.com>
Co-authored-by: Matthew Leibowitz <mattleibow@live.com>
Co-authored-by: Gerald Versluis <gerald.versluis@microsoft.com>

* Enable Windows Image device tests (#20167)

* [X] allow x:Type extension for BPConverter (#18540)

- fixes #18324

* Source Generator Performance Improvements (#19990)

* Source Generator Performance Improvements:
- Reversed lookup order (Extension second)
- Introduced type cache

```
PERF PROGRESS:
SourceGen - Maui.Controls.Sample (262 XAML files)

1) Unchanged:
- 15640 GetTypeByMetadata calls
- 223s

2) Extensions lookup in XmlTypeXamlExtensions.GetTypeReference() second
- 6232 GetTypeByMetadata calls (~60% reduction)
- 90s                          (~60% reduction)

3) With type cache
- 203 GetTypeByMetadata calls (~97% reduction => ~99% total reduction)
- 6s                          (~93% reduction => ~97% total reduction => 37 times faster!)
```

* - Set uinitial lookupNames capacity to 2 since there won't be more than 2

* Added appium UITest for FlyoutNavigationBetweenItemsWithNavigationStacks (#19444) Fixes #16675

* Add better exception for missing Maps on Windows (#19046)

* Add better exception for missing Maps on Windows

* Update AppHostBuilderExtensions.cs

* [ci] Add missing demands (#20183)

* Add comments to internal methods for XAML Hot Reload usage (#20215)

* Add comments to internal methods for XAML Hot Reload usage

* spacing

* Adding Mobile tag to MAUI project templates (#20191)

Co-authored-by: Luke Westendorf <lukewest@microsoft.com>

---------

Co-authored-by: Tim Miller <drasticactions@users.noreply.github.com>
Co-authored-by: Matthew Leibowitz <mattleibow@live.com>
Co-authored-by: Gerald Versluis <gerald.versluis@microsoft.com>
Co-authored-by: Mike Corsaro <corsarom@gmail.com>
Co-authored-by: Mike Corsaro <mikecorsaro@microsoft.com>
Co-authored-by: TJ Lambert <50846373+tj-devel709@users.noreply.github.com>
Co-authored-by: Javier Suárez <javiersuarezruiz@hotmail.com>
Co-authored-by: GitHub Actions Autoformatter <autoformat@example.com>
Co-authored-by: Javier Suárez <6755973+jsuarezruiz@users.noreply.github.com>
Co-authored-by: Stephane Delcroix <stephane@delcroix.org>
Co-authored-by: Marco Goertz <mgoertz@microsoft.com>
Co-authored-by: MSLukeWest <42553283+MSLukeWest@users.noreply.github.com>
Co-authored-by: Luke Westendorf <lukewest@microsoft.com>
PureWeen pushed a commit that referenced this pull request Feb 8, 2024
* Fix gifs initial animation state on Android

* Device tests

* Auto-format source code

* Updated sample

* Updated device tests

* Refactor code

* Update src/Core/src/Handlers/Image/ImageHandler.Android.cs

Co-authored-by: Matthew Leibowitz <mattleibow@live.com>

* Remove unnecessary change

* Fix build errors

* Merge branch 'main' into fix-7019

* Fix merge mistake

---------

Co-authored-by: GitHub Actions Autoformatter <autoformat@example.com>
Co-authored-by: Javier Suárez <6755973+jsuarezruiz@users.noreply.github.com>
Co-authored-by: Matthew Leibowitz <mattleibow@live.com>
Co-authored-by: Gerald Versluis <gerald.versluis@microsoft.com>
@github-actions github-actions bot locked and limited conversation to collaborators Feb 25, 2024
@PureWeen PureWeen modified the milestones: Under Consideration, .NET 8 SR4 Mar 11, 2024
@Eilon Eilon removed the legacy-area-controls Label, Button, CheckBox, Slider, Stepper, Switch, Picker, Entry, Editor label May 10, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Animated gifs do not start initially
7 participants