Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
{
"type": "prerelease",
"comment": "Fire onLoad event when a bitmap image is opeed",
"packageName": "react-native-windows",
"email": "[email protected]",
"commit": "22edad7ea69232ae7636424c237df4eee7577f54",
"dependentChangeType": "patch",
"date": "2020-04-29T22:37:41.642Z"
}
37 changes: 20 additions & 17 deletions vnext/ReactUWP/Views/Image/ReactImage.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -213,29 +213,15 @@ winrt::fire_and_forget ReactImage::SetBackground(bool fireLoadEndEvent) {
if (createImageBrush) {
imageBrush = winrt::ImageBrush{};

// ImageOpened and ImageFailed are mutually exclusive. One event of the other will
// always fire whenever an ImageBrush has the ImageSource value set or reset.
strong_this->m_imageBrushOpenedRevoker = imageBrush.ImageOpened(
winrt::auto_revoke, [weak_this, imageBrush, fireLoadEndEvent](const auto &, const auto &) {
strong_this->m_imageBrushOpenedRevoker =
imageBrush.ImageOpened(winrt::auto_revoke, [weak_this, imageBrush](const auto &, const auto &) {
if (auto strong_this{weak_this.get()}) {
if (auto bitmap{imageBrush.ImageSource().try_as<winrt::BitmapImage>()}) {
strong_this->m_imageSource.height = bitmap.PixelHeight();
strong_this->m_imageSource.width = bitmap.PixelWidth();
}

imageBrush.Stretch(strong_this->ResizeModeToStretch(strong_this->m_resizeMode));

if (fireLoadEndEvent) {
strong_this->m_onLoadEndEvent(*strong_this, true);
}
}
});

strong_this->m_imageBrushFailedRevoker =
imageBrush.ImageFailed(winrt::auto_revoke, [weak_this, fireLoadEndEvent](const auto &, const auto &) {
const auto strong_this{weak_this.get()};
if (strong_this && fireLoadEndEvent) {
strong_this->m_onLoadEndEvent(*strong_this, false);
}
});
}
Expand Down Expand Up @@ -274,7 +260,24 @@ winrt::fire_and_forget ReactImage::SetBackground(bool fireLoadEndEvent) {
bitmapImage = winrt::BitmapImage{};

strong_this->m_bitmapImageOpened = bitmapImage.ImageOpened(
winrt::auto_revoke, [imageBrush](const auto &, const auto &) { imageBrush.Opacity(1); });
winrt::auto_revoke, [imageBrush, weak_this, fireLoadEndEvent](const auto &, const auto &) {
imageBrush.Opacity(1);

auto strong_this{weak_this.get()};
if (strong_this && fireLoadEndEvent) {
strong_this->m_onLoadEndEvent(*strong_this, true);
Copy link
Contributor

Choose a reason for hiding this comment

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

m_onLoadEndEvent [](start = 31, length = 16)

This code path confuses me. It runs when we have ImageBrush already set, but this ImageBrush doesn't have a BitmapImage. @marlenecota what scenario is this code path handling? If we really do need some extra lagic here, it seems like we also need to listen for ImageFailed.

Copy link
Contributor

Choose a reason for hiding this comment

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

This codepath was needed for when you change the dynamically change the image source.
Original PR: https://github.com/microsoft/react-native-windows/pull/3712/files#r358397245
Flicker tracking issue: #3794

If we really do need to fire the loadEnd event here, then I agree ImageFailed is probably needed as well.

@lamxdoan what's the repro code?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@marlenecota we had an image component that would listen on the image load event and trigger an animation set opacity from 0 to 1. We noticed in some cases when we updated the imageUrl that it didn't fire the load event.

Added ImageFailed now.

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks!


In reply to: 418365658 [](ancestors = 418365658)

}
});

strong_this->m_bitmapImageFailed = bitmapImage.ImageFailed(
winrt::auto_revoke, [imageBrush, weak_this, fireLoadEndEvent](const auto &, const auto &) {
imageBrush.Opacity(1);

auto strong_this{weak_this.get()};
if (strong_this && fireLoadEndEvent) {
strong_this->m_onLoadEndEvent(*strong_this, false);
}
});

imageBrush.ImageSource(bitmapImage);
}
Expand Down
2 changes: 1 addition & 1 deletion vnext/ReactUWP/Views/Image/ReactImage.h
Original file line number Diff line number Diff line change
Expand Up @@ -67,8 +67,8 @@ struct ReactImage : xaml::Controls::GridT<ReactImage> {
xaml::FrameworkElement::SizeChanged_revoker m_sizeChangedRevoker;
xaml::Media::LoadedImageSurface::LoadCompleted_revoker m_surfaceLoadedRevoker;
xaml::Media::Imaging::BitmapImage::ImageOpened_revoker m_bitmapImageOpened;
xaml::Media::Imaging::BitmapImage::ImageFailed_revoker m_bitmapImageFailed;
xaml::Media::ImageBrush::ImageOpened_revoker m_imageBrushOpenedRevoker;
xaml::Media::ImageBrush::ImageFailed_revoker m_imageBrushFailedRevoker;
xaml::Media::Imaging::SvgImageSource::Opened_revoker m_svgImageSourceOpenedRevoker;
xaml::Media::Imaging::SvgImageSource::OpenFailed_revoker m_svgImageSourceOpenFailedRevoker;
};
Expand Down