Skip to content

Adds support for specifying a quality indexed array of URLs#557

Merged
garrettmoon merged 6 commits into
masterfrom
lowResImages
Sep 11, 2017
Merged

Adds support for specifying a quality indexed array of URLs#557
garrettmoon merged 6 commits into
masterfrom
lowResImages

Conversation

@garrettmoon

Copy link
Copy Markdown
Member

This allows PINRemoteImage to download from a set of URLs depending on network conditions.

More context perhaps:
pinterest/PINRemoteImage#407
pinterest/PINRemoteImage#403

@ghost

ghost commented Sep 8, 2017

Copy link
Copy Markdown

🚫 CI failed with log

@ghost

ghost commented Sep 8, 2017

Copy link
Copy Markdown

🚫 CI failed with log

@nguyenhuy nguyenhuy left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Some questions aside, LGTM!

[self _locked__setImage:result];

_imageLoaded = YES;
}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Looks like we're fetching images from low cost to higher cost and don't bail as soon as there is a cache hit. Does this mean that if there are multiple images, we'll end up fetching all of them but only use the best one? If that's the case, should we instead fetch in reverse order and stop as early as possible?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Yeah, good catch!

@garrettmoon
garrettmoon merged commit 3c77d4a into master Sep 11, 2017
garrettmoon added a commit that referenced this pull request Dec 5, 2017
garrettmoon added a commit that referenced this pull request Dec 5, 2017
)

* Revert "Adds support for specifying a quality indexed array of URLs (#557)"

This reverts commit 3c77d4a.

* Add CHANGELOG entry
@appleguy
appleguy deleted the lowResImages branch December 28, 2017 00:56
bernieperez pushed a commit to AtomTickets/Texture that referenced this pull request Apr 25, 2018
…roup#557)

* Add support for downloading a set of URLs on ASNetworkImageNode

* Should be building now;

* Remove old unused code

* Add a changelog message

* Bump PINRemoteImage

* Huy's comments
bernieperez pushed a commit to AtomTickets/Texture that referenced this pull request Apr 25, 2018
…extureGroup#699)

* Revert "Adds support for specifying a quality indexed array of URLs (TextureGroup#557)"

This reverts commit 3c77d4a.

* Add CHANGELOG entry
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants