Closed timothyparez closed 3 years ago
Thank you.
Rather than adding another redundant property, wouldn't it be better if IsAsync
itself is overridden with this behavior?
Well, to be honest, I'm not sure how. But even so. The LoadAsync property and the IsAsync property on the binding offer different behaviors, they are not exactly the same.
Let's say we have have the following:
<c:Image ImageUrl="http://foo.com/1.jpg"/>
<c:Image ImageUrl="http://foo.com/1.jpg"/>
<c:Image ImageUrl="http://foo.com/1.jpg"/>
When IsAsync
is not set, WPF will not execute the binding for the second
image until the first one completed.
When IsAsync
is set to true
, WPF will execute the first one and then
the second one immediately without waiting for the result of the first one.
This behavior is not broken in your original code (unlike what I expected when I filed the bug),
it does exactly what it's supposed to do. The problem is that it will execute the binding for
the first image which immediately freezes the UI thread. As a result, even though IsAsync
is set to true
the second binding will not be executed until the first one completed and
the UI remains unresponsive. (The image loading occurs in the UI thread)
LoadAsync
refers to the fact that the image is being loaded asynchronously while
IsAsync
refers to the fact that the binding itself is being executed asynchronously.
Ideally the Image class should have loaded the image asynchronously from the start, but since it didn't I figured there were only two options:
LoadAsync
propertyI figured it might be best not to break it for existing users.
I hope this makes some sense :-)
Thank you for explaining the thought process, @timothyparez
There was another pull request that got merged after this one was submitted, so there are some minor tweaks we need to make to this one to consider the new Cache Mode
that was added via the other pull request - please see #5 for the details.
And the way I see it, since LoadAsync
is essentially doing what IsAsync
should have done in the first place, this pull request is more of a fix to that original issue and we do not need to retain backward compatibility. So I think we should override the IsAsync
property from within Image.cs
so that the images are loaded asynchronously.
Considering we now have two cache modes from the pull request #5, I think we should also check for FileCache.AppCacheMode
to be set to CacheMode.Dedicated
when IsAsync
is true, to call the asynchronous loading of the images from either the network or from the local cache folder.
As I had mentioned in #5, I hadn't tested the changes that added two cache modes (WinINet
and Dedicated
) when I merged it in. But I tested it now and it seems to be working fine with the WinINet
mode using the IE cache directory to do all the caching by itself.
Coming back to the asynchronous loading of images - I also tested the Dedicated
cache mode, which is the old/original, filesystem-based caching (that this control had as its only caching mode when this pull request was opened) using an older app that I am not maintaining anymore. And it looks like the IsAsync
setting on the Binding
does seem to be working fine without blocking the UI thread. The images do take a while to get downloaded and show up on the window, especially when the Images per page is bumped up all the way to 100, but it seems to be doing that without blocking the UI.
If you have a flickr account you could probably take a look at the code in the cachedimage-test
branch there and see if there is any difference from your app where you're experiencing UI-blocking issues.
Setting this property to true prevents the UI thread from freezing (For example when loading large images or over a slow connection)
Notes: