Skip to content

Fix image flickering #221

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.

Sign up for GitHub

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

Merged
merged 8 commits into from
Nov 26, 2020
Merged

Fix image flickering #221

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
2c14c21
Lazy Load Image
bao-qian Oct 23, 2020
1439ee7
Use a default image to list when the image hasn't been loaded in cache.
taooceros Nov 15, 2020
99086e2
change SetImage to method instead of property to avoid unintended use
taooceros Nov 16, 2020
a7310d5
Use Customized LazyAsync class to load image instead of the weird way…
taooceros Nov 16, 2020
b3df2fd
change default image property name
taooceros Nov 16, 2020
dd8a31c
Add new to Value Property of the LazyAsync clas
taooceros Nov 25, 2020
efcb6a7
add status check when base.Value has faulted
jjw24 Nov 26, 2020
ac945f4
revert unneeded condition on Constant.MissingImgIcon
jjw24 Nov 26, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 5 additions & 7 deletions Flow.Launcher.Infrastructure/Image/ImageCache.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ public class ImageCache
private const int MaxCached = 50;
public ConcurrentDictionary<string, ImageUsage> Data { get; private set; } = new ConcurrentDictionary<string, ImageUsage>();
private const int permissibleFactor = 2;

public void Initialization(Dictionary<string, int> usage)
{
foreach (var key in usage.Keys)
Expand All @@ -44,14 +44,14 @@ public ImageSource this[string path]
value.usage++;
return value.imageSource;
}

return null;
}
set
{
Data.AddOrUpdate(
path,
new ImageUsage(0, value),
path,
new ImageUsage(0, value),
(k, v) =>
{
v.imageSource = value;
Expand All @@ -66,7 +66,6 @@ public ImageSource this[string path]
{
// To delete the images from the data dictionary based on the resizing of the Usage Dictionary.


foreach (var key in Data.OrderBy(x => x.Value.usage).Take(Data.Count - MaxCached).Select(x => x.Key))
{
if (!(key.Equals(Constant.ErrorIcon) || key.Equals(Constant.DefaultIcon)))
Copy link
Member Author

@taooceros taooceros Nov 27, 2020

Choose a reason for hiding this comment

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

Question: Should we keep this two here in resizing check? I think the ErrorIcon should be replaced with MissingIcon, though.

Copy link
Member

@jjw24 jjw24 Nov 27, 2020

Choose a reason for hiding this comment

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

Agree, I think we should replace error with missing. Ideally missing img usage should be low, but I still see it often when using Explorer plugin, so since missing and default imgs are commonly used, we should continue to exclude them from removal. I will make a separate pr just to keep it clean.

Copy link
Member Author

@taooceros taooceros Nov 27, 2020

Choose a reason for hiding this comment

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

I think since they are commonly used, their usage won't be low so that they won't be removed. If they are low, then just removing them won't cause some issue.
Btw, I think we should use Except in Linq to except the key that we don't want to remove, which seems more clear.

Expand All @@ -80,7 +79,7 @@ public ImageSource this[string path]

public bool ContainsKey(string key)
{
var contains = Data.ContainsKey(key);
var contains = Data.ContainsKey(key) && Data[key] != null;
return contains;
}

Expand All @@ -97,5 +96,4 @@ public int UniqueImagesInCache()
return Data.Values.Select(x => x.imageSource).Distinct().Count();
}
}

}
11 changes: 9 additions & 2 deletions Flow.Launcher.Infrastructure/Image/ImageLoader.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@ public static class ImageLoader
private static readonly ConcurrentDictionary<string, string> GuidToKey = new ConcurrentDictionary<string, string>();
private static IImageHashGenerator _hashGenerator;
private static bool EnableImageHash = true;
public static ImageSource DefaultImage { get; } = new BitmapImage(new Uri(Constant.MissingImgIcon));


private static readonly string[] ImageExtensions =
{
Expand Down Expand Up @@ -61,7 +63,7 @@ public static void Save()
{
lock (_storage)
{
_storage.Save(ImageCache.Data.Select(x => (x.Key, x.Value.usage)).ToDictionary(x => x.Key, y => y.usage));
_storage.Save(ImageCache.Data.Select(x => (x.Key, x.Value.usage)).ToDictionary(x => x.Key, x => x.usage));
}
}

Expand Down Expand Up @@ -211,6 +213,11 @@ private static BitmapSource GetThumbnail(string path, ThumbnailOptions option =
option);
}

public static bool CacheContainImage(string path)
{
return ImageCache.ContainsKey(path) && ImageCache[path] != null;
}

public static ImageSource Load(string path, bool loadFullImage = false)
{
var imageResult = LoadInternal(path, loadFullImage);
Expand All @@ -221,7 +228,7 @@ public static ImageSource Load(string path, bool loadFullImage = false)
string hash = EnableImageHash ? _hashGenerator.GetHashFromImage(img) : null;
if (hash != null)
{

if (GuidToKey.TryGetValue(hash, out string key))
{ // image already exists
img = ImageCache[key] ?? img;
Expand Down
2 changes: 1 addition & 1 deletion Flow.Launcher/ResultListBox.xaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@
<ColumnDefinition Width="0" />
</Grid.ColumnDefinitions>
<Image x:Name="ImageIcon" Width="32" Height="32" HorizontalAlignment="Left"
Source="{Binding Image ,IsAsync=True}" />
Source="{Binding Image.Value}" />
<Grid Margin="5 0 5 0" Grid.Column="1" HorizontalAlignment="Stretch">
<Grid.RowDefinitions>
<RowDefinition />
Expand Down
88 changes: 68 additions & 20 deletions Flow.Launcher/ViewModel/ResultViewModel.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,63 +1,112 @@
using System;
using System.Threading.Tasks;
using System.Windows;
using System.Windows.Media;
using System.Windows.Threading;
using Flow.Launcher.Infrastructure;
using Flow.Launcher.Infrastructure.Image;
using Flow.Launcher.Infrastructure.Logger;
using Flow.Launcher.Infrastructure.UserSettings;
using Flow.Launcher.Plugin;


namespace Flow.Launcher.ViewModel
{
public class ResultViewModel : BaseModel
{
public class LazyAsync<T> : Lazy<Task<T>>
{
private T defaultValue;

private readonly Action _updateCallback;
public new T Value
{
get
{
if (!IsValueCreated)
{
base.Value.ContinueWith(_ =>
{
_updateCallback();
});

return defaultValue;
}

if (!base.Value.IsCompleted || base.Value.IsFaulted)
return defaultValue;

return base.Value.Result;
}
}
public LazyAsync(Func<Task<T>> factory, T defaultValue, Action updateCallback) : base(factory)
{
if (defaultValue != null)
{
this.defaultValue = defaultValue;
}

_updateCallback = updateCallback;
}
}

public ResultViewModel(Result result, Settings settings)
{
if (result != null)
{
Result = result;

Image = new LazyAsync<ImageSource>(
SetImage,
ImageLoader.DefaultImage,
() =>
{
OnPropertyChanged(nameof(Image));
});
}

Settings = settings;
}

public Settings Settings { get; private set; }

public Visibility ShowOpenResultHotkey => Settings.ShowOpenResultHotkey ? Visibility.Visible : Visibility.Hidden;
public Visibility ShowOpenResultHotkey => Settings.ShowOpenResultHotkey ? Visibility.Visible : Visibility.Hidden;

public string OpenResultModifiers => Settings.OpenResultModifiers;

public string ShowTitleToolTip => string.IsNullOrEmpty(Result.TitleToolTip)
? Result.Title
? Result.Title
: Result.TitleToolTip;

public string ShowSubTitleToolTip => string.IsNullOrEmpty(Result.SubTitleToolTip)
? Result.SubTitle
? Result.SubTitle
: Result.SubTitleToolTip;

public ImageSource Image
public LazyAsync<ImageSource> Image { get; set; }

private async Task<ImageSource> SetImage()
{
get
var imagePath = Result.IcoPath;
if (string.IsNullOrEmpty(imagePath) && Result.Icon != null)
{
var imagePath = Result.IcoPath;
if (string.IsNullOrEmpty(imagePath) && Result.Icon != null)
try
{
try
{
return Result.Icon();
}
catch (Exception e)
{
Log.Exception($"|ResultViewModel.Image|IcoPath is empty and exception when calling Icon() for result <{Result.Title}> of plugin <{Result.PluginDirectory}>", e);
imagePath = Constant.MissingImgIcon;
}
return Result.Icon();
}
catch (Exception e)
{
Log.Exception($"|ResultViewModel.Image|IcoPath is empty and exception when calling Icon() for result <{Result.Title}> of plugin <{Result.PluginDirectory}>", e);
imagePath = Constant.MissingImgIcon;
}

}

if (ImageLoader.CacheContainImage(imagePath))
{
// will get here either when icoPath has value\icon delegate is null\when had exception in delegate
return ImageLoader.Load(imagePath);
}
else
{
return await Task.Run(() => ImageLoader.Load(imagePath));
}
}

public Result Result { get; }
Expand All @@ -84,6 +133,5 @@ public override string ToString()
{
return Result.ToString();
}

}
}