Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for Progressive JPEG. #188

Open
oferRounds opened this issue Apr 22, 2018 · 15 comments
Open

Add support for Progressive JPEG. #188

oferRounds opened this issue Apr 22, 2018 · 15 comments
Labels
feature A new feature to add.

Comments

@oferRounds
Copy link

oferRounds commented Apr 22, 2018

Hi!

Thanks for this awesome library!

I wanted to ask please: does the library support progressive JPEG?

Thanks!

@DylanVann DylanVann changed the title Question: Support for progressive JPEG? Apr 25, 2018
@DylanVann DylanVann added the feature A new feature to add. label Apr 25, 2018
@DylanVann
Copy link
Owner

Thanks, glad you've found it useful. Progressive JPEG probably won't be supported anytime soon. It's not officially supported by either of the underlying native libraries. By not supported I mean I believe it will display them, but it will not display them progressively.

I guess Progressive JPEG support is one nice feature of Fresco.

@oferRounds
Copy link
Author

oferRounds commented Apr 25, 2018

Cool, thanks! I know it’s a big change, was it considered to use PinRemoteImage? It supports P-JPEG, and also – coming from iOS – I found it better to work with comparing to SDWebImage.

@oferRounds
Copy link
Author

Just looked on Fresco - yes, it looks great with their built in support for both progressive JPEGs and WebPs.

Maybe it‘s worth checking the option to migrate to Fresco and PinRemoteImage.

@DylanVann
Copy link
Owner

There isn't much chance of migrating to Fresco since problems with Fresco are why I made this in the first place.

Last time I checked SDWebImage was one of the fastest / most performant image loading libraries, so I wouldn't want to switch off unless I'm sure PINRemoteImage can provide the same performance. It's also still in beta.

@oferRounds
Copy link
Author

oferRounds commented May 7, 2018

I understand, no problem. But just to mention: as far as I know – and according to their page also – PINRemoteImage is not in beta. Pinterest themselves use it in their iOS production app. I also used it in the past on my non-RN app.

@DylanVann
Copy link
Owner

Oh, I suppose it's just that the pod on master is in beta. I didn't catch that.

I'll take a closer look. It does look nice, and having support for all those formats in one dependency is great. I'd also need to look into how to set headers, and how to preload.

There is an issue open to moving to another library already because there are some issues I have with SDWebImage: #13

I guess the most challenging thing would be determining if it's actually faster. Relevant article: https://bpoplauschi.wordpress.com/2014/03/21/ios-image-caching-sdwebimage-vs-fastimage/

@oferRounds
Copy link
Author

Oh, cool!

@oferRounds
Copy link
Author

@DylanVann any thoughts on this?

@tgensol
Copy link

tgensol commented Dec 29, 2018

Any news on this ? 😃

@oferRounds
Copy link
Author

oferRounds commented Jun 12, 2019

@DylanVann any new thoughts on this? I see that SDWebImage supports it (by passing SDWebImageProgressiveDownload on the options param)

@JimTeva
Copy link

JimTeva commented May 14, 2020

Hi @DylanVann, thank you for the great library.
Could you please add SDWebImageProgressiveLoad as it is now implemented in SDWebImage here. Thank you

@romanravkov
Copy link

@DylanVann any news? we are really looking forward to :)

@tgensol
Copy link

tgensol commented Nov 18, 2020

I need to do more tests, but It looks like just by adding  SDWebImageProgressiveLoad in the SDWebImageOptions options of this file https://github.com/DylanVann/react-native-fast-image/blob/master/ios/FastImage/FFFastImageView.m (line 159) does the trick. My app seems to display progressive jpeg better

diff --git a/node_modules/react-native-fast-image/ios/FastImage/FFFastImageView.m b/node_modules/react-native-fast-image/ios/FastImage/FFFastImageView.m
index c5f79b4..fc2a601 100644
--- a/node_modules/react-native-fast-image/ios/FastImage/FFFastImageView.m
+++ b/node_modules/react-native-fast-image/ios/FastImage/FFFastImageView.m
@@ -156,7 +156,7 @@ - (void)reloadImage
         SDWebImageContext *context = @{SDWebImageContextDownloadRequestModifier : requestModifier};
         
         // Set priority.
-        SDWebImageOptions options = SDWebImageRetryFailed | SDWebImageHandleCookies;
+        SDWebImageOptions options = SDWebImageRetryFailed | SDWebImageHandleCookies | SDWebImageProgressiveLoad;
         switch (_source.priority) {
             case FFFPriorityLow:
                 options |= SDWebImageLowPriority;
@oferRounds
Copy link
Author

I’ve tried this approach, and it didn’t work well for me. I suggest moving to PinRemoteImage – I’ve tested it as a fork of this branch on Production for a while (switched the iOS depedency), and it works great

@tgensol
Copy link

tgensol commented Nov 25, 2020

On my side it is working just as expected, so I will keep the SDWebImageProgressiveLoad

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature A new feature to add.
5 participants