From 966135e72d05b94ae977748a9e95c63d2c1f77ca Mon Sep 17 00:00:00 2001 From: Marlene Cota Date: Tue, 26 Nov 2019 19:48:03 -0800 Subject: [PATCH 01/17] Use BitmapImage for cover, contain, and stretch resizeModes --- packages/playground/Samples/image.tsx | 7 +- vnext/ReactUWP/Views/Image/ReactImage.cpp | 154 +++++++++++++++--- vnext/ReactUWP/Views/Image/ReactImage.h | 14 +- .../ReactUWP/Views/Image/ReactImageBrush.cpp | 6 +- vnext/ReactUWP/Views/Image/ReactImageBrush.h | 6 +- 5 files changed, 151 insertions(+), 36 deletions(-) diff --git a/packages/playground/Samples/image.tsx b/packages/playground/Samples/image.tsx index aac93924745..0bbd20ae7c4 100644 --- a/packages/playground/Samples/image.tsx +++ b/packages/playground/Samples/image.tsx @@ -27,9 +27,10 @@ export default class Bootstrap extends React.Component< } > { state = { - selectedResizeMode: 'center' as 'center', - useLargeImage: false, - imageUrl: 'http://facebook.github.io/react-native/img/header_logo.png', + selectedResizeMode: 'contain' as 'contain', + useLargeImage: true, + imageUrl: + 'https://cdn.freebiesupply.com/logos/large/2x/react-logo-png-transparent.png', }; switchImageUrl = () => { diff --git a/vnext/ReactUWP/Views/Image/ReactImage.cpp b/vnext/ReactUWP/Views/Image/ReactImage.cpp index 24ab84f66af..69da1b92181 100644 --- a/vnext/ReactUWP/Views/Image/ReactImage.cpp +++ b/vnext/ReactUWP/Views/Image/ReactImage.cpp @@ -14,7 +14,9 @@ namespace winrt { using namespace Windows::Foundation; using namespace Windows::Storage::Streams; using namespace Windows::UI; +using namespace Windows::UI::Xaml; using namespace Windows::UI::Xaml::Media; +using namespace Windows::UI::Xaml::Media::Imaging; using namespace Windows::Web::Http; } // namespace winrt @@ -29,8 +31,31 @@ namespace react { namespace uwp { ReactImage::ReactImage() { - m_brush = ReactImageBrush::Create(); - this->Background(m_brush.as()); + if (m_useCompositionBrush) { + m_brush = ReactImageBrush::Create(); + this->Background(m_brush.as()); + } else { + m_bitmapBrush.Stretch(winrt::Stretch::Uniform); + this->Background(m_bitmapBrush); + + // auto weak_this{get_weak()}; + + //// ImageOpened and ImageFailed are mutually exclusive. One event of the other will + //// always fire whenever an ImageBrush has the ImageSource value set or reset. + // m_imageOpenedRevoker = + // m_bitmapBrush.ImageOpened(winrt::auto_revoke, [weak_this](const auto &, const auto &) { + // if (auto strong_this{weak_this.get()}) { + // strong_this->m_onLoadEndEvent(*strong_this, true); + // } + // }); + + // m_imageFailedRevoker = + // m_bitmapBrush.ImageFailed(winrt::auto_revoke, [weak_this](const auto &, const auto &) { + // if (auto strong_this{weak_this.get()}) { + // strong_this->m_onLoadEndEvent(*strong_this, false); + // } + // }); + } } /*static*/ winrt::com_ptr ReactImage::Create() { @@ -38,8 +63,10 @@ ReactImage::ReactImage() { } winrt::Size ReactImage::ArrangeOverride(winrt::Size finalSize) { - auto brush{Background().as()}; - brush->AvailableSize(finalSize); + if (m_useCompositionBrush) { + auto brush{Background().as()}; + brush->AvailableSize(finalSize); + } return finalSize; } @@ -52,6 +79,58 @@ void ReactImage::OnLoadEnd(winrt::event_token const &token) noexcept { m_onLoadEndEvent.remove(token); } +void ReactImage::ResizeMode(react::uwp::ResizeMode value) { + if (m_resizeMode != value) { + m_resizeMode = value; + + bool switchBrushes{false}; + if (m_useCompositionBrush != ShouldUseCompositionBrush()) { + switchBrushes = true; + m_useCompositionBrush = ShouldUseCompositionBrush(); + } + + if (!m_useCompositionBrush) { + switch (m_resizeMode) { + case ResizeMode::Cover: + m_bitmapBrush.Stretch(winrt::Stretch::UniformToFill); + break; + case ResizeMode::Stretch: + m_bitmapBrush.Stretch(winrt::Stretch::Fill); + break; + default: + m_bitmapBrush.Stretch(winrt::Stretch::Uniform); + break; + } + } else { + if (!m_brush) { + m_brush = ReactImageBrush::Create(); + } + + m_brush->ResizeMode(value); + } + + if (switchBrushes) { + Source(m_imageSource); + + if (m_useCompositionBrush) { + this->Background(m_brush.as()); + } else { + this->Background(m_bitmapBrush); + } + } + } +} + +bool ReactImage::ShouldUseCompositionBrush() { + switch (m_resizeMode) { + case ResizeMode::Repeat: + case ResizeMode::Center: + return true; + default: + return false; + } +} + winrt::fire_and_forget ReactImage::Source(ImageSource source) { std::string uriString{source.uri}; if (uriString.length() == 0) { @@ -86,29 +165,58 @@ winrt::fire_and_forget ReactImage::Source(ImageSource source) { } if (!needsDownload || memoryStream) { - auto surface = needsDownload || inlineData ? winrt::LoadedImageSurface::StartLoadFromStream(memoryStream) - : winrt::LoadedImageSurface::StartLoadFromUri(uri); - - strong_this->m_surfaceLoadedRevoker = surface.LoadCompleted( - winrt::auto_revoke, - [weak_this, surface]( - winrt::LoadedImageSurface const & /*sender*/, - winrt::LoadedImageSourceLoadCompletedEventArgs const &args) { - if (auto strong_this{weak_this.get()}) { - bool succeeded{false}; - if (args.Status() == winrt::LoadedImageSourceLoadStatus::Success) { - strong_this->m_brush->Source(surface); - succeeded = true; - } - strong_this->m_onLoadEndEvent(*strong_this, succeeded); + if (m_useCompositionBrush) { + auto surface = needsDownload || inlineData ? winrt::LoadedImageSurface::StartLoadFromStream(memoryStream) + : + winrt::LoadedImageSurface::StartLoadFromUri(uri); + + strong_this->m_surfaceLoadedRevoker = surface.LoadCompleted( + winrt::auto_revoke, + [weak_this, surface]( + winrt::LoadedImageSurface const & /*sender*/, + winrt::LoadedImageSourceLoadCompletedEventArgs const &args) { + if (auto strong_this{weak_this.get()}) { + bool succeeded{false}; + if (args.Status() == winrt::LoadedImageSourceLoadStatus::Success) { + strong_this->m_brush->Source(surface); + succeeded = true; } - }); + strong_this->m_onLoadEndEvent(*strong_this, succeeded); + } + }); + } + else { + winrt::BitmapImage bitmap; + + // 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_imageOpenedRevoker = + bitmap.ImageOpened(winrt::auto_revoke, [weak_this, bitmap](const auto &, const auto &) { + if (auto strong_this{weak_this.get()}) { + strong_this->m_bitmapBrush.ImageSource(bitmap); + strong_this->m_onLoadEndEvent(*strong_this, true); + } + }); + + strong_this->m_imageFailedRevoker = bitmap.ImageFailed(winrt::auto_revoke, [weak_this](const auto &, const auto &) { + if (auto strong_this{weak_this.get()}) { + strong_this->m_onLoadEndEvent(*strong_this, false); } + }); + + if (needsDownload || inlineData) { + co_await bitmap.SetSourceAsync(memoryStream); + } else { + bitmap = {uri}; } - } catch (winrt::hresult_error const &) { - if (auto strong_this{weak_this.get()}) - strong_this->m_onLoadEndEvent(*strong_this, false); } +} +} // namespace uwp +} // namespace react +catch (winrt::hresult_error const &) { + if (auto strong_this{weak_this.get()}) + strong_this->m_onLoadEndEvent(*strong_this, false); +} } // namespace uwp winrt::IAsyncOperation GetImageStreamAsync(ImageSource source) { diff --git a/vnext/ReactUWP/Views/Image/ReactImage.h b/vnext/ReactUWP/Views/Image/ReactImage.h index 77563ecde16..241e934d55e 100644 --- a/vnext/ReactUWP/Views/Image/ReactImage.h +++ b/vnext/ReactUWP/Views/Image/ReactImage.h @@ -8,6 +8,7 @@ #include #include #include +#include #include #include @@ -49,17 +50,22 @@ struct ReactImage : winrt::Windows::UI::Xaml::Controls::CanvasT { winrt::fire_and_forget Source(ImageSource source); react::uwp::ResizeMode ResizeMode() { - return m_brush->ResizeMode(); - } - void ResizeMode(react::uwp::ResizeMode value) { - m_brush->ResizeMode(value); + return m_resizeMode; } + void ResizeMode(react::uwp::ResizeMode value); private: + bool ShouldUseCompositionBrush(); + + bool m_useCompositionBrush = false; ImageSource m_imageSource; + winrt::Windows::UI::Xaml::Media::ImageBrush m_bitmapBrush{}; winrt::com_ptr m_brush; + react::uwp::ResizeMode m_resizeMode{ResizeMode::Contain}; winrt::event> m_onLoadEndEvent; winrt::Windows::UI::Xaml::Media::LoadedImageSurface::LoadCompleted_revoker m_surfaceLoadedRevoker; + winrt::Windows::UI::Xaml::Media::Imaging::BitmapImage::ImageOpened_revoker m_imageOpenedRevoker; + winrt::Windows::UI::Xaml::Media::Imaging::BitmapImage::ImageFailed_revoker m_imageFailedRevoker; }; // Helper functions diff --git a/vnext/ReactUWP/Views/Image/ReactImageBrush.cpp b/vnext/ReactUWP/Views/Image/ReactImageBrush.cpp index ae030f63f3e..d700cca1760 100644 --- a/vnext/ReactUWP/Views/Image/ReactImageBrush.cpp +++ b/vnext/ReactUWP/Views/Image/ReactImageBrush.cpp @@ -70,7 +70,7 @@ void ReactImageBrush::UpdateCompositionBrush() { surfaceBrush.Stretch(ResizeModeToStretch()); auto compositionBrush{surfaceBrush.as()}; - if (ResizeMode() == ResizeMode::Repeat) { + if (m_resizeMode == ResizeMode::Repeat) { // If ResizeMode is set to Repeat, then we need to use a // CompositionEffectBrush. The CompositionSurfaceBrush holding the image // is used as its source. @@ -81,7 +81,7 @@ void ReactImageBrush::UpdateCompositionBrush() { // and anytime we switch between Surface and Effect brushes (to/from // ResizeMode::Repeat) if (CompositionBrush() != compositionBrush) { - if (ResizeMode() == ResizeMode::Repeat) { + if (m_resizeMode == ResizeMode::Repeat) { surfaceBrush.HorizontalAlignmentRatio(0.0f); surfaceBrush.VerticalAlignmentRatio(0.0f); } else { @@ -108,7 +108,7 @@ bool ReactImageBrush::IsImageSmallerThanView() { winrt::CompositionStretch ReactImageBrush::ResizeModeToStretch() { auto stretch{winrt::CompositionStretch::None}; - switch (ResizeMode()) { + switch (m_resizeMode) { case ResizeMode::Contain: stretch = winrt::CompositionStretch::Uniform; break; diff --git a/vnext/ReactUWP/Views/Image/ReactImageBrush.h b/vnext/ReactUWP/Views/Image/ReactImageBrush.h index 8bc3b98bc51..bc5c7c1a0c9 100644 --- a/vnext/ReactUWP/Views/Image/ReactImageBrush.h +++ b/vnext/ReactUWP/Views/Image/ReactImageBrush.h @@ -25,9 +25,9 @@ struct ReactImageBrush : winrt::Windows::UI::Xaml::Media::XamlCompositionBrushBa void OnDisconnected(); // Public Properties - react::uwp::ResizeMode ResizeMode() { - return m_resizeMode; - } + //react::uwp::ResizeMode ResizeMode() { + // return m_resizeMode; + //} void ResizeMode(react::uwp::ResizeMode value); winrt::Windows::Foundation::Size AvailableSize() { From d119b33a7bbb22dc525271df50f7fbd9368fcb3e Mon Sep 17 00:00:00 2001 From: Marlene Cota Date: Wed, 27 Nov 2019 02:10:49 -0800 Subject: [PATCH 02/17] Fix comments --- vnext/ReactUWP/Views/Image/ReactImageBrush.cpp | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/vnext/ReactUWP/Views/Image/ReactImageBrush.cpp b/vnext/ReactUWP/Views/Image/ReactImageBrush.cpp index d700cca1760..320cd972998 100644 --- a/vnext/ReactUWP/Views/Image/ReactImageBrush.cpp +++ b/vnext/ReactUWP/Views/Image/ReactImageBrush.cpp @@ -71,15 +71,13 @@ void ReactImageBrush::UpdateCompositionBrush() { auto compositionBrush{surfaceBrush.as()}; if (m_resizeMode == ResizeMode::Repeat) { - // If ResizeMode is set to Repeat, then we need to use a - // CompositionEffectBrush. The CompositionSurfaceBrush holding the image - // is used as its source. + // If ResizeMode is set to Repeat, then we need to use a CompositionEffectBrush. + // The CompositionSurfaceBrush holding the image is used as its source. compositionBrush = GetOrCreateEffectBrush(surfaceBrush); } - // The CompositionBrush is only set after the image is first loaded, - // and anytime we switch between Surface and Effect brushes (to/from - // ResizeMode::Repeat) + // The CompositionBrush is only set after the image is first loaded and anytime + // we switch between Surface and Effect brushes (to/from ResizeMode::Repeat) if (CompositionBrush() != compositionBrush) { if (m_resizeMode == ResizeMode::Repeat) { surfaceBrush.HorizontalAlignmentRatio(0.0f); From 5462c56657845a949c580e7b5419a421616a9ee4 Mon Sep 17 00:00:00 2001 From: Marlene Cota Date: Wed, 27 Nov 2019 02:11:51 -0800 Subject: [PATCH 03/17] timing issues --- vnext/ReactUWP/Views/Image/ReactImage.cpp | 164 +++++++++---------- vnext/ReactUWP/Views/Image/ReactImage.h | 9 +- vnext/ReactUWP/Views/Image/ReactImageBrush.h | 2 +- 3 files changed, 89 insertions(+), 86 deletions(-) diff --git a/vnext/ReactUWP/Views/Image/ReactImage.cpp b/vnext/ReactUWP/Views/Image/ReactImage.cpp index 69da1b92181..906bdc89d89 100644 --- a/vnext/ReactUWP/Views/Image/ReactImage.cpp +++ b/vnext/ReactUWP/Views/Image/ReactImage.cpp @@ -37,24 +37,6 @@ ReactImage::ReactImage() { } else { m_bitmapBrush.Stretch(winrt::Stretch::Uniform); this->Background(m_bitmapBrush); - - // auto weak_this{get_weak()}; - - //// ImageOpened and ImageFailed are mutually exclusive. One event of the other will - //// always fire whenever an ImageBrush has the ImageSource value set or reset. - // m_imageOpenedRevoker = - // m_bitmapBrush.ImageOpened(winrt::auto_revoke, [weak_this](const auto &, const auto &) { - // if (auto strong_this{weak_this.get()}) { - // strong_this->m_onLoadEndEvent(*strong_this, true); - // } - // }); - - // m_imageFailedRevoker = - // m_bitmapBrush.ImageFailed(winrt::auto_revoke, [weak_this](const auto &, const auto &) { - // if (auto strong_this{weak_this.get()}) { - // strong_this->m_onLoadEndEvent(*strong_this, false); - // } - // }); } } @@ -66,6 +48,8 @@ winrt::Size ReactImage::ArrangeOverride(winrt::Size finalSize) { if (m_useCompositionBrush) { auto brush{Background().as()}; brush->AvailableSize(finalSize); + } else { + m_availableSize = finalSize; } return finalSize; @@ -79,7 +63,7 @@ void ReactImage::OnLoadEnd(winrt::event_token const &token) noexcept { m_onLoadEndEvent.remove(token); } -void ReactImage::ResizeMode(react::uwp::ResizeMode value) { +winrt::Windows::Foundation::IAsyncAction ReactImage::ResizeMode(react::uwp::ResizeMode value) { if (m_resizeMode != value) { m_resizeMode = value; @@ -87,7 +71,7 @@ void ReactImage::ResizeMode(react::uwp::ResizeMode value) { if (m_useCompositionBrush != ShouldUseCompositionBrush()) { switchBrushes = true; m_useCompositionBrush = ShouldUseCompositionBrush(); - } + } if (!m_useCompositionBrush) { switch (m_resizeMode) { @@ -104,19 +88,33 @@ void ReactImage::ResizeMode(react::uwp::ResizeMode value) { } else { if (!m_brush) { m_brush = ReactImageBrush::Create(); + m_brush->AvailableSize(m_availableSize); } + } + if (m_brush) { m_brush->ResizeMode(value); } if (switchBrushes) { - Source(m_imageSource); + // get weak reference before any co_await calls + auto weak_this{get_weak()}; + + try { + co_await Source(m_imageSource, m_memoryStream != nullptr); - if (m_useCompositionBrush) { - this->Background(m_brush.as()); - } else { - this->Background(m_bitmapBrush); - } + if (auto strong_this{weak_this.get()}) { + if (strong_this->m_useCompositionBrush) { + strong_this->Background(strong_this->m_brush.as()); + } else { + strong_this->Background(strong_this->m_bitmapBrush); + } + } + } catch (winrt::hresult_error const &e) { + if (auto strong_this{weak_this.get()}) { + strong_this->m_onLoadEndEvent(*strong_this, false); + } + } } } } @@ -131,7 +129,7 @@ bool ReactImage::ShouldUseCompositionBrush() { } } -winrt::fire_and_forget ReactImage::Source(ImageSource source) { +winrt::Windows::Foundation::IAsyncAction ReactImage::Source(ImageSource source, bool useMemoryStreamCache) { std::string uriString{source.uri}; if (uriString.length() == 0) { m_onLoadEndEvent(*this, false); @@ -144,7 +142,8 @@ winrt::fire_and_forget ReactImage::Source(ImageSource source) { winrt::Uri uri{Utf8ToUtf16(uriString)}; winrt::hstring scheme{uri.SchemeName()}; - bool needsDownload = (scheme == L"http") || (scheme == L"https"); + bool needsDownload = + ((scheme == L"http") || (scheme == L"https")) && (m_useCompositionBrush || !source.headers.empty()); bool inlineData = scheme == L"data"; // get weak reference before any co_await calls auto weak_this{get_weak()}; @@ -152,72 +151,73 @@ winrt::fire_and_forget ReactImage::Source(ImageSource source) { try { m_imageSource = source; - winrt::InMemoryRandomAccessStream memoryStream; - if (needsDownload) { - memoryStream = co_await GetImageStreamAsync(source); - } else if (inlineData) { - memoryStream = co_await GetImageInlineDataAsync(source); + if (!useMemoryStreamCache) { + if (needsDownload) { + m_memoryStream = co_await GetImageStreamAsync(source); + } else if (inlineData) { + m_memoryStream = co_await GetImageInlineDataAsync(source); + } } if (auto strong_this{weak_this.get()}) { - if ((needsDownload || inlineData) && !memoryStream) { + if ((needsDownload || inlineData) && !m_memoryStream) { strong_this->m_onLoadEndEvent(*strong_this, false); } - if (!needsDownload || memoryStream) { - if (m_useCompositionBrush) { - auto surface = needsDownload || inlineData ? winrt::LoadedImageSurface::StartLoadFromStream(memoryStream) - : - winrt::LoadedImageSurface::StartLoadFromUri(uri); - - strong_this->m_surfaceLoadedRevoker = surface.LoadCompleted( - winrt::auto_revoke, - [weak_this, surface]( - winrt::LoadedImageSurface const & /*sender*/, - winrt::LoadedImageSourceLoadCompletedEventArgs const &args) { - if (auto strong_this{weak_this.get()}) { - bool succeeded{false}; - if (args.Status() == winrt::LoadedImageSourceLoadStatus::Success) { - strong_this->m_brush->Source(surface); - succeeded = true; - } - strong_this->m_onLoadEndEvent(*strong_this, succeeded); - } - }); - } - else { - winrt::BitmapImage bitmap; - - // 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_imageOpenedRevoker = - bitmap.ImageOpened(winrt::auto_revoke, [weak_this, bitmap](const auto &, const auto &) { - if (auto strong_this{weak_this.get()}) { - strong_this->m_bitmapBrush.ImageSource(bitmap); - strong_this->m_onLoadEndEvent(*strong_this, true); + if (!needsDownload || m_memoryStream) { + if (strong_this->m_useCompositionBrush) { + auto surface = needsDownload || inlineData ? winrt::LoadedImageSurface::StartLoadFromStream(m_memoryStream) + : winrt::LoadedImageSurface::StartLoadFromUri(uri); + + strong_this->m_surfaceLoadedRevoker = surface.LoadCompleted( + winrt::auto_revoke, + [weak_this, surface]( + winrt::LoadedImageSurface const & /*sender*/, + winrt::LoadedImageSourceLoadCompletedEventArgs const &args) { + if (auto strong_this{weak_this.get()}) { + bool succeeded{false}; + if (args.Status() == winrt::LoadedImageSourceLoadStatus::Success) { + strong_this->m_brush->Source(surface); + succeeded = true; + } + strong_this->m_onLoadEndEvent(*strong_this, succeeded); + } + }); + } else { + winrt::BitmapImage bitmap; + + // 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_imageOpenedRevoker = + bitmap.ImageOpened(winrt::auto_revoke, [weak_this, bitmap](const auto &, const auto &) { + if (auto strong_this{weak_this.get()}) { + strong_this->m_onLoadEndEvent(*strong_this, true); + } + }); + + strong_this->m_imageFailedRevoker = + bitmap.ImageFailed(winrt::auto_revoke, [weak_this](const auto &, const auto &) { + if (auto strong_this{weak_this.get()}) { + strong_this->m_onLoadEndEvent(*strong_this, false); + } + }); + + if (needsDownload || inlineData) { + bitmap.SetSourceAsync(strong_this->m_memoryStream); + } else { + bitmap.UriSource(uri); } - }); - strong_this->m_imageFailedRevoker = bitmap.ImageFailed(winrt::auto_revoke, [weak_this](const auto &, const auto &) { - if (auto strong_this{weak_this.get()}) { - strong_this->m_onLoadEndEvent(*strong_this, false); + strong_this->m_bitmapBrush.ImageSource(bitmap); + } } - }); - - if (needsDownload || inlineData) { - co_await bitmap.SetSourceAsync(memoryStream); - } else { - bitmap = {uri}; + } // namespace uwp + } catch (winrt::hresult_error const &) { + if (auto strong_this{weak_this.get()}) { + strong_this->m_onLoadEndEvent(*strong_this, false); } } } -} // namespace uwp -} // namespace react -catch (winrt::hresult_error const &) { - if (auto strong_this{weak_this.get()}) - strong_this->m_onLoadEndEvent(*strong_this, false); -} -} // namespace uwp winrt::IAsyncOperation GetImageStreamAsync(ImageSource source) { try { diff --git a/vnext/ReactUWP/Views/Image/ReactImage.h b/vnext/ReactUWP/Views/Image/ReactImage.h index 241e934d55e..21d39fed084 100644 --- a/vnext/ReactUWP/Views/Image/ReactImage.h +++ b/vnext/ReactUWP/Views/Image/ReactImage.h @@ -47,21 +47,24 @@ struct ReactImage : winrt::Windows::UI::Xaml::Controls::CanvasT { ImageSource Source() { return m_imageSource; } - winrt::fire_and_forget Source(ImageSource source); + winrt::Windows::Foundation::IAsyncAction Source(ImageSource source, bool useMemoryStreamCache = false); react::uwp::ResizeMode ResizeMode() { return m_resizeMode; } - void ResizeMode(react::uwp::ResizeMode value); + winrt::Windows::Foundation::IAsyncAction ResizeMode(react::uwp::ResizeMode value); private: bool ShouldUseCompositionBrush(); - bool m_useCompositionBrush = false; + bool m_useCompositionBrush{false}; ImageSource m_imageSource; winrt::Windows::UI::Xaml::Media::ImageBrush m_bitmapBrush{}; winrt::com_ptr m_brush; + winrt::Windows::Foundation::Size m_availableSize; react::uwp::ResizeMode m_resizeMode{ResizeMode::Contain}; + winrt::Windows::Storage::Streams::InMemoryRandomAccessStream m_memoryStream{nullptr}; + winrt::event> m_onLoadEndEvent; winrt::Windows::UI::Xaml::Media::LoadedImageSurface::LoadCompleted_revoker m_surfaceLoadedRevoker; winrt::Windows::UI::Xaml::Media::Imaging::BitmapImage::ImageOpened_revoker m_imageOpenedRevoker; diff --git a/vnext/ReactUWP/Views/Image/ReactImageBrush.h b/vnext/ReactUWP/Views/Image/ReactImageBrush.h index bc5c7c1a0c9..9602a76810f 100644 --- a/vnext/ReactUWP/Views/Image/ReactImageBrush.h +++ b/vnext/ReactUWP/Views/Image/ReactImageBrush.h @@ -25,7 +25,7 @@ struct ReactImageBrush : winrt::Windows::UI::Xaml::Media::XamlCompositionBrushBa void OnDisconnected(); // Public Properties - //react::uwp::ResizeMode ResizeMode() { + // react::uwp::ResizeMode ResizeMode() { // return m_resizeMode; //} void ResizeMode(react::uwp::ResizeMode value); From 65ce14d3ac627f22f88e515b34198d58aa50bf68 Mon Sep 17 00:00:00 2001 From: Marlene Cota Date: Wed, 27 Nov 2019 17:58:40 -0800 Subject: [PATCH 04/17] wip --- vnext/ReactUWP/Views/Image/ReactImage.cpp | 112 ++++++++++------------ vnext/ReactUWP/Views/Image/ReactImage.h | 17 ++-- 2 files changed, 64 insertions(+), 65 deletions(-) diff --git a/vnext/ReactUWP/Views/Image/ReactImage.cpp b/vnext/ReactUWP/Views/Image/ReactImage.cpp index 906bdc89d89..dc5a105bd8b 100644 --- a/vnext/ReactUWP/Views/Image/ReactImage.cpp +++ b/vnext/ReactUWP/Views/Image/ReactImage.cpp @@ -31,13 +31,13 @@ namespace react { namespace uwp { ReactImage::ReactImage() { - if (m_useCompositionBrush) { - m_brush = ReactImageBrush::Create(); - this->Background(m_brush.as()); - } else { - m_bitmapBrush.Stretch(winrt::Stretch::Uniform); - this->Background(m_bitmapBrush); - } + // if (m_useCompositionBrush) { + // m_compositionBrush = ReactImageBrush::Create(); + // this->Background(m_compositionBrush.as()); + //} else { + // m_bitmapBrush.Stretch(winrt::Stretch::Uniform); + // this->Background(m_bitmapBrush); + //} } /*static*/ winrt::com_ptr ReactImage::Create() { @@ -63,57 +63,25 @@ void ReactImage::OnLoadEnd(winrt::event_token const &token) noexcept { m_onLoadEndEvent.remove(token); } -winrt::Windows::Foundation::IAsyncAction ReactImage::ResizeMode(react::uwp::ResizeMode value) { +winrt::fire_and_forget ReactImage::ResizeMode(react::uwp::ResizeMode value) { if (m_resizeMode != value) { m_resizeMode = value; bool switchBrushes{false}; + if (m_useCompositionBrush != ShouldUseCompositionBrush()) { switchBrushes = true; m_useCompositionBrush = ShouldUseCompositionBrush(); } - if (!m_useCompositionBrush) { - switch (m_resizeMode) { - case ResizeMode::Cover: - m_bitmapBrush.Stretch(winrt::Stretch::UniformToFill); - break; - case ResizeMode::Stretch: - m_bitmapBrush.Stretch(winrt::Stretch::Fill); - break; - default: - m_bitmapBrush.Stretch(winrt::Stretch::Uniform); - break; - } - } else { - if (!m_brush) { - m_brush = ReactImageBrush::Create(); - m_brush->AvailableSize(m_availableSize); - } - } - - if (m_brush) { - m_brush->ResizeMode(value); - } - if (switchBrushes) { - // get weak reference before any co_await calls - auto weak_this{get_weak()}; - - try { - co_await Source(m_imageSource, m_memoryStream != nullptr); - - if (auto strong_this{weak_this.get()}) { - if (strong_this->m_useCompositionBrush) { - strong_this->Background(strong_this->m_brush.as()); - } else { - strong_this->Background(strong_this->m_bitmapBrush); - } - } - } catch (winrt::hresult_error const &e) { - if (auto strong_this{weak_this.get()}) { - strong_this->m_onLoadEndEvent(*strong_this, false); - } + co_await Source(m_imageSource, m_memoryStream != nullptr); + } else { + if (m_useCompositionBrush) { + Background().as()->ResizeMode(m_resizeMode); + } else { + const auto bitmapBrush{Background().as()}; + bitmapBrush.Stretch(ResizeModeToStretch(m_resizeMode)); } } } @@ -129,6 +97,17 @@ bool ReactImage::ShouldUseCompositionBrush() { } } +winrt::Stretch ReactImage::ResizeModeToStretch(react::uwp::ResizeMode value) { + switch (value) { + case ResizeMode::Cover: + return winrt::Stretch::UniformToFill; + case ResizeMode::Stretch: + return winrt::Stretch::Fill; + default: + return winrt::Stretch::Uniform; + } +} + winrt::Windows::Foundation::IAsyncAction ReactImage::Source(ImageSource source, bool useMemoryStreamCache) { std::string uriString{source.uri}; if (uriString.length() == 0) { @@ -160,55 +139,70 @@ winrt::Windows::Foundation::IAsyncAction ReactImage::Source(ImageSource source, } if (auto strong_this{weak_this.get()}) { - if ((needsDownload || inlineData) && !m_memoryStream) { + if ((needsDownload || inlineData) && !strong_this->m_memoryStream) { strong_this->m_onLoadEndEvent(*strong_this, false); } - if (!needsDownload || m_memoryStream) { + if (!needsDownload || strong_this->m_memoryStream) { if (strong_this->m_useCompositionBrush) { - auto surface = needsDownload || inlineData ? winrt::LoadedImageSurface::StartLoadFromStream(m_memoryStream) - : winrt::LoadedImageSurface::StartLoadFromUri(uri); + const auto compositionBrush = ReactImageBrush::Create(); + compositionBrush->AvailableSize(strong_this->m_availableSize); + compositionBrush->ResizeMode(strong_this->m_resizeMode); + + auto surface = (needsDownload || inlineData) + ? winrt::LoadedImageSurface::StartLoadFromStream(strong_this->m_memoryStream) + : winrt::LoadedImageSurface::StartLoadFromUri(uri); strong_this->m_surfaceLoadedRevoker = surface.LoadCompleted( winrt::auto_revoke, - [weak_this, surface]( + [weak_this, compositionBrush, surface]( winrt::LoadedImageSurface const & /*sender*/, winrt::LoadedImageSourceLoadCompletedEventArgs const &args) { if (auto strong_this{weak_this.get()}) { bool succeeded{false}; if (args.Status() == winrt::LoadedImageSourceLoadStatus::Success) { - strong_this->m_brush->Source(surface); + compositionBrush->Source(surface); succeeded = true; } strong_this->m_onLoadEndEvent(*strong_this, succeeded); } }); + + strong_this->Background(compositionBrush.as()); + } else { - winrt::BitmapImage bitmap; + winrt::ImageBrush bitmapBrush{}; // 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_imageOpenedRevoker = - bitmap.ImageOpened(winrt::auto_revoke, [weak_this, bitmap](const auto &, const auto &) { + bitmapBrush.ImageOpened(winrt::auto_revoke, [weak_this, bitmapBrush](const auto &, const auto &) { if (auto strong_this{weak_this.get()}) { + const auto bitmap{bitmapBrush.ImageSource().as()}; + strong_this->m_imageSource.height = bitmap.PixelHeight(); + strong_this->m_imageSource.width = bitmap.PixelWidth(); strong_this->m_onLoadEndEvent(*strong_this, true); } }); strong_this->m_imageFailedRevoker = - bitmap.ImageFailed(winrt::auto_revoke, [weak_this](const auto &, const auto &) { + bitmapBrush.ImageFailed(winrt::auto_revoke, [weak_this](const auto &, const auto &) { if (auto strong_this{weak_this.get()}) { strong_this->m_onLoadEndEvent(*strong_this, false); } }); + winrt::BitmapImage bitmap{}; + if (needsDownload || inlineData) { - bitmap.SetSourceAsync(strong_this->m_memoryStream); + co_await bitmap.SetSourceAsync(strong_this->m_memoryStream); } else { bitmap.UriSource(uri); } - strong_this->m_bitmapBrush.ImageSource(bitmap); + bitmapBrush.Stretch(ResizeModeToStretch(strong_this->m_resizeMode)); + bitmapBrush.ImageSource(bitmap); + strong_this->Background(bitmapBrush); } } } // namespace uwp diff --git a/vnext/ReactUWP/Views/Image/ReactImage.h b/vnext/ReactUWP/Views/Image/ReactImage.h index 21d39fed084..d1a1e7392f5 100644 --- a/vnext/ReactUWP/Views/Image/ReactImage.h +++ b/vnext/ReactUWP/Views/Image/ReactImage.h @@ -52,23 +52,28 @@ struct ReactImage : winrt::Windows::UI::Xaml::Controls::CanvasT { react::uwp::ResizeMode ResizeMode() { return m_resizeMode; } - winrt::Windows::Foundation::IAsyncAction ResizeMode(react::uwp::ResizeMode value); + winrt::fire_and_forget ResizeMode(react::uwp::ResizeMode value); private: bool ShouldUseCompositionBrush(); + winrt::Windows::UI::Xaml::Media::Stretch ResizeModeToStretch(react::uwp::ResizeMode value); + //winrt::Windows::UI::Xaml::Media::ImageBrush GetBackgroundAsImageBrush(); + //winrt::com_ptr GetBackgroundAsCompositionBrush(); bool m_useCompositionBrush{false}; ImageSource m_imageSource; - winrt::Windows::UI::Xaml::Media::ImageBrush m_bitmapBrush{}; - winrt::com_ptr m_brush; - winrt::Windows::Foundation::Size m_availableSize; + //winrt::Windows::UI::Xaml::Media::Imaging::BitmapImage m_bitmapImage{nullptr}; + //winrt::Windows::UI::Xaml::Media::LoadedImageSurface m_loadedImageSurface{nullptr}; + //winrt::Windows::UI::Xaml::Media::ImageBrush m_bitmapBrush{}; + //winrt::com_ptr m_compositionBrush{nullptr}; + winrt::Windows::Foundation::Size m_availableSize{}; react::uwp::ResizeMode m_resizeMode{ResizeMode::Contain}; winrt::Windows::Storage::Streams::InMemoryRandomAccessStream m_memoryStream{nullptr}; winrt::event> m_onLoadEndEvent; winrt::Windows::UI::Xaml::Media::LoadedImageSurface::LoadCompleted_revoker m_surfaceLoadedRevoker; - winrt::Windows::UI::Xaml::Media::Imaging::BitmapImage::ImageOpened_revoker m_imageOpenedRevoker; - winrt::Windows::UI::Xaml::Media::Imaging::BitmapImage::ImageFailed_revoker m_imageFailedRevoker; + winrt::Windows::UI::Xaml::Media::ImageBrush::ImageOpened_revoker m_imageOpenedRevoker; + winrt::Windows::UI::Xaml::Media::ImageBrush::ImageFailed_revoker m_imageFailedRevoker; }; // Helper functions From da3ea8b4126d6fd85273d81259b83ceff83bd2e6 Mon Sep 17 00:00:00 2001 From: Marlene Cota Date: Wed, 27 Nov 2019 18:39:06 -0800 Subject: [PATCH 05/17] Move 'center' resizeMode to BitmapImage --- vnext/ReactUWP/Views/Image/ReactImage.cpp | 39 ++++++++++------------- vnext/ReactUWP/Views/Image/ReactImage.h | 8 +---- 2 files changed, 17 insertions(+), 30 deletions(-) diff --git a/vnext/ReactUWP/Views/Image/ReactImage.cpp b/vnext/ReactUWP/Views/Image/ReactImage.cpp index dc5a105bd8b..a352251f1ac 100644 --- a/vnext/ReactUWP/Views/Image/ReactImage.cpp +++ b/vnext/ReactUWP/Views/Image/ReactImage.cpp @@ -30,16 +30,6 @@ using Microsoft::Common::Unicode::Utf8ToUtf16; namespace react { namespace uwp { -ReactImage::ReactImage() { - // if (m_useCompositionBrush) { - // m_compositionBrush = ReactImageBrush::Create(); - // this->Background(m_compositionBrush.as()); - //} else { - // m_bitmapBrush.Stretch(winrt::Stretch::Uniform); - // this->Background(m_bitmapBrush); - //} -} - /*static*/ winrt::com_ptr ReactImage::Create() { return winrt::make_self(); } @@ -88,13 +78,7 @@ winrt::fire_and_forget ReactImage::ResizeMode(react::uwp::ResizeMode value) { } bool ReactImage::ShouldUseCompositionBrush() { - switch (m_resizeMode) { - case ResizeMode::Repeat: - case ResizeMode::Center: - return true; - default: - return false; - } + return m_resizeMode == ResizeMode::Repeat; } winrt::Stretch ReactImage::ResizeModeToStretch(react::uwp::ResizeMode value) { @@ -103,8 +87,15 @@ winrt::Stretch ReactImage::ResizeModeToStretch(react::uwp::ResizeMode value) { return winrt::Stretch::UniformToFill; case ResizeMode::Stretch: return winrt::Stretch::Fill; - default: + case ResizeMode::Contain: return winrt::Stretch::Uniform; + default: + const auto bitmap{Background().as().ImageSource().as()}; + if (bitmap.PixelHeight() < m_availableSize.Height && bitmap.PixelWidth() < m_availableSize.Width) { + return winrt::Stretch::None; + } else { + return winrt::Stretch::Uniform; + } } } @@ -124,6 +115,7 @@ winrt::Windows::Foundation::IAsyncAction ReactImage::Source(ImageSource source, bool needsDownload = ((scheme == L"http") || (scheme == L"https")) && (m_useCompositionBrush || !source.headers.empty()); bool inlineData = scheme == L"data"; + // get weak reference before any co_await calls auto weak_this{get_weak()}; @@ -150,8 +142,8 @@ winrt::Windows::Foundation::IAsyncAction ReactImage::Source(ImageSource source, compositionBrush->ResizeMode(strong_this->m_resizeMode); auto surface = (needsDownload || inlineData) - ? winrt::LoadedImageSurface::StartLoadFromStream(strong_this->m_memoryStream) - : winrt::LoadedImageSurface::StartLoadFromUri(uri); + ? winrt::LoadedImageSurface::StartLoadFromStream(strong_this->m_memoryStream) + : winrt::LoadedImageSurface::StartLoadFromUri(uri); strong_this->m_surfaceLoadedRevoker = surface.LoadCompleted( winrt::auto_revoke, @@ -162,14 +154,13 @@ winrt::Windows::Foundation::IAsyncAction ReactImage::Source(ImageSource source, bool succeeded{false}; if (args.Status() == winrt::LoadedImageSourceLoadStatus::Success) { compositionBrush->Source(surface); + strong_this->Background(compositionBrush.as()); succeeded = true; } strong_this->m_onLoadEndEvent(*strong_this, succeeded); } }); - strong_this->Background(compositionBrush.as()); - } else { winrt::ImageBrush bitmapBrush{}; @@ -181,6 +172,9 @@ winrt::Windows::Foundation::IAsyncAction ReactImage::Source(ImageSource source, const auto bitmap{bitmapBrush.ImageSource().as()}; strong_this->m_imageSource.height = bitmap.PixelHeight(); strong_this->m_imageSource.width = bitmap.PixelWidth(); + + bitmapBrush.Stretch(strong_this->ResizeModeToStretch(strong_this->m_resizeMode)); + strong_this->m_onLoadEndEvent(*strong_this, true); } }); @@ -200,7 +194,6 @@ winrt::Windows::Foundation::IAsyncAction ReactImage::Source(ImageSource source, bitmap.UriSource(uri); } - bitmapBrush.Stretch(ResizeModeToStretch(strong_this->m_resizeMode)); bitmapBrush.ImageSource(bitmap); strong_this->Background(bitmapBrush); } diff --git a/vnext/ReactUWP/Views/Image/ReactImage.h b/vnext/ReactUWP/Views/Image/ReactImage.h index d1a1e7392f5..be68dc87f42 100644 --- a/vnext/ReactUWP/Views/Image/ReactImage.h +++ b/vnext/ReactUWP/Views/Image/ReactImage.h @@ -31,7 +31,7 @@ struct ImageSource { struct ReactImage : winrt::Windows::UI::Xaml::Controls::CanvasT { using Super = winrt::Windows::UI::Xaml::Controls::CanvasT; - ReactImage(); + ReactImage() = default; public: static winrt::com_ptr Create(); @@ -57,15 +57,9 @@ struct ReactImage : winrt::Windows::UI::Xaml::Controls::CanvasT { private: bool ShouldUseCompositionBrush(); winrt::Windows::UI::Xaml::Media::Stretch ResizeModeToStretch(react::uwp::ResizeMode value); - //winrt::Windows::UI::Xaml::Media::ImageBrush GetBackgroundAsImageBrush(); - //winrt::com_ptr GetBackgroundAsCompositionBrush(); bool m_useCompositionBrush{false}; ImageSource m_imageSource; - //winrt::Windows::UI::Xaml::Media::Imaging::BitmapImage m_bitmapImage{nullptr}; - //winrt::Windows::UI::Xaml::Media::LoadedImageSurface m_loadedImageSurface{nullptr}; - //winrt::Windows::UI::Xaml::Media::ImageBrush m_bitmapBrush{}; - //winrt::com_ptr m_compositionBrush{nullptr}; winrt::Windows::Foundation::Size m_availableSize{}; react::uwp::ResizeMode m_resizeMode{ResizeMode::Contain}; winrt::Windows::Storage::Streams::InMemoryRandomAccessStream m_memoryStream{nullptr}; From e5c984e553bb690eb4bab5fa8372ff28d2d543f0 Mon Sep 17 00:00:00 2001 From: Marlene Cota Date: Wed, 27 Nov 2019 18:55:57 -0800 Subject: [PATCH 06/17] code cleanup --- vnext/ReactUWP/Views/Image/ReactImage.cpp | 14 +++++--------- 1 file changed, 5 insertions(+), 9 deletions(-) diff --git a/vnext/ReactUWP/Views/Image/ReactImage.cpp b/vnext/ReactUWP/Views/Image/ReactImage.cpp index a352251f1ac..daf0e9b5103 100644 --- a/vnext/ReactUWP/Views/Image/ReactImage.cpp +++ b/vnext/ReactUWP/Views/Image/ReactImage.cpp @@ -35,12 +35,12 @@ namespace uwp { } winrt::Size ReactImage::ArrangeOverride(winrt::Size finalSize) { + m_availableSize = finalSize; + if (m_useCompositionBrush) { auto brush{Background().as()}; brush->AvailableSize(finalSize); - } else { - m_availableSize = finalSize; - } + } return finalSize; } @@ -67,12 +67,8 @@ winrt::fire_and_forget ReactImage::ResizeMode(react::uwp::ResizeMode value) { if (switchBrushes) { co_await Source(m_imageSource, m_memoryStream != nullptr); } else { - if (m_useCompositionBrush) { - Background().as()->ResizeMode(m_resizeMode); - } else { - const auto bitmapBrush{Background().as()}; - bitmapBrush.Stretch(ResizeModeToStretch(m_resizeMode)); - } + const auto bitmapBrush{Background().as()}; + bitmapBrush.Stretch(ResizeModeToStretch(m_resizeMode)); } } } From 79d1421431390c6ca2b748563e5efb5c3e0e81ca Mon Sep 17 00:00:00 2001 From: Marlene Cota Date: Wed, 27 Nov 2019 20:36:05 -0800 Subject: [PATCH 07/17] ReactImage->Source() refactor --- vnext/ReactUWP/Views/Image/ReactImage.cpp | 156 ++++++++++--------- vnext/ReactUWP/Views/Image/ReactImage.h | 5 +- vnext/ReactUWP/Views/Image/ReactImageBrush.h | 3 - 3 files changed, 87 insertions(+), 77 deletions(-) diff --git a/vnext/ReactUWP/Views/Image/ReactImage.cpp b/vnext/ReactUWP/Views/Image/ReactImage.cpp index daf0e9b5103..3ad544f20f5 100644 --- a/vnext/ReactUWP/Views/Image/ReactImage.cpp +++ b/vnext/ReactUWP/Views/Image/ReactImage.cpp @@ -40,7 +40,7 @@ winrt::Size ReactImage::ArrangeOverride(winrt::Size finalSize) { if (m_useCompositionBrush) { auto brush{Background().as()}; brush->AvailableSize(finalSize); - } + } return finalSize; } @@ -53,7 +53,7 @@ void ReactImage::OnLoadEnd(winrt::event_token const &token) noexcept { m_onLoadEndEvent.remove(token); } -winrt::fire_and_forget ReactImage::ResizeMode(react::uwp::ResizeMode value) { +void ReactImage::ResizeMode(react::uwp::ResizeMode value) { if (m_resizeMode != value) { m_resizeMode = value; @@ -65,7 +65,7 @@ winrt::fire_and_forget ReactImage::ResizeMode(react::uwp::ResizeMode value) { } if (switchBrushes) { - co_await Source(m_imageSource, m_memoryStream != nullptr); + SetBackground(m_memoryStream != nullptr, false); } else { const auto bitmapBrush{Background().as()}; bitmapBrush.Stretch(ResizeModeToStretch(m_resizeMode)); @@ -85,7 +85,7 @@ winrt::Stretch ReactImage::ResizeModeToStretch(react::uwp::ResizeMode value) { return winrt::Stretch::Fill; case ResizeMode::Contain: return winrt::Stretch::Uniform; - default: + default: // ResizeMode::Center const auto bitmap{Background().as().ImageSource().as()}; if (bitmap.PixelHeight() < m_availableSize.Height && bitmap.PixelWidth() < m_availableSize.Width) { return winrt::Stretch::None; @@ -95,7 +95,7 @@ winrt::Stretch ReactImage::ResizeModeToStretch(react::uwp::ResizeMode value) { } } -winrt::Windows::Foundation::IAsyncAction ReactImage::Source(ImageSource source, bool useMemoryStreamCache) { +winrt::fire_and_forget ReactImage::Source(ImageSource source) { std::string uriString{source.uri}; if (uriString.length() == 0) { m_onLoadEndEvent(*this, false); @@ -118,12 +118,10 @@ winrt::Windows::Foundation::IAsyncAction ReactImage::Source(ImageSource source, try { m_imageSource = source; - if (!useMemoryStreamCache) { - if (needsDownload) { - m_memoryStream = co_await GetImageStreamAsync(source); - } else if (inlineData) { - m_memoryStream = co_await GetImageInlineDataAsync(source); - } + if (needsDownload) { + m_memoryStream = co_await GetImageStreamAsync(source); + } else if (inlineData) { + m_memoryStream = co_await GetImageInlineDataAsync(source); } if (auto strong_this{weak_this.get()}) { @@ -132,67 +130,7 @@ winrt::Windows::Foundation::IAsyncAction ReactImage::Source(ImageSource source, } if (!needsDownload || strong_this->m_memoryStream) { - if (strong_this->m_useCompositionBrush) { - const auto compositionBrush = ReactImageBrush::Create(); - compositionBrush->AvailableSize(strong_this->m_availableSize); - compositionBrush->ResizeMode(strong_this->m_resizeMode); - - auto surface = (needsDownload || inlineData) - ? winrt::LoadedImageSurface::StartLoadFromStream(strong_this->m_memoryStream) - : winrt::LoadedImageSurface::StartLoadFromUri(uri); - - strong_this->m_surfaceLoadedRevoker = surface.LoadCompleted( - winrt::auto_revoke, - [weak_this, compositionBrush, surface]( - winrt::LoadedImageSurface const & /*sender*/, - winrt::LoadedImageSourceLoadCompletedEventArgs const &args) { - if (auto strong_this{weak_this.get()}) { - bool succeeded{false}; - if (args.Status() == winrt::LoadedImageSourceLoadStatus::Success) { - compositionBrush->Source(surface); - strong_this->Background(compositionBrush.as()); - succeeded = true; - } - strong_this->m_onLoadEndEvent(*strong_this, succeeded); - } - }); - - } else { - winrt::ImageBrush bitmapBrush{}; - - // 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_imageOpenedRevoker = - bitmapBrush.ImageOpened(winrt::auto_revoke, [weak_this, bitmapBrush](const auto &, const auto &) { - if (auto strong_this{weak_this.get()}) { - const auto bitmap{bitmapBrush.ImageSource().as()}; - strong_this->m_imageSource.height = bitmap.PixelHeight(); - strong_this->m_imageSource.width = bitmap.PixelWidth(); - - bitmapBrush.Stretch(strong_this->ResizeModeToStretch(strong_this->m_resizeMode)); - - strong_this->m_onLoadEndEvent(*strong_this, true); - } - }); - - strong_this->m_imageFailedRevoker = - bitmapBrush.ImageFailed(winrt::auto_revoke, [weak_this](const auto &, const auto &) { - if (auto strong_this{weak_this.get()}) { - strong_this->m_onLoadEndEvent(*strong_this, false); - } - }); - - winrt::BitmapImage bitmap{}; - - if (needsDownload || inlineData) { - co_await bitmap.SetSourceAsync(strong_this->m_memoryStream); - } else { - bitmap.UriSource(uri); - } - - bitmapBrush.ImageSource(bitmap); - strong_this->Background(bitmapBrush); - } + strong_this->SetBackground(needsDownload || inlineData, true); } } // namespace uwp } catch (winrt::hresult_error const &) { @@ -202,6 +140,80 @@ winrt::Windows::Foundation::IAsyncAction ReactImage::Source(ImageSource source, } } +void ReactImage::SetBackground(bool fromStream, bool fireLoadEndEvent) { + const winrt::Uri uri{Utf8ToUtf16(m_imageSource.uri)}; + + if (m_useCompositionBrush) { + const auto compositionBrush = ReactImageBrush::Create(); + compositionBrush->AvailableSize(m_availableSize); + compositionBrush->ResizeMode(m_resizeMode); + + auto surface = fromStream ? winrt::LoadedImageSurface::StartLoadFromStream(m_memoryStream) + : winrt::LoadedImageSurface::StartLoadFromUri(uri); + + m_surfaceLoadedRevoker = surface.LoadCompleted( + winrt::auto_revoke, + [weak_this{get_weak()}, compositionBrush, surface, fireLoadEndEvent]( + winrt::LoadedImageSurface const & /*sender*/, winrt::LoadedImageSourceLoadCompletedEventArgs const &args) { + if (auto strong_this{weak_this.get()}) { + bool succeeded{false}; + if (args.Status() == winrt::LoadedImageSourceLoadStatus::Success) { + winrt::Size dipsSize{surface.DecodedSize()}; + strong_this->m_imageSource.height = dipsSize.Height; + strong_this->m_imageSource.width = dipsSize.Width; + + compositionBrush->Source(surface); + strong_this->Background(compositionBrush.as()); + succeeded = true; + } + + if (fireLoadEndEvent) { + strong_this->m_onLoadEndEvent(*strong_this, succeeded); + } + } + }); + + } else { + winrt::ImageBrush bitmapBrush{}; + + // ImageOpened and ImageFailed are mutually exclusive. One event of the other will + // always fire whenever an ImageBrush has the ImageSource value set or reset. + m_imageOpenedRevoker = bitmapBrush.ImageOpened( + winrt::auto_revoke, [weak_this{get_weak()}, bitmapBrush, fireLoadEndEvent](const auto &, const auto &) { + if (auto strong_this{weak_this.get()}) { + const auto bitmap{bitmapBrush.ImageSource().as()}; + strong_this->m_imageSource.height = bitmap.PixelHeight(); + strong_this->m_imageSource.width = bitmap.PixelWidth(); + + bitmapBrush.Stretch(strong_this->ResizeModeToStretch(strong_this->m_resizeMode)); + + if (fireLoadEndEvent) { + strong_this->m_onLoadEndEvent(*strong_this, true); + } + } + }); + + m_imageFailedRevoker = bitmapBrush.ImageFailed( + winrt::auto_revoke, [weak_this{get_weak()}, fireLoadEndEvent](const auto &, const auto &) { + const auto strong_this{weak_this.get()}; + if (strong_this && fireLoadEndEvent) { + strong_this->m_onLoadEndEvent(*strong_this, false); + } + }); + + winrt::BitmapImage bitmap{}; + + if (fromStream) { + bitmap.SetSourceAsync(m_memoryStream); + } else { + bitmap.UriSource(uri); + } + + bitmapBrush.ImageSource(bitmap); + Background(bitmapBrush); + } +} + winrt::IAsyncOperation GetImageStreamAsync(ImageSource source) { try { co_await winrt::resume_background(); diff --git a/vnext/ReactUWP/Views/Image/ReactImage.h b/vnext/ReactUWP/Views/Image/ReactImage.h index be68dc87f42..24dbb3b0f29 100644 --- a/vnext/ReactUWP/Views/Image/ReactImage.h +++ b/vnext/ReactUWP/Views/Image/ReactImage.h @@ -47,16 +47,17 @@ struct ReactImage : winrt::Windows::UI::Xaml::Controls::CanvasT { ImageSource Source() { return m_imageSource; } - winrt::Windows::Foundation::IAsyncAction Source(ImageSource source, bool useMemoryStreamCache = false); + winrt::fire_and_forget Source(ImageSource source); react::uwp::ResizeMode ResizeMode() { return m_resizeMode; } - winrt::fire_and_forget ResizeMode(react::uwp::ResizeMode value); + void ResizeMode(react::uwp::ResizeMode value); private: bool ShouldUseCompositionBrush(); winrt::Windows::UI::Xaml::Media::Stretch ResizeModeToStretch(react::uwp::ResizeMode value); + void SetBackground(bool fromStream, bool fireLoadEndEvent); bool m_useCompositionBrush{false}; ImageSource m_imageSource; diff --git a/vnext/ReactUWP/Views/Image/ReactImageBrush.h b/vnext/ReactUWP/Views/Image/ReactImageBrush.h index 9602a76810f..7859d4f106c 100644 --- a/vnext/ReactUWP/Views/Image/ReactImageBrush.h +++ b/vnext/ReactUWP/Views/Image/ReactImageBrush.h @@ -25,9 +25,6 @@ struct ReactImageBrush : winrt::Windows::UI::Xaml::Media::XamlCompositionBrushBa void OnDisconnected(); // Public Properties - // react::uwp::ResizeMode ResizeMode() { - // return m_resizeMode; - //} void ResizeMode(react::uwp::ResizeMode value); winrt::Windows::Foundation::Size AvailableSize() { From 023ef6ee7d849d6fa1fc319040ead78d11328bf3 Mon Sep 17 00:00:00 2001 From: Marlene Cota Date: Mon, 2 Dec 2019 13:44:28 -0800 Subject: [PATCH 08/17] Clean up for PR --- packages/playground/Samples/image.tsx | 7 +++---- vnext/ReactUWP/Views/Image/ReactImage.cpp | 2 +- vnext/ReactUWP/Views/Image/ReactImageBrush.cpp | 6 +++--- vnext/ReactUWP/Views/Image/ReactImageBrush.h | 3 +++ 4 files changed, 10 insertions(+), 8 deletions(-) diff --git a/packages/playground/Samples/image.tsx b/packages/playground/Samples/image.tsx index 0bbd20ae7c4..aac93924745 100644 --- a/packages/playground/Samples/image.tsx +++ b/packages/playground/Samples/image.tsx @@ -27,10 +27,9 @@ export default class Bootstrap extends React.Component< } > { state = { - selectedResizeMode: 'contain' as 'contain', - useLargeImage: true, - imageUrl: - 'https://cdn.freebiesupply.com/logos/large/2x/react-logo-png-transparent.png', + selectedResizeMode: 'center' as 'center', + useLargeImage: false, + imageUrl: 'http://facebook.github.io/react-native/img/header_logo.png', }; switchImageUrl = () => { diff --git a/vnext/ReactUWP/Views/Image/ReactImage.cpp b/vnext/ReactUWP/Views/Image/ReactImage.cpp index 3ad544f20f5..2561fc1df5b 100644 --- a/vnext/ReactUWP/Views/Image/ReactImage.cpp +++ b/vnext/ReactUWP/Views/Image/ReactImage.cpp @@ -132,7 +132,7 @@ winrt::fire_and_forget ReactImage::Source(ImageSource source) { if (!needsDownload || strong_this->m_memoryStream) { strong_this->SetBackground(needsDownload || inlineData, true); } - } // namespace uwp + } } catch (winrt::hresult_error const &) { if (auto strong_this{weak_this.get()}) { strong_this->m_onLoadEndEvent(*strong_this, false); diff --git a/vnext/ReactUWP/Views/Image/ReactImageBrush.cpp b/vnext/ReactUWP/Views/Image/ReactImageBrush.cpp index 320cd972998..4a59db48ac1 100644 --- a/vnext/ReactUWP/Views/Image/ReactImageBrush.cpp +++ b/vnext/ReactUWP/Views/Image/ReactImageBrush.cpp @@ -70,7 +70,7 @@ void ReactImageBrush::UpdateCompositionBrush() { surfaceBrush.Stretch(ResizeModeToStretch()); auto compositionBrush{surfaceBrush.as()}; - if (m_resizeMode == ResizeMode::Repeat) { + if (ResizeMode() == ResizeMode::Repeat) { // If ResizeMode is set to Repeat, then we need to use a CompositionEffectBrush. // The CompositionSurfaceBrush holding the image is used as its source. compositionBrush = GetOrCreateEffectBrush(surfaceBrush); @@ -79,7 +79,7 @@ void ReactImageBrush::UpdateCompositionBrush() { // The CompositionBrush is only set after the image is first loaded and anytime // we switch between Surface and Effect brushes (to/from ResizeMode::Repeat) if (CompositionBrush() != compositionBrush) { - if (m_resizeMode == ResizeMode::Repeat) { + if (ResizeMode() == ResizeMode::Repeat) { surfaceBrush.HorizontalAlignmentRatio(0.0f); surfaceBrush.VerticalAlignmentRatio(0.0f); } else { @@ -106,7 +106,7 @@ bool ReactImageBrush::IsImageSmallerThanView() { winrt::CompositionStretch ReactImageBrush::ResizeModeToStretch() { auto stretch{winrt::CompositionStretch::None}; - switch (m_resizeMode) { + switch (ResizeMode()) { case ResizeMode::Contain: stretch = winrt::CompositionStretch::Uniform; break; diff --git a/vnext/ReactUWP/Views/Image/ReactImageBrush.h b/vnext/ReactUWP/Views/Image/ReactImageBrush.h index 7859d4f106c..8bc3b98bc51 100644 --- a/vnext/ReactUWP/Views/Image/ReactImageBrush.h +++ b/vnext/ReactUWP/Views/Image/ReactImageBrush.h @@ -25,6 +25,9 @@ struct ReactImageBrush : winrt::Windows::UI::Xaml::Media::XamlCompositionBrushBa void OnDisconnected(); // Public Properties + react::uwp::ResizeMode ResizeMode() { + return m_resizeMode; + } void ResizeMode(react::uwp::ResizeMode value); winrt::Windows::Foundation::Size AvailableSize() { From eb276a24225a24f925237b8209926fb21efba154 Mon Sep 17 00:00:00 2001 From: Marlene Cota Date: Mon, 2 Dec 2019 13:46:48 -0800 Subject: [PATCH 09/17] Change files --- ...t-native-windows-2019-12-02-13-46-48-bitmap-image.json | 8 ++++++++ 1 file changed, 8 insertions(+) create mode 100644 change/react-native-windows-2019-12-02-13-46-48-bitmap-image.json diff --git a/change/react-native-windows-2019-12-02-13-46-48-bitmap-image.json b/change/react-native-windows-2019-12-02-13-46-48-bitmap-image.json new file mode 100644 index 00000000000..cb1dcbc112d --- /dev/null +++ b/change/react-native-windows-2019-12-02-13-46-48-bitmap-image.json @@ -0,0 +1,8 @@ +{ + "type": "prerelease", + "comment": "Conditionally use BitmapImage", + "packageName": "react-native-windows", + "email": "email not defined", + "commit": "023ef6ee7d849d6fa1fc319040ead78d11328bf3", + "date": "2019-12-02T21:46:48.495Z" +} \ No newline at end of file From b90d193647a9c73abb94b423076e3b0b457777ff Mon Sep 17 00:00:00 2001 From: Marlene Cota Date: Mon, 9 Dec 2019 16:30:23 -0800 Subject: [PATCH 10/17] PR feedback --- .../Modules/ImageViewManagerModule.cpp | 2 +- .../ReactUWP/Views/Image/ImageViewManager.cpp | 12 ++-- vnext/ReactUWP/Views/Image/ImageViewManager.h | 2 +- vnext/ReactUWP/Views/Image/ReactImage.cpp | 67 +++++++++++-------- vnext/ReactUWP/Views/Image/ReactImage.h | 15 ++--- 5 files changed, 55 insertions(+), 43 deletions(-) diff --git a/vnext/ReactUWP/Modules/ImageViewManagerModule.cpp b/vnext/ReactUWP/Modules/ImageViewManagerModule.cpp index 2b1730cc5f5..a018641f7f5 100644 --- a/vnext/ReactUWP/Modules/ImageViewManagerModule.cpp +++ b/vnext/ReactUWP/Modules/ImageViewManagerModule.cpp @@ -61,7 +61,7 @@ winrt::fire_and_forget GetImageSizeAsync( bool succeeded{false}; try { - ImageSource source; + ReactImageSource source; source.uri = uriString; winrt::Uri uri{Microsoft::Common::Unicode::Utf8ToUtf16(uriString)}; diff --git a/vnext/ReactUWP/Views/Image/ImageViewManager.cpp b/vnext/ReactUWP/Views/Image/ImageViewManager.cpp index f909df5e7c2..6c5acb2aee6 100644 --- a/vnext/ReactUWP/Views/Image/ImageViewManager.cpp +++ b/vnext/ReactUWP/Views/Image/ImageViewManager.cpp @@ -23,9 +23,9 @@ using namespace Windows::UI::Xaml::Controls; // Such code is better to move to a seperate parser layer template <> -struct json_type_traits { - static react::uwp::ImageSource parseJson(const folly::dynamic &json) { - react::uwp::ImageSource source; +struct json_type_traits { + static react::uwp::ReactImageSource parseJson(const folly::dynamic &json) { + react::uwp::ReactImageSource source; for (auto &item : json.items()) { if (item.first == "uri") source.uri = item.second.asString(); @@ -80,7 +80,7 @@ class ImageShadowNode : public ShadowNodeBase { m_onLoadEndToken = reactImage->OnLoadEnd([imageViewManager{static_cast(GetViewManager())}, reactImage](const auto &, const bool &succeeded) { - ImageSource source{reactImage->Source()}; + ReactImageSource source{reactImage->Source()}; imageViewManager->EmitImageEvent(reactImage.as(), succeeded ? "topLoad" : "topError", source); imageViewManager->EmitImageEvent(reactImage.as(), "topLoadEnd", source); @@ -134,7 +134,7 @@ void ImageViewManager::UpdateProperties(ShadowNodeBase *nodeToUpdate, const foll Super::UpdateProperties(nodeToUpdate, reactDiffMap); } -void ImageViewManager::EmitImageEvent(winrt::Canvas canvas, const char *eventName, ImageSource &source) { +void ImageViewManager::EmitImageEvent(winrt::Canvas canvas, const char *eventName, ReactImageSource &source) { auto reactInstance{m_wkReactInstance.lock()}; if (reactInstance == nullptr) return; @@ -152,7 +152,7 @@ void ImageViewManager::setSource(winrt::Canvas canvas, const folly::dynamic &dat if (instance == nullptr) return; - auto sources{json_type_traits>::parseJson(data)}; + auto sources{json_type_traits>::parseJson(data)}; sources[0].bundleRootPath = instance->GetBundleRootPath(); auto reactImage{canvas.as()}; diff --git a/vnext/ReactUWP/Views/Image/ImageViewManager.h b/vnext/ReactUWP/Views/Image/ImageViewManager.h index 83e9aad2c4c..1eb6c3e00ae 100644 --- a/vnext/ReactUWP/Views/Image/ImageViewManager.h +++ b/vnext/ReactUWP/Views/Image/ImageViewManager.h @@ -19,7 +19,7 @@ class ImageViewManager : public FrameworkElementViewManager { folly::dynamic GetExportedCustomDirectEventTypeConstants() const override; folly::dynamic GetNativeProps() const override; facebook::react::ShadowNode *createShadow() const override; - void EmitImageEvent(winrt::Windows::UI::Xaml::Controls::Canvas canvas, const char *eventName, ImageSource &source); + void EmitImageEvent(winrt::Windows::UI::Xaml::Controls::Canvas canvas, const char *eventName, ReactImageSource &source); protected: XamlView CreateViewCore(int64_t tag) override; diff --git a/vnext/ReactUWP/Views/Image/ReactImage.cpp b/vnext/ReactUWP/Views/Image/ReactImage.cpp index 2561fc1df5b..f47d3d88c10 100644 --- a/vnext/ReactUWP/Views/Image/ReactImage.cpp +++ b/vnext/ReactUWP/Views/Image/ReactImage.cpp @@ -57,14 +57,11 @@ void ReactImage::ResizeMode(react::uwp::ResizeMode value) { if (m_resizeMode != value) { m_resizeMode = value; - bool switchBrushes{false}; - - if (m_useCompositionBrush != ShouldUseCompositionBrush()) { - switchBrushes = true; - m_useCompositionBrush = ShouldUseCompositionBrush(); - } + bool shouldUseCompositionBrush{m_resizeMode == ResizeMode::Repeat}; + bool switchBrushes{m_useCompositionBrush != shouldUseCompositionBrush}; if (switchBrushes) { + m_useCompositionBrush = shouldUseCompositionBrush; SetBackground(m_memoryStream != nullptr, false); } else { const auto bitmapBrush{Background().as()}; @@ -73,10 +70,6 @@ void ReactImage::ResizeMode(react::uwp::ResizeMode value) { } } -bool ReactImage::ShouldUseCompositionBrush() { - return m_resizeMode == ResizeMode::Repeat; -} - winrt::Stretch ReactImage::ResizeModeToStretch(react::uwp::ResizeMode value) { switch (value) { case ResizeMode::Cover: @@ -86,6 +79,10 @@ winrt::Stretch ReactImage::ResizeModeToStretch(react::uwp::ResizeMode value) { case ResizeMode::Contain: return winrt::Stretch::Uniform; default: // ResizeMode::Center + // This function should never be called for the 'repeat' resizeMode case. + // That is handled by the shouldUseCompositionBrush/switchBrushes code path. + assert(value != ResizeMode::Repeat); + const auto bitmap{Background().as().ImageSource().as()}; if (bitmap.PixelHeight() < m_availableSize.Height && bitmap.PixelWidth() < m_availableSize.Width) { return winrt::Stretch::None; @@ -95,13 +92,16 @@ winrt::Stretch ReactImage::ResizeModeToStretch(react::uwp::ResizeMode value) { } } -winrt::fire_and_forget ReactImage::Source(ImageSource source) { +winrt::fire_and_forget ReactImage::Source(ReactImageSource source) { std::string uriString{source.uri}; if (uriString.length() == 0) { m_onLoadEndEvent(*this, false); return; } + m_imageSource = source; + m_memoryStream = nullptr; + if (source.packagerAsset && uriString.find("file://") == 0) { uriString.replace(0, 7, source.bundleRootPath); } @@ -116,8 +116,6 @@ winrt::fire_and_forget ReactImage::Source(ImageSource source) { auto weak_this{get_weak()}; try { - m_imageSource = source; - if (needsDownload) { m_memoryStream = co_await GetImageStreamAsync(source); } else if (inlineData) { @@ -125,12 +123,17 @@ winrt::fire_and_forget ReactImage::Source(ImageSource source) { } if (auto strong_this{weak_this.get()}) { - if ((needsDownload || inlineData) && !strong_this->m_memoryStream) { + bool fromStream{needsDownload || inlineData}; + + // Fire failed load event if we're not loading from URI and the memory stream is null. + if (fromStream && !strong_this->m_memoryStream) { strong_this->m_onLoadEndEvent(*strong_this, false); + return; } - if (!needsDownload || strong_this->m_memoryStream) { - strong_this->SetBackground(needsDownload || inlineData, true); + // Verify we're loading from URI or we have a valid memory stream before calling SetBackground. + if (!fromStream || strong_this->m_memoryStream) { + strong_this->SetBackground(fromStream, true); } } } catch (winrt::hresult_error const &) { @@ -140,7 +143,7 @@ winrt::fire_and_forget ReactImage::Source(ImageSource source) { } } -void ReactImage::SetBackground(bool fromStream, bool fireLoadEndEvent) { +winrt::fire_and_forget ReactImage::SetBackground(bool fromStream, bool fireLoadEndEvent) { const winrt::Uri uri{Utf8ToUtf16(m_imageSource.uri)}; if (m_useCompositionBrush) { @@ -201,20 +204,30 @@ void ReactImage::SetBackground(bool fromStream, bool fireLoadEndEvent) { } }); - winrt::BitmapImage bitmap{}; + // get weak reference before any co_await calls + auto weak_this{get_weak()}; - if (fromStream) { - bitmap.SetSourceAsync(m_memoryStream); - } else { - bitmap.UriSource(uri); - } + try { + winrt::BitmapImage bitmap{}; + + if (fromStream) { + co_await bitmap.SetSourceAsync(m_memoryStream); + } else { + bitmap.UriSource(uri); + } - bitmapBrush.ImageSource(bitmap); - Background(bitmapBrush); + bitmapBrush.ImageSource(bitmap); + Background(bitmapBrush); + } catch (winrt::hresult_error const &) { + const auto strong_this{weak_this.get()}; + if (strong_this && fireLoadEndEvent) { + strong_this->m_onLoadEndEvent(*strong_this, false); + } + } } } -winrt::IAsyncOperation GetImageStreamAsync(ImageSource source) { +winrt::IAsyncOperation GetImageStreamAsync(ReactImageSource source) { try { co_await winrt::resume_background(); @@ -253,7 +266,7 @@ winrt::IAsyncOperation GetImageStreamAsync(Im return nullptr; } -winrt::IAsyncOperation GetImageInlineDataAsync(ImageSource source) { +winrt::IAsyncOperation GetImageInlineDataAsync(ReactImageSource source) { size_t start = source.uri.find(','); if (start == std::string::npos || start + 1 > source.uri.length()) return nullptr; diff --git a/vnext/ReactUWP/Views/Image/ReactImage.h b/vnext/ReactUWP/Views/Image/ReactImage.h index 24dbb3b0f29..03c680a9d14 100644 --- a/vnext/ReactUWP/Views/Image/ReactImage.h +++ b/vnext/ReactUWP/Views/Image/ReactImage.h @@ -17,7 +17,7 @@ namespace react { namespace uwp { -struct ImageSource { +struct ReactImageSource { std::string uri; std::string method; std::string bundleRootPath; @@ -44,10 +44,10 @@ struct ReactImage : winrt::Windows::UI::Xaml::Controls::CanvasT { void OnLoadEnd(winrt::event_token const &token) noexcept; // Public Properties - ImageSource Source() { + ReactImageSource Source() { return m_imageSource; } - winrt::fire_and_forget Source(ImageSource source); + winrt::fire_and_forget Source(ReactImageSource source); react::uwp::ResizeMode ResizeMode() { return m_resizeMode; @@ -55,12 +55,11 @@ struct ReactImage : winrt::Windows::UI::Xaml::Controls::CanvasT { void ResizeMode(react::uwp::ResizeMode value); private: - bool ShouldUseCompositionBrush(); winrt::Windows::UI::Xaml::Media::Stretch ResizeModeToStretch(react::uwp::ResizeMode value); - void SetBackground(bool fromStream, bool fireLoadEndEvent); + winrt::fire_and_forget SetBackground(bool fromStream, bool fireLoadEndEvent); bool m_useCompositionBrush{false}; - ImageSource m_imageSource; + ReactImageSource m_imageSource; winrt::Windows::Foundation::Size m_availableSize{}; react::uwp::ResizeMode m_resizeMode{ResizeMode::Contain}; winrt::Windows::Storage::Streams::InMemoryRandomAccessStream m_memoryStream{nullptr}; @@ -73,8 +72,8 @@ struct ReactImage : winrt::Windows::UI::Xaml::Controls::CanvasT { // Helper functions winrt::Windows::Foundation::IAsyncOperation -GetImageStreamAsync(ImageSource source); +GetImageStreamAsync(ReactImageSource source); winrt::Windows::Foundation::IAsyncOperation -GetImageInlineDataAsync(ImageSource source); +GetImageInlineDataAsync(ReactImageSource source); } // namespace uwp } // namespace react From b5fd641a3babd63d84615185016849f1bb77d6e3 Mon Sep 17 00:00:00 2001 From: Marlene Cota Date: Tue, 10 Dec 2019 16:41:37 -0800 Subject: [PATCH 11/17] only create ImageBrush and BitmapImage is needed --- vnext/ReactUWP/Views/Image/ImageViewManager.h | 3 +- vnext/ReactUWP/Views/Image/ReactImage.cpp | 74 +++++++++++-------- 2 files changed, 47 insertions(+), 30 deletions(-) diff --git a/vnext/ReactUWP/Views/Image/ImageViewManager.h b/vnext/ReactUWP/Views/Image/ImageViewManager.h index 1eb6c3e00ae..bae1507e798 100644 --- a/vnext/ReactUWP/Views/Image/ImageViewManager.h +++ b/vnext/ReactUWP/Views/Image/ImageViewManager.h @@ -19,7 +19,8 @@ class ImageViewManager : public FrameworkElementViewManager { folly::dynamic GetExportedCustomDirectEventTypeConstants() const override; folly::dynamic GetNativeProps() const override; facebook::react::ShadowNode *createShadow() const override; - void EmitImageEvent(winrt::Windows::UI::Xaml::Controls::Canvas canvas, const char *eventName, ReactImageSource &source); + void + EmitImageEvent(winrt::Windows::UI::Xaml::Controls::Canvas canvas, const char *eventName, ReactImageSource &source); protected: XamlView CreateViewCore(int64_t tag) override; diff --git a/vnext/ReactUWP/Views/Image/ReactImage.cpp b/vnext/ReactUWP/Views/Image/ReactImage.cpp index f47d3d88c10..2d5fdc332b6 100644 --- a/vnext/ReactUWP/Views/Image/ReactImage.cpp +++ b/vnext/ReactUWP/Views/Image/ReactImage.cpp @@ -63,8 +63,7 @@ void ReactImage::ResizeMode(react::uwp::ResizeMode value) { if (switchBrushes) { m_useCompositionBrush = shouldUseCompositionBrush; SetBackground(m_memoryStream != nullptr, false); - } else { - const auto bitmapBrush{Background().as()}; + } else if (auto bitmapBrush{Background().as()}) { bitmapBrush.Stretch(ResizeModeToStretch(m_resizeMode)); } } @@ -177,38 +176,48 @@ winrt::fire_and_forget ReactImage::SetBackground(bool fromStream, bool fireLoadE }); } else { - winrt::ImageBrush bitmapBrush{}; - - // ImageOpened and ImageFailed are mutually exclusive. One event of the other will - // always fire whenever an ImageBrush has the ImageSource value set or reset. - m_imageOpenedRevoker = bitmapBrush.ImageOpened( - winrt::auto_revoke, [weak_this{get_weak()}, bitmapBrush, fireLoadEndEvent](const auto &, const auto &) { - if (auto strong_this{weak_this.get()}) { - const auto bitmap{bitmapBrush.ImageSource().as()}; - strong_this->m_imageSource.height = bitmap.PixelHeight(); - strong_this->m_imageSource.width = bitmap.PixelWidth(); - - bitmapBrush.Stretch(strong_this->ResizeModeToStretch(strong_this->m_resizeMode)); - - if (fireLoadEndEvent) { - strong_this->m_onLoadEndEvent(*strong_this, true); + bool setBackground{false}; + winrt::ImageBrush bitmapBrush{Background().try_as()}; + if (!bitmapBrush) { + bitmapBrush = winrt::ImageBrush{}; + setBackground = true; + + // ImageOpened and ImageFailed are mutually exclusive. One event of the other will + // always fire whenever an ImageBrush has the ImageSource value set or reset. + m_imageOpenedRevoker = bitmapBrush.ImageOpened( + winrt::auto_revoke, [weak_this{get_weak()}, bitmapBrush, fireLoadEndEvent](const auto &, const auto &) { + if (auto strong_this{weak_this.get()}) { + const auto bitmap{bitmapBrush.ImageSource().as()}; + strong_this->m_imageSource.height = bitmap.PixelHeight(); + strong_this->m_imageSource.width = bitmap.PixelWidth(); + + bitmapBrush.Stretch(strong_this->ResizeModeToStretch(strong_this->m_resizeMode)); + + if (fireLoadEndEvent) { + strong_this->m_onLoadEndEvent(*strong_this, true); + } } - } - }); + }); - m_imageFailedRevoker = bitmapBrush.ImageFailed( - winrt::auto_revoke, [weak_this{get_weak()}, fireLoadEndEvent](const auto &, const auto &) { - const auto strong_this{weak_this.get()}; - if (strong_this && fireLoadEndEvent) { - strong_this->m_onLoadEndEvent(*strong_this, false); - } - }); + m_imageFailedRevoker = bitmapBrush.ImageFailed( + winrt::auto_revoke, [weak_this{get_weak()}, fireLoadEndEvent](const auto &, const auto &) { + const auto strong_this{weak_this.get()}; + if (strong_this && fireLoadEndEvent) { + strong_this->m_onLoadEndEvent(*strong_this, false); + } + }); + } // get weak reference before any co_await calls auto weak_this{get_weak()}; try { - winrt::BitmapImage bitmap{}; + winrt::BitmapImage bitmap{bitmapBrush.ImageSource().try_as()}; + bool setImageSource{false}; + if (!bitmap) { + bitmap = winrt::BitmapImage{}; + setImageSource = true; + } if (fromStream) { co_await bitmap.SetSourceAsync(m_memoryStream); @@ -216,8 +225,15 @@ winrt::fire_and_forget ReactImage::SetBackground(bool fromStream, bool fireLoadE bitmap.UriSource(uri); } - bitmapBrush.ImageSource(bitmap); - Background(bitmapBrush); + if (setImageSource) { + bitmapBrush.ImageSource(bitmap); + + auto strong_this{weak_this.get()}; + if (setBackground && strong_this) { + strong_this->Background(bitmapBrush); + } + } + } catch (winrt::hresult_error const &) { const auto strong_this{weak_this.get()}; if (strong_this && fireLoadEndEvent) { From 3ed5ca607c68f96148385e38f03e3e732fa8e6c6 Mon Sep 17 00:00:00 2001 From: Marlene Cota Date: Thu, 12 Dec 2019 14:08:24 -0800 Subject: [PATCH 12/17] Remove memory stream cache + flicker workaround --- .../ReactUWP/Views/Image/ImageViewManager.cpp | 4 + vnext/ReactUWP/Views/Image/ReactImage.cpp | 175 +++++++++--------- vnext/ReactUWP/Views/Image/ReactImage.h | 15 +- 3 files changed, 101 insertions(+), 93 deletions(-) diff --git a/vnext/ReactUWP/Views/Image/ImageViewManager.cpp b/vnext/ReactUWP/Views/Image/ImageViewManager.cpp index 6c5acb2aee6..9af905f6b5c 100644 --- a/vnext/ReactUWP/Views/Image/ImageViewManager.cpp +++ b/vnext/ReactUWP/Views/Image/ImageViewManager.cpp @@ -155,6 +155,10 @@ void ImageViewManager::setSource(winrt::Canvas canvas, const folly::dynamic &dat auto sources{json_type_traits>::parseJson(data)}; sources[0].bundleRootPath = instance->GetBundleRootPath(); + if (sources[0].packagerAsset && sources[0].uri.find("file://") == 0) { + sources[0].uri.replace(0, 7, sources[0].bundleRootPath); + } + auto reactImage{canvas.as()}; EmitImageEvent(canvas, "topLoadStart", sources[0]); diff --git a/vnext/ReactUWP/Views/Image/ReactImage.cpp b/vnext/ReactUWP/Views/Image/ReactImage.cpp index 2d5fdc332b6..979805c1112 100644 --- a/vnext/ReactUWP/Views/Image/ReactImage.cpp +++ b/vnext/ReactUWP/Views/Image/ReactImage.cpp @@ -62,7 +62,7 @@ void ReactImage::ResizeMode(react::uwp::ResizeMode value) { if (switchBrushes) { m_useCompositionBrush = shouldUseCompositionBrush; - SetBackground(m_memoryStream != nullptr, false); + SetBackground(false); } else if (auto bitmapBrush{Background().as()}) { bitmapBrush.Stretch(ResizeModeToStretch(m_resizeMode)); } @@ -82,8 +82,7 @@ winrt::Stretch ReactImage::ResizeModeToStretch(react::uwp::ResizeMode value) { // That is handled by the shouldUseCompositionBrush/switchBrushes code path. assert(value != ResizeMode::Repeat); - const auto bitmap{Background().as().ImageSource().as()}; - if (bitmap.PixelHeight() < m_availableSize.Height && bitmap.PixelWidth() < m_availableSize.Width) { + if (m_imageSource.height < m_availableSize.Height && m_imageSource.width < m_availableSize.Width) { return winrt::Stretch::None; } else { return winrt::Stretch::Uniform; @@ -91,78 +90,85 @@ winrt::Stretch ReactImage::ResizeModeToStretch(react::uwp::ResizeMode value) { } } -winrt::fire_and_forget ReactImage::Source(ReactImageSource source) { - std::string uriString{source.uri}; - if (uriString.length() == 0) { +void ReactImage::Source(ReactImageSource source) { + if (source.uri.length() == 0) { m_onLoadEndEvent(*this, false); return; } m_imageSource = source; - m_memoryStream = nullptr; - if (source.packagerAsset && uriString.find("file://") == 0) { - uriString.replace(0, 7, source.bundleRootPath); + winrt::Uri uri{Utf8ToUtf16(m_imageSource.uri)}; + winrt::hstring scheme{uri.SchemeName()}; + + if (((scheme == L"http") || (scheme == L"https")) && !m_imageSource.headers.empty()) { + m_imageSource.sourceType = ImageSourceType::Download; + } else if (scheme == L"data") { + m_imageSource.sourceType = ImageSourceType::InlineData; } - winrt::Uri uri{Utf8ToUtf16(uriString)}; - winrt::hstring scheme{uri.SchemeName()}; - bool needsDownload = - ((scheme == L"http") || (scheme == L"https")) && (m_useCompositionBrush || !source.headers.empty()); - bool inlineData = scheme == L"data"; + SetBackground(true); +} + +winrt::IAsyncOperation ReactImage::GetImageMemoryStreamAsync( + ReactImageSource source) { + switch (source.sourceType) { + case ImageSourceType::Download: + return co_await GetImageStreamAsync(source); + case ImageSourceType::InlineData: + return co_await GetImageInlineDataAsync(source); + default: // ImageSourceType::Uri + return nullptr; + } +} + +winrt::fire_and_forget ReactImage::SetBackground(bool fireLoadEndEvent) { + const ReactImageSource source{m_imageSource}; + const winrt::Uri uri{Utf8ToUtf16(source.uri)}; + const bool fromStream{source.sourceType != ImageSourceType::Uri}; + + winrt::InMemoryRandomAccessStream memoryStream{nullptr}; // get weak reference before any co_await calls auto weak_this{get_weak()}; try { - if (needsDownload) { - m_memoryStream = co_await GetImageStreamAsync(source); - } else if (inlineData) { - m_memoryStream = co_await GetImageInlineDataAsync(source); - } + memoryStream = co_await GetImageMemoryStreamAsync(source); - if (auto strong_this{weak_this.get()}) { - bool fromStream{needsDownload || inlineData}; - - // Fire failed load event if we're not loading from URI and the memory stream is null. - if (fromStream && !strong_this->m_memoryStream) { + // Fire failed load event if we're not loading from URI and the memory stream is null. + if (fromStream && !memoryStream) { + if (auto strong_this{weak_this.get()}) { strong_this->m_onLoadEndEvent(*strong_this, false); - return; - } - - // Verify we're loading from URI or we have a valid memory stream before calling SetBackground. - if (!fromStream || strong_this->m_memoryStream) { - strong_this->SetBackground(fromStream, true); } + return; } } catch (winrt::hresult_error const &) { - if (auto strong_this{weak_this.get()}) { + const auto strong_this{weak_this.get()}; + if (strong_this && fireLoadEndEvent) { strong_this->m_onLoadEndEvent(*strong_this, false); } } -} - -winrt::fire_and_forget ReactImage::SetBackground(bool fromStream, bool fireLoadEndEvent) { - const winrt::Uri uri{Utf8ToUtf16(m_imageSource.uri)}; - if (m_useCompositionBrush) { - const auto compositionBrush = ReactImageBrush::Create(); - compositionBrush->AvailableSize(m_availableSize); - compositionBrush->ResizeMode(m_resizeMode); + if (auto strong_this{weak_this.get()}) { + if (strong_this->m_useCompositionBrush) { + const auto compositionBrush{ReactImageBrush::Create()}; + compositionBrush->AvailableSize(strong_this->m_availableSize); + compositionBrush->ResizeMode(strong_this->m_resizeMode); - auto surface = fromStream ? winrt::LoadedImageSurface::StartLoadFromStream(m_memoryStream) - : winrt::LoadedImageSurface::StartLoadFromUri(uri); + const auto surface = fromStream ? winrt::LoadedImageSurface::StartLoadFromStream(memoryStream) + : winrt::LoadedImageSurface::StartLoadFromUri(uri); - m_surfaceLoadedRevoker = surface.LoadCompleted( + strong_this->m_surfaceLoadedRevoker = surface.LoadCompleted( winrt::auto_revoke, - [weak_this{get_weak()}, compositionBrush, surface, fireLoadEndEvent]( - winrt::LoadedImageSurface const & /*sender*/, winrt::LoadedImageSourceLoadCompletedEventArgs const &args) { + [weak_this, compositionBrush, surface, fireLoadEndEvent]( + winrt::LoadedImageSurface const & /*sender*/, + winrt::LoadedImageSourceLoadCompletedEventArgs const &args) { if (auto strong_this{weak_this.get()}) { bool succeeded{false}; if (args.Status() == winrt::LoadedImageSourceLoadStatus::Success) { - winrt::Size dipsSize{surface.DecodedSize()}; - strong_this->m_imageSource.height = dipsSize.Height; - strong_this->m_imageSource.width = dipsSize.Width; + winrt::Size size{surface.DecodedPhysicalSize()}; + strong_this->m_imageSource.height = size.Height; + strong_this->m_imageSource.width = size.Width; compositionBrush->Source(surface); strong_this->Background(compositionBrush.as()); @@ -174,24 +180,22 @@ winrt::fire_and_forget ReactImage::SetBackground(bool fromStream, bool fireLoadE } } }); - - } else { - bool setBackground{false}; - winrt::ImageBrush bitmapBrush{Background().try_as()}; - if (!bitmapBrush) { - bitmapBrush = winrt::ImageBrush{}; - setBackground = true; - - // ImageOpened and ImageFailed are mutually exclusive. One event of the other will - // always fire whenever an ImageBrush has the ImageSource value set or reset. - m_imageOpenedRevoker = bitmapBrush.ImageOpened( - winrt::auto_revoke, [weak_this{get_weak()}, bitmapBrush, fireLoadEndEvent](const auto &, const auto &) { + } else { + winrt::ImageBrush imageBrush{strong_this->Background().try_as()}; + bool createImageBrush{!imageBrush}; + 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 &) { if (auto strong_this{weak_this.get()}) { - const auto bitmap{bitmapBrush.ImageSource().as()}; + const auto bitmap{imageBrush.ImageSource().as()}; strong_this->m_imageSource.height = bitmap.PixelHeight(); strong_this->m_imageSource.width = bitmap.PixelWidth(); - bitmapBrush.Stretch(strong_this->ResizeModeToStretch(strong_this->m_resizeMode)); + imageBrush.Stretch(strong_this->ResizeModeToStretch(strong_this->m_resizeMode)); if (fireLoadEndEvent) { strong_this->m_onLoadEndEvent(*strong_this, true); @@ -199,45 +203,40 @@ winrt::fire_and_forget ReactImage::SetBackground(bool fromStream, bool fireLoadE } }); - m_imageFailedRevoker = bitmapBrush.ImageFailed( - winrt::auto_revoke, [weak_this{get_weak()}, fireLoadEndEvent](const auto &, const auto &) { + 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); } }); - } + } - // get weak reference before any co_await calls - auto weak_this{get_weak()}; + winrt::BitmapImage bitmapImage{imageBrush.ImageSource().try_as()}; - try { - winrt::BitmapImage bitmap{bitmapBrush.ImageSource().try_as()}; - bool setImageSource{false}; - if (!bitmap) { - bitmap = winrt::BitmapImage{}; - setImageSource = true; - } + if (!bitmapImage) { + bitmapImage = winrt::BitmapImage{}; - if (fromStream) { - co_await bitmap.SetSourceAsync(m_memoryStream); - } else { - bitmap.UriSource(uri); - } + strong_this->m_bitmapImageOpened = bitmapImage.ImageOpened( + winrt::auto_revoke, [imageBrush](const auto &, const auto &) { + imageBrush.Opacity(1); + }); - if (setImageSource) { - bitmapBrush.ImageSource(bitmap); + imageBrush.ImageSource(bitmapImage); + } - auto strong_this{weak_this.get()}; - if (setBackground && strong_this) { - strong_this->Background(bitmapBrush); - } + if (createImageBrush) { + strong_this->Background(imageBrush); } - } catch (winrt::hresult_error const &) { - const auto strong_this{weak_this.get()}; - if (strong_this && fireLoadEndEvent) { - strong_this->m_onLoadEndEvent(*strong_this, false); + if (fromStream) { + co_await bitmapImage.SetSourceAsync(memoryStream); + } else { + bitmapImage.UriSource(uri); + + // TODO: When we change the source of a BitmapImage, we're getting a flicker of the old image + // being resized to the size of the new image. This is a temporary workaround. + imageBrush.Opacity(0); } } } diff --git a/vnext/ReactUWP/Views/Image/ReactImage.h b/vnext/ReactUWP/Views/Image/ReactImage.h index 03c680a9d14..7b0c85ef5ba 100644 --- a/vnext/ReactUWP/Views/Image/ReactImage.h +++ b/vnext/ReactUWP/Views/Image/ReactImage.h @@ -17,6 +17,8 @@ namespace react { namespace uwp { +enum class ImageSourceType { Uri = 0, Download = 1, InlineData = 2 }; + struct ReactImageSource { std::string uri; std::string method; @@ -26,6 +28,7 @@ struct ReactImageSource { double height = 0; double scale = 1.0; bool packagerAsset = false; + ImageSourceType sourceType = ImageSourceType::Uri; }; struct ReactImage : winrt::Windows::UI::Xaml::Controls::CanvasT { @@ -47,7 +50,7 @@ struct ReactImage : winrt::Windows::UI::Xaml::Controls::CanvasT { ReactImageSource Source() { return m_imageSource; } - winrt::fire_and_forget Source(ReactImageSource source); + void Source(ReactImageSource source); react::uwp::ResizeMode ResizeMode() { return m_resizeMode; @@ -56,18 +59,20 @@ struct ReactImage : winrt::Windows::UI::Xaml::Controls::CanvasT { private: winrt::Windows::UI::Xaml::Media::Stretch ResizeModeToStretch(react::uwp::ResizeMode value); - winrt::fire_and_forget SetBackground(bool fromStream, bool fireLoadEndEvent); + winrt::Windows::Foundation::IAsyncOperation + GetImageMemoryStreamAsync(ReactImageSource source); + winrt::fire_and_forget SetBackground(bool fireLoadEndEvent); bool m_useCompositionBrush{false}; ReactImageSource m_imageSource; winrt::Windows::Foundation::Size m_availableSize{}; react::uwp::ResizeMode m_resizeMode{ResizeMode::Contain}; - winrt::Windows::Storage::Streams::InMemoryRandomAccessStream m_memoryStream{nullptr}; winrt::event> m_onLoadEndEvent; winrt::Windows::UI::Xaml::Media::LoadedImageSurface::LoadCompleted_revoker m_surfaceLoadedRevoker; - winrt::Windows::UI::Xaml::Media::ImageBrush::ImageOpened_revoker m_imageOpenedRevoker; - winrt::Windows::UI::Xaml::Media::ImageBrush::ImageFailed_revoker m_imageFailedRevoker; + winrt::Windows::UI::Xaml::Media::Imaging::BitmapImage::ImageOpened_revoker m_bitmapImageOpened; + winrt::Windows::UI::Xaml::Media::ImageBrush::ImageOpened_revoker m_imageBrushOpenedRevoker; + winrt::Windows::UI::Xaml::Media::ImageBrush::ImageFailed_revoker m_imageBrushFailedRevoker; }; // Helper functions From adf03c00e568011b8244387ab6dc6b21c1710fae Mon Sep 17 00:00:00 2001 From: Marlene Cota Date: Thu, 12 Dec 2019 14:34:19 -0800 Subject: [PATCH 13/17] don't cache availablesize + formatting --- vnext/ReactUWP/Views/Image/ReactImage.cpp | 82 +++++++++++------------ vnext/ReactUWP/Views/Image/ReactImage.h | 1 - 2 files changed, 39 insertions(+), 44 deletions(-) diff --git a/vnext/ReactUWP/Views/Image/ReactImage.cpp b/vnext/ReactUWP/Views/Image/ReactImage.cpp index 979805c1112..319c9f8e3d3 100644 --- a/vnext/ReactUWP/Views/Image/ReactImage.cpp +++ b/vnext/ReactUWP/Views/Image/ReactImage.cpp @@ -35,8 +35,6 @@ namespace uwp { } winrt::Size ReactImage::ArrangeOverride(winrt::Size finalSize) { - m_availableSize = finalSize; - if (m_useCompositionBrush) { auto brush{Background().as()}; brush->AvailableSize(finalSize); @@ -82,7 +80,7 @@ winrt::Stretch ReactImage::ResizeModeToStretch(react::uwp::ResizeMode value) { // That is handled by the shouldUseCompositionBrush/switchBrushes code path. assert(value != ResizeMode::Repeat); - if (m_imageSource.height < m_availableSize.Height && m_imageSource.width < m_availableSize.Width) { + if (m_imageSource.height < ActualHeight() && m_imageSource.width < ActualWidth()) { return winrt::Stretch::None; } else { return winrt::Stretch::Uniform; @@ -152,34 +150,34 @@ winrt::fire_and_forget ReactImage::SetBackground(bool fireLoadEndEvent) { if (auto strong_this{weak_this.get()}) { if (strong_this->m_useCompositionBrush) { const auto compositionBrush{ReactImageBrush::Create()}; - compositionBrush->AvailableSize(strong_this->m_availableSize); + compositionBrush->AvailableSize(strong_this->ActualSize()); compositionBrush->ResizeMode(strong_this->m_resizeMode); const auto surface = fromStream ? winrt::LoadedImageSurface::StartLoadFromStream(memoryStream) : winrt::LoadedImageSurface::StartLoadFromUri(uri); strong_this->m_surfaceLoadedRevoker = surface.LoadCompleted( - winrt::auto_revoke, - [weak_this, compositionBrush, surface, fireLoadEndEvent]( - winrt::LoadedImageSurface const & /*sender*/, - winrt::LoadedImageSourceLoadCompletedEventArgs const &args) { - if (auto strong_this{weak_this.get()}) { - bool succeeded{false}; - if (args.Status() == winrt::LoadedImageSourceLoadStatus::Success) { - winrt::Size size{surface.DecodedPhysicalSize()}; - strong_this->m_imageSource.height = size.Height; - strong_this->m_imageSource.width = size.Width; - - compositionBrush->Source(surface); - strong_this->Background(compositionBrush.as()); - succeeded = true; - } + winrt::auto_revoke, + [weak_this, compositionBrush, surface, fireLoadEndEvent]( + winrt::LoadedImageSurface const & /*sender*/, + winrt::LoadedImageSourceLoadCompletedEventArgs const &args) { + if (auto strong_this{weak_this.get()}) { + bool succeeded{false}; + if (args.Status() == winrt::LoadedImageSourceLoadStatus::Success) { + winrt::Size size{surface.DecodedPhysicalSize()}; + strong_this->m_imageSource.height = size.Height; + strong_this->m_imageSource.width = size.Width; + + compositionBrush->Source(surface); + strong_this->Background(compositionBrush.as()); + succeeded = true; + } - if (fireLoadEndEvent) { - strong_this->m_onLoadEndEvent(*strong_this, succeeded); + if (fireLoadEndEvent) { + strong_this->m_onLoadEndEvent(*strong_this, succeeded); + } } - } - }); + }); } else { winrt::ImageBrush imageBrush{strong_this->Background().try_as()}; bool createImageBrush{!imageBrush}; @@ -189,27 +187,27 @@ winrt::fire_and_forget ReactImage::SetBackground(bool fireLoadEndEvent) { // 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 &) { - if (auto strong_this{weak_this.get()}) { - const auto bitmap{imageBrush.ImageSource().as()}; - strong_this->m_imageSource.height = bitmap.PixelHeight(); - strong_this->m_imageSource.width = bitmap.PixelWidth(); + winrt::auto_revoke, [weak_this, imageBrush, fireLoadEndEvent](const auto &, const auto &) { + if (auto strong_this{weak_this.get()}) { + const auto bitmap{imageBrush.ImageSource().as()}; + strong_this->m_imageSource.height = bitmap.PixelHeight(); + strong_this->m_imageSource.width = bitmap.PixelWidth(); - imageBrush.Stretch(strong_this->ResizeModeToStretch(strong_this->m_resizeMode)); + imageBrush.Stretch(strong_this->ResizeModeToStretch(strong_this->m_resizeMode)); - if (fireLoadEndEvent) { - strong_this->m_onLoadEndEvent(*strong_this, true); + 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); - } - }); + 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); + } + }); } winrt::BitmapImage bitmapImage{imageBrush.ImageSource().try_as()}; @@ -218,9 +216,7 @@ 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](const auto &, const auto &) { imageBrush.Opacity(1); }); imageBrush.ImageSource(bitmapImage); } diff --git a/vnext/ReactUWP/Views/Image/ReactImage.h b/vnext/ReactUWP/Views/Image/ReactImage.h index 7b0c85ef5ba..8d49a4b2281 100644 --- a/vnext/ReactUWP/Views/Image/ReactImage.h +++ b/vnext/ReactUWP/Views/Image/ReactImage.h @@ -65,7 +65,6 @@ struct ReactImage : winrt::Windows::UI::Xaml::Controls::CanvasT { bool m_useCompositionBrush{false}; ReactImageSource m_imageSource; - winrt::Windows::Foundation::Size m_availableSize{}; react::uwp::ResizeMode m_resizeMode{ResizeMode::Contain}; winrt::event> m_onLoadEndEvent; From 6909f6a5232433ca0f48b9011ee79883ba2dab89 Mon Sep 17 00:00:00 2001 From: Marlene Cota Date: Fri, 13 Dec 2019 17:35:44 -0800 Subject: [PATCH 14/17] SizeChanged event handler --- vnext/ReactUWP/Views/Image/ReactImage.cpp | 13 ++++++++++--- vnext/ReactUWP/Views/Image/ReactImage.h | 1 + 2 files changed, 11 insertions(+), 3 deletions(-) diff --git a/vnext/ReactUWP/Views/Image/ReactImage.cpp b/vnext/ReactUWP/Views/Image/ReactImage.cpp index 319c9f8e3d3..f2b9d04dfef 100644 --- a/vnext/ReactUWP/Views/Image/ReactImage.cpp +++ b/vnext/ReactUWP/Views/Image/ReactImage.cpp @@ -36,8 +36,9 @@ namespace uwp { winrt::Size ReactImage::ArrangeOverride(winrt::Size finalSize) { if (m_useCompositionBrush) { - auto brush{Background().as()}; - brush->AvailableSize(finalSize); + if (auto brush{Background().try_as()}) { + brush->AvailableSize(finalSize); + } } return finalSize; @@ -150,12 +151,16 @@ winrt::fire_and_forget ReactImage::SetBackground(bool fireLoadEndEvent) { if (auto strong_this{weak_this.get()}) { if (strong_this->m_useCompositionBrush) { const auto compositionBrush{ReactImageBrush::Create()}; - compositionBrush->AvailableSize(strong_this->ActualSize()); compositionBrush->ResizeMode(strong_this->m_resizeMode); const auto surface = fromStream ? winrt::LoadedImageSurface::StartLoadFromStream(memoryStream) : winrt::LoadedImageSurface::StartLoadFromUri(uri); + m_sizeChangedRevoker = strong_this->SizeChanged( + winrt::auto_revoke, [compositionBrush](const auto &, const winrt::SizeChangedEventArgs &args) { + compositionBrush->AvailableSize(args.NewSize()); + }); + strong_this->m_surfaceLoadedRevoker = surface.LoadCompleted( winrt::auto_revoke, [weak_this, compositionBrush, surface, fireLoadEndEvent]( @@ -176,6 +181,8 @@ winrt::fire_and_forget ReactImage::SetBackground(bool fireLoadEndEvent) { if (fireLoadEndEvent) { strong_this->m_onLoadEndEvent(*strong_this, succeeded); } + + strong_this->m_sizeChangedRevoker.revoke(); } }); } else { diff --git a/vnext/ReactUWP/Views/Image/ReactImage.h b/vnext/ReactUWP/Views/Image/ReactImage.h index 8d49a4b2281..10313399505 100644 --- a/vnext/ReactUWP/Views/Image/ReactImage.h +++ b/vnext/ReactUWP/Views/Image/ReactImage.h @@ -68,6 +68,7 @@ struct ReactImage : winrt::Windows::UI::Xaml::Controls::CanvasT { react::uwp::ResizeMode m_resizeMode{ResizeMode::Contain}; winrt::event> m_onLoadEndEvent; + winrt::Windows::UI::Xaml::FrameworkElement::SizeChanged_revoker m_sizeChangedRevoker; winrt::Windows::UI::Xaml::Media::LoadedImageSurface::LoadCompleted_revoker m_surfaceLoadedRevoker; winrt::Windows::UI::Xaml::Media::Imaging::BitmapImage::ImageOpened_revoker m_bitmapImageOpened; winrt::Windows::UI::Xaml::Media::ImageBrush::ImageOpened_revoker m_imageBrushOpenedRevoker; From 219ee3add71bc85bb645b80b468a169fec2fe687 Mon Sep 17 00:00:00 2001 From: Marlene Cota Date: Fri, 13 Dec 2019 18:10:05 -0800 Subject: [PATCH 15/17] Fix dynamic resizeMode switch edge case --- vnext/ReactUWP/Views/Image/ReactImage.cpp | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/vnext/ReactUWP/Views/Image/ReactImage.cpp b/vnext/ReactUWP/Views/Image/ReactImage.cpp index f2b9d04dfef..e24bb4df1a4 100644 --- a/vnext/ReactUWP/Views/Image/ReactImage.cpp +++ b/vnext/ReactUWP/Views/Image/ReactImage.cpp @@ -159,7 +159,7 @@ winrt::fire_and_forget ReactImage::SetBackground(bool fireLoadEndEvent) { m_sizeChangedRevoker = strong_this->SizeChanged( winrt::auto_revoke, [compositionBrush](const auto &, const winrt::SizeChangedEventArgs &args) { compositionBrush->AvailableSize(args.NewSize()); - }); + }); strong_this->m_surfaceLoadedRevoker = surface.LoadCompleted( winrt::auto_revoke, @@ -173,6 +173,13 @@ winrt::fire_and_forget ReactImage::SetBackground(bool fireLoadEndEvent) { strong_this->m_imageSource.height = size.Height; strong_this->m_imageSource.width = size.Width; + // If we are dynamically switching the resizeMode to 'repeat', then + // the SizeChanged event has already fired and the ReactImageBrush's + // size has not been set. Use ActualSize in that case. + if (compositionBrush->AvailableSize() == winrt::Size{0, 0}) { + compositionBrush->AvailableSize(strong_this->ActualSize()); + } + compositionBrush->Source(surface); strong_this->Background(compositionBrush.as()); succeeded = true; From 44714bd9ca213a3958043572784d29860cb70a61 Mon Sep 17 00:00:00 2001 From: Marlene Cota Date: Tue, 17 Dec 2019 15:54:10 -0800 Subject: [PATCH 16/17] Fix merge conflict + add inline data to image playground --- packages/playground/Samples/image.tsx | 109 ++++++++++-------- .../ReactUWP/Views/Image/ImageViewManager.cpp | 2 +- vnext/ReactUWP/Views/Image/ImageViewManager.h | 2 +- yarn.lock | 7 +- 4 files changed, 64 insertions(+), 56 deletions(-) diff --git a/packages/playground/Samples/image.tsx b/packages/playground/Samples/image.tsx index 96628fb6b42..816d7e40b23 100644 --- a/packages/playground/Samples/image.tsx +++ b/packages/playground/Samples/image.tsx @@ -12,79 +12,85 @@ const largeImageUri = const smallImageUri = 'http://facebook.github.io/react-native/img/header_logo.png'; +const dataImageUri = + 'data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAADMAAAAzCAYAAAA6oTAqAAAAEXRFWHRTb2Z0d2FyZQBwbmdjcnVzaEB1SfMAAABQSURBVGje7dSxCQBACARB+2/ab8BEeQNhFi6WSYzYLYudDQYGBgYGBgYGBgYGBgYGBgZmcvDqYGBgmhivGQYGBgYGBgYGBgYGBgYGBgbmQw+P/eMrC5UTVAAAAABJRU5ErkJggg=='; + export default class Bootstrap extends React.Component< {}, { - selectedResizeMode: - | 'center' - | 'stretch' - | 'cover' - | 'contain' - | 'repeat' - | undefined; - useLargeImage: boolean; + selectedResizeMode: string; inlcudeBorder: boolean; - imageUrl: string; + selectedSource: string; + imageUri: string; } > { state = { - selectedResizeMode: 'center' as 'center', - useLargeImage: false, + selectedResizeMode: 'center', + selectedSource: 'small', inlcudeBorder: false, - imageUrl: 'http://facebook.github.io/react-native/img/header_logo.png', + imageUri: 'http://facebook.github.io/react-native/img/header_logo.png', }; - switchImageUrl = () => { - const useLargeImage = !this.state.useLargeImage; - this.setState({useLargeImage}); + switchImageUri = (value: string) => { + this.setState({selectedSource: value}); + + let imageUri = ''; + + if (value === 'small') { + imageUri = smallImageUri; + } else if (value === 'large') { + imageUri = largeImageUri; + } else if (value === 'data') { + imageUri = dataImageUri; + } - const imageUrl = useLargeImage ? largeImageUri : smallImageUri; - this.setState({imageUrl}); + this.setState({imageUri}); }; render() { return ( - - ResizeMode - - this.setState({selectedResizeMode: value}) - }> - - - - - - - - - Image Size - Small - - Large - - - No Border - this.setState({inlcudeBorder: value})} - /> - Round Border - + ResizeMode + this.setState({selectedResizeMode: value})}> + + + + + + + + + Image Source + this.switchImageUri(value)}> + + + + + + + No Border + + this.setState({inlcudeBorder: value}) + } + /> + Round Border @@ -103,6 +109,7 @@ const styles = StyleSheet.create({ rowContainer: { flexDirection: 'row', alignItems: 'center', + marginBottom: 5, }, imageContainer: { marginTop: 5, @@ -125,7 +132,7 @@ const styles = StyleSheet.create({ title: { fontWeight: 'bold', margin: 5, - width: 80, + width: 100, }, }); diff --git a/vnext/ReactUWP/Views/Image/ImageViewManager.cpp b/vnext/ReactUWP/Views/Image/ImageViewManager.cpp index e765f154bad..9b0214528ec 100644 --- a/vnext/ReactUWP/Views/Image/ImageViewManager.cpp +++ b/vnext/ReactUWP/Views/Image/ImageViewManager.cpp @@ -143,7 +143,7 @@ void ImageViewManager::UpdateProperties(ShadowNodeBase *nodeToUpdate, const foll UpdateCornerRadiusOnElement(nodeToUpdate, grid); } -void ImageViewManager::EmitImageEvent(winrt::Grid grid, const char *eventName, ImageSource &source) { +void ImageViewManager::EmitImageEvent(winrt::Grid grid, const char *eventName, ReactImageSource &source) { auto reactInstance{m_wkReactInstance.lock()}; if (reactInstance == nullptr) return; diff --git a/vnext/ReactUWP/Views/Image/ImageViewManager.h b/vnext/ReactUWP/Views/Image/ImageViewManager.h index c22697ab611..a27d18c7eab 100644 --- a/vnext/ReactUWP/Views/Image/ImageViewManager.h +++ b/vnext/ReactUWP/Views/Image/ImageViewManager.h @@ -19,7 +19,7 @@ class ImageViewManager : public FrameworkElementViewManager { folly::dynamic GetExportedCustomDirectEventTypeConstants() const override; folly::dynamic GetNativeProps() const override; facebook::react::ShadowNode *createShadow() const override; - void EmitImageEvent(winrt::Windows::UI::Xaml::Controls::Grid grid, const char *eventName, ImageSource &source); + void EmitImageEvent(winrt::Windows::UI::Xaml::Controls::Grid grid, const char *eventName, ReactImageSource &source); protected: XamlView CreateViewCore(int64_t tag) override; diff --git a/yarn.lock b/yarn.lock index 222def14e8d..a3549b7051f 100644 --- a/yarn.lock +++ b/yarn.lock @@ -10054,9 +10054,10 @@ react-is@^16.8.1, react-is@^16.8.4, react-is@^16.8.6: resolved "https://registry.yarnpkg.com/react-is/-/react-is-16.9.0.tgz#21ca9561399aad0ff1a7701c01683e8ca981edcb" integrity sha512-tJBzzzIgnnRfEm046qRcURvwQnZVXmuCbscxUO5RWrGTXpon2d4c8mI0D8WE6ydVIm29JiLB6+RslkIvym9Rjw== -"react-native@https://github.com/microsoft/react-native/archive/v0.60.0-microsoft.28.tar.gz": - version "0.60.0-microsoft.28" - resolved "https://github.com/microsoft/react-native/archive/v0.60.0-microsoft.28.tar.gz#fad0f343cf2ee7c277f89b86f770efa71872839e" +"react-native@https://github.com/microsoft/react-native/archive/v0.60.0-microsoft.31.tar.gz": + version "0.60.0-microsoft.31" + uid "26b4041e78b54517e3494beb6478bc7ee0a3a726" + resolved "https://github.com/microsoft/react-native/archive/v0.60.0-microsoft.31.tar.gz#26b4041e78b54517e3494beb6478bc7ee0a3a726" dependencies: "@babel/core" "^7.4.0" "@babel/generator" "^7.4.0" From 1dcde3d896003de1641a2c62e5d2ae477e38c625 Mon Sep 17 00:00:00 2001 From: Marlene Cota Date: Tue, 17 Dec 2019 16:09:31 -0800 Subject: [PATCH 17/17] fix playground buildci --- packages/playground/Samples/image.tsx | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/packages/playground/Samples/image.tsx b/packages/playground/Samples/image.tsx index 816d7e40b23..1e0a8be5533 100644 --- a/packages/playground/Samples/image.tsx +++ b/packages/playground/Samples/image.tsx @@ -18,14 +18,20 @@ const dataImageUri = export default class Bootstrap extends React.Component< {}, { - selectedResizeMode: string; + selectedResizeMode: + | 'center' + | 'stretch' + | 'cover' + | 'contain' + | 'repeat' + | undefined; inlcudeBorder: boolean; selectedSource: string; imageUri: string; } > { state = { - selectedResizeMode: 'center', + selectedResizeMode: 'center' as 'center', selectedSource: 'small', inlcudeBorder: false, imageUri: 'http://facebook.github.io/react-native/img/header_logo.png',