printdotio / printio-ios-sdk

SDK that enables the printing of any photo, from any source, onto any product!
Other
21 stars 11 forks source link

Low: Pre-uploaded images + thumbnails when using `setImages` #267

Closed Xyand closed 7 years ago

Xyand commented 10 years ago

In our use case we pre-upload the photos to a remote server in the background and then send them to the SDK using setImages. If I specify the local images then the SDK would re-upload them, if I specify the remote urls then it would take time for the SDK to show the thumbnails. Is there a way to specify both a remote url and a local one?

If not, then it could be a really useful feature.

perisicboro commented 10 years ago

@Xyand setImages accepts both, UIImage objects and remote urls in same array.

Xyand commented 10 years ago

@perisicboro I know. what I would actually like to send is both remote image url, and local image UIImage/ALAsset/PHAsset for the same image. This way every image will have a remote full resolution copy that won't have to be re-uploaded, and a local accessible copy to be used for design and thumbnails. This is what do in out app at the moment.

Btw, what UIImage should I provide? Full resolution? Doesn't it cause memory issues when handling many images?

AustenB commented 10 years ago

Albert - I believe @perisicboro https://github.com/perisicboro is saying you can pass them both in together. Is this what you want?

Boro let Albert know if we support sending in small files for use client side and larger files to use as full URLs

On Mon, Oct 13, 2014 at 2:45 PM, Albert notifications@github.com wrote:

@perisicboro https://github.com/perisicboro I know. what I would actually like to send is both remote image url, and local image UIImage/ALAsset/PHAsset for the same image. This way every image will have a remote full resolution copy that won't have to be re-uploaded, and a local accessible copy to be used for design and thumbnails. This is what do in out app at the moment.

Btw, what UIImage should I provide? Full resolution? Doesn't it cause memory issues when handling many images?

— Reply to this email directly or view it on GitHub https://github.com/printdotio/printio-ios-sdk/issues/267#issuecomment-58936476 .

Austen Bernstein Founder & CEO printio

Xyand commented 10 years ago

From what I understand @perisicboro is saying that I can mix different kinds of images likes this:

NSArray *images = @[@"http://www.google.com/landscape.jpeg", image]; 
[printIO setImages:images];

What I would like to do is (symbolic writing - please ignore specific interface):

// Local thumbnail and pre-uploaded remote image
NSArray *images = @[@{'thumb': uiimage_thumb, 'full': @'http:/google.com/img.jpg'}, ...]; 

// Or local thumbnail and local image for upload (for memory effciency)
NSArray *images = @[@{'thumb': uiimage_thumb, 'full': local_path_or_alasset_instance}, ...]; 
AustenB commented 10 years ago

I will let @perisicboro https://github.com/perisicboro respond.

On Tue, Oct 14, 2014 at 7:48 AM, Albert notifications@github.com wrote:

From what I understand @perisicboro https://github.com/perisicboro is saying that I can mix different kinds of images likes this:

NSArray *images = @[@"http://www.google.com/landscape.jpeg", image]; [printIO setImages:images];

What I would like to do is (symbolic writing - please ignore specific interface):

// Local thumbnail and pre-uploaded remote image NSArray *images = @[@{'thumb': uiimage_thumb, 'full': @'http:/google.com/img.jpg'}, ...];

// Or local thumbnail and local image for upload (for memory effciency) NSArray *images = @[@{'thumb': uiimage_thumb, 'full': local_path_or_alasset_instance}, ...];

— Reply to this email directly or view it on GitHub https://github.com/printdotio/printio-ios-sdk/issues/267#issuecomment-59030090 .

Austen Bernstein Founder & CEO printio

AustenB commented 10 years ago

@Xyand this is difficult because of the customization features that exist in our SDK - if you use a 200x200 thumbnail and someone zooms on customization it wont look right. Do you have ideas on how to avoid this issue?

Xyand commented 10 years ago

@AustenB there are two separate issues here, but both should be fairly easily solvable.

  1. Pass local image representation + remote image url - should be trivial as we're talking about the same image exactly. We save here unnecessary potential uploads and downloads (see #274). Passing exactly the same image would be the responsibility of the developer who uses the SDK, but you can easily validate it on the server.
  2. Make the local representation thumbnail - the simple solution would be to use aspect ratio scaled thumbnails. These can be created manually or by using native ALAsset's aspectRatioThumbnail or fullScreenImage, if using ALAssetsLibrary or the new iOS8 equivalent.

If fact the thumbnails don't have to be local. Out backend server (and a couple of storage providers) can create scaled images on the server by calling it with an appropriate url like: images.cdn.com/w_100,h_100,aspect_fit/image1.jpg or images.cdn.com/w_200,h_200,aspect_fit/image1.jpg. So it could make sense to send urls to smaller images as thumbnail and full res for manufacturing.

berendo commented 7 years ago

What I would like to do is (symbolic writing - please ignore specific interface):

// Local thumbnail and pre-uploaded remote image
NSArray *images = @[@{'thumb': uiimage_thumb, 'full': @'http:/google.com/img.jpg'}, ...]; 

This is exactly what your Javascript SDK provides (see "Passing In Images" at https://www.gooten.com/platforms/javascript-sdk/documentation/web-widget/functionality, specifically the pass in an object literal that allows for more options example) and is absolutely a must for the iOS SDK as well. When high-resolution photos are on a backend service, consuming full-resolution download bandwidth just to derive local thumbnails is not reasonable.

I don't think this is a low-priority issue at all...

perisicboro commented 7 years ago

@berendo Consider using custom photo source instead, where you can set thumb url, and original image url:

https://github.com/printdotio/printio-ios-sdk/blob/master/docs/Photo-Sources/custom_photo_sources.md

berendo commented 7 years ago

Consider using custom photo source instead

I have (before commenting on this issue). Unfortunately, there does not appear to be any way to use a custom photo source implementation to pass in a pre-selected set of photos.

perisicboro commented 7 years ago

@berendo Passing thumb url along with original image url will be added to sdk. I will inform you when finish it and release new sdk version.

berendo commented 7 years ago

Passing thumb url along with original image url will be added to sdk. I will inform you when finish it and release new sdk version.

That's fantastic to hear! In the meantime, if #529 (a.k.a. #274) can be addressed, I have a temporary hack that works, if the upload of the entire original can be cut out of the loop. Keep us posted, please!

perisicboro commented 7 years ago

Fixed

berendo commented 7 years ago

Fixed

@perisicboro I did not see any (obvious) updates to the online documentation. Could you point me toward (or elaborate here) the details on how to pass separate thumbnails vs original URLs to [PrintIO setImages:]? Thanks!

D'uh! Please ignore... I located the notes on being able to instantiate PIOPassedImage objects in the array given to [PrintIO setImages:].

berendo commented 7 years ago

@perisicboro unfortunately, I'm still seeing the framework download the full-resolution images, even though I'm passing it PIOPassedImage instances with separate thumbnail and image URLs. For example, I confirmed at a breakpoint just before calling setImages: that I have:

(lldb) po images[0].imageUrl
▿ Optional<String>
  - some : "https://.../3/files/2246450100/media?signature=0f65eb8c321dcb05ac12b5826517c5dccb0584f7&v=4&auto_rotate=true"

(lldb) po images[0].thumbnailUrl
▿ Optional<String>
  - some : "https://.../3/files/2246450100/thumbnails/0f65eb8c321dcb05ac12b5826517c5dccb0584f7/s1280/thumbnail.jpg?v=4"

and yet, as soon as I select a product and the thumbnails have to be rendered by the framework, there's a noticeable delay and then I see:

... [17/Aug/2017:06:04:33 +0000] "GET /3/files/2246450100/media?signature=0f65eb8c321dcb05ac12b5826517c5dccb0584f7&v=4&auto_rotate=true HTTP/1.1" 200 2653379 826115 "-" "MysteryHouse/6.3.2-rc CFNetwork/811.4.18 Darwin/16.6.0" - - -

in our API server logs in indicating that the original was downloaded.

To further corroborate this, I've passed file: URLs for thumbnailUrl (since our app already has locally-cached assets) and I still see the downloads of the originals from the API server.

perisicboro commented 7 years ago

@berendo Of course SDK downloads full res images, because Image Editor works with full res images. Thumbs are used in Photo Selection only for preview.

berendo commented 7 years ago

There were no images edited - I only got as far as photo and quantity selection. What's the point of downloading the full resolution originals if the upload of those are going to be suppressed given #274 has been fixed (again)?

perisicboro commented 7 years ago

Next screen after photo selection is Product Editor (Image Editor) where we use downloaded images, not thumbnails

berendo commented 7 years ago

I really don't understand why the full resolution originals need to be downloaded, given your backend will simply be given the URL to get the images from anyway. It is really not acceptable to expect 10+ MB JPEG originals to be downloaded to a mobile device just to order prints. We just want this to work like your JavaScript SDK.

perisicboro commented 7 years ago

The point of downloading is to give user ability to customize product with original image, so for example he can see how deep he can zoom, and to have real preview of product. Your responsibility is to manage image size on your side.

perisicboro commented 7 years ago

Imagine, that you have thumbs in this editor:

simulator screen shot aug 17 2017 9 38 19 am simulator screen shot aug 17 2017 9 38 34 am

berendo commented 7 years ago

No, I'm sorry, this makes no sense whatsoever. First of all, the reason we want to provide a separate "thumbnail" URL is because we already have pre-downloaded/high-resolution images available. The framework should use those and, if the user zooms in beyond the scale afforded by the "thumbnail," then download the full resolution original. Common usage flows don't have the user zooming into the images much at all. Waiting for the download of 10+ full resolution images is a very bad user experience.

Again, why can't this be made consistent with what your JavaScript SDK does? It appears to correctly use the thumbnail URLs for all visualization within the web app, passing the full resolution image solely by URL (not uploading), and wholly avoiding the download of the full resolution image to the browser.

berendo commented 7 years ago

Imagine, that you have thumbs in this editor:

Sure, but, first of all, you're taking the term "thumbnail" too literally. As I said before, in our case, they are quite high resolution (sometimes even full resolution) and have been already downloaded and cached by other parts of our app, so we'd like to use them as much as possible. The framework is free to download anything higher resolution if the user enters an editing context that is not served sufficiently by the provided images. But, having the original fetched as soon as the user needs to adjust quantities of a few 5x7 prints that are already visualized as fairly small UI elements when we've provided 2048-pixel (long axis) thumbnails is not acceptable.

Second, I think you really are ignoring the user experience problem of having the user wait for the download of 10+ full resolution photos, each of which can be 10+ MB in the case of DSLR-captured images. This gets even more compounded if the user is not on Wi-Fi, but cellular, and data usage concerns also enter the calculus. Our app is a completely online/offline capable app, which intelligently prefetches the likely-to-be used portions of the use's photo library in varying degrees of resolution, and we'd like to benefit from that by skipping the long delay after the product selection phase.

berendo commented 7 years ago

I have re-re-verified that your Javascript SDK does not fetch the full-resolution original at this step:

image

or even if I go deeper into the cropping editor:

image

In using the Javascript SDK, we use the same methodology of providing a high-resolution (2048-pixel long axis) "thumbnail" URL for the UI and a full-resolution original for production. The "thumbnail" is used by the UI and the production is fetched only by the Gooten backend.

This is the behavior that should be implemented in the iOS SDK as well.

perisicboro commented 7 years ago

@berendo So, I will implement it in SDK on this way:

berendo commented 7 years ago
  • if you pass in imageUrl and thumbUrl, thumbUrl will be downloaded and used for editing, while imageUrl will be only forwarded as print image url

That would be great, thank you.

I doubt you'd go out of your to make this impossible, but I'll mention anyway: please let thumbUrl also allow file: URL scheme as well. Based on what I've observed, I think the instantiation of the thumbnail within the framework involves an [NSData dataWithContentsOfURL:], in which case file: scheme support should fall out for free.

This would allow us to provide high-resolutions thumbnails from our app's "cloud cache" if they've already been prefetched elsewhere in the app.

perisicboro commented 7 years ago

@berendo So, I will implement it in SDK on this way:

if you pass in imageUrl and thumbUrl, thumbUrl will be downloaded and used for editing, while imageUrl will be only forwarded as print image url if you pass in only imageUrl , that image will be downloaded and used for editing and will be forwarded as print image url

Unfortunately, this feature isn't in our plans right now, so I need to postpone this implementation for now.

berendo commented 7 years ago

Unfortunately, this feature isn't in our plans right now, so I need to postpone this implementation for now.

Wow, that's disappointing... I'll proceed with my monkey-patch of the SDK externally via the Objective-C runtime to do the right thing™ as it is unusable for our application in its current state. I'd strongly suggest you guys prioritize this bug to bring the functionality to par with the Javascript SDK.