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

Fix android image loading race condition and improve logging #25870

Merged
merged 8 commits into from
Dec 23, 2024

Conversation

albyrock87
Copy link
Contributor

Description of Change

This PR aims to solve three issues:

  1. Switching between Shell Tabs destroys the Fragment causing unwanted image unloading:
    Reloading images upon re-attach was handled in Check to see if Image Should Reload When Attached #24023 but we can provide a better UX here and avoid flashes while going back to the initial tab.
  2. Random flashes while scrolling collection view caused by a race condition in image loading:
    Glide is race-condition free, but we were setting the Drawable asynchronously in the next loop, this may generate glitches under some scrolling condition in collection view
  3. It's really hard to debug our beloved Glide integration:
    I've added proper logging mechanics which can be enabled via adb before running the application:
    adb shell setprop log.tag.Glide VERBOSE
    adb shell setprop log.tag.MauiCustomTarget VERBOSE
    adb shell setprop log.tag.MauiCustomViewTarget VERBOSE
    
    Example logs:
    D Glide   : Finished loading BitmapDrawable from MEMORY_CACHE for FontModel{color=#FF000000, glyph='f111', textSize=84.0, typeface=android.graphics.Typeface@898fd220} with size [-2147483648x-2147483648] in 0.202625 ms
    V MauiCustomViewTarget: onResourceReady: FontModel{color=#FF000000, glyph='f111', textSize=84.0, typeface=android.graphics.Typeface@898fd220}
    

Issues Fixed

#14587
#25783

@albyrock87 albyrock87 requested a review from a team as a code owner November 14, 2024 20:49
@dotnet-policy-service dotnet-policy-service bot added the community ✨ Community Contribution label Nov 14, 2024
@jsuarezruiz jsuarezruiz added the area-image Image loading, sources, caching label Nov 15, 2024
@jsuarezruiz
Copy link
Contributor

/azp run

Copy link

Azure Pipelines successfully started running 3 pipeline(s).

@albyrock87 albyrock87 force-pushed the android-image-loading-race-condition branch from cea2228 to 9498be1 Compare November 17, 2024 09:51
@rmarinho
Copy link
Member

/rebase

@github-actions github-actions bot force-pushed the android-image-loading-race-condition branch from 9498be1 to 5d2eda3 Compare November 18, 2024 00:30
@rmarinho
Copy link
Member

/azp run

Copy link

Azure Pipelines successfully started running 3 pipeline(s).

Copy link
Member

@PureWeen PureWeen left a comment

Choose a reason for hiding this comment

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

@albyrock87 albyrock87 force-pushed the android-image-loading-race-condition branch from 5d2eda3 to 7543596 Compare November 19, 2024 09:08
albyrock87 added a commit to nalu-development/maui-custom that referenced this pull request Nov 19, 2024
@rmarinho
Copy link
Member

/rebase

@github-actions github-actions bot force-pushed the android-image-loading-race-condition branch from 360e4d5 to c2db931 Compare November 19, 2024 16:25
@rmarinho
Copy link
Member

/azp run

Copy link

Azure Pipelines successfully started running 3 pipeline(s).

@albyrock87 albyrock87 force-pushed the android-image-loading-race-condition branch from 5f5a957 to f3cf294 Compare November 20, 2024 12:22
@PureWeen
Copy link
Member

/azp run

Copy link

Azure Pipelines successfully started running 3 pipeline(s).

@PureWeen
Copy link
Member

/azp run

Copy link

Azure Pipelines successfully started running 3 pipeline(s).

@PureWeen
Copy link
Member

/azp run

Copy link

Azure Pipelines successfully started running 3 pipeline(s).

@PureWeen
Copy link
Member

/azp run

Copy link

Azure Pipelines successfully started running 3 pipeline(s).

@PureWeen
Copy link
Member

/azp run

Copy link

Azure Pipelines successfully started running 3 pipeline(s).

Copy link
Member

@jonathanpeppers jonathanpeppers left a comment

Choose a reason for hiding this comment

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

The logging changes look good now. 👍

But someone will need to manually test that it fixes: "android image loading race condition"

@PureWeen
Copy link
Member

  • failing test is unrelated

@PureWeen PureWeen merged commit 06af84d into dotnet:main Dec 23, 2024
102 of 104 checks passed
@jfversluis jfversluis added this to the .NET 9 SR3 milestone Jan 6, 2025
@github-actions github-actions bot locked and limited conversation to collaborators Feb 5, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-image Image loading, sources, caching community ✨ Community Contribution platform/android 🤖
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants