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

There is no any simple way to set priority for ImageRequest #9

Closed
xp-vit opened this issue Feb 1, 2017 · 3 comments
Closed

There is no any simple way to set priority for ImageRequest #9

xp-vit opened this issue Feb 1, 2017 · 3 comments

Comments

@xp-vit
Copy link

xp-vit commented Feb 1, 2017

ImageRequest.java has Priority.LOW by default. There is no any easy way to load images with different priority. The only way currently is to implement own ImageLoader and extend ImageRequest.java to support other priorities. I think that ability to have different Priorities for Images loading. Let me know if you are agree and I may work on a patch, since I already have this feature added to valley for my project.

@jpd236
Copy link
Collaborator

jpd236 commented Feb 1, 2017

I'm not quite sure I see what's difficult here. You can extend ImageRequest and override getPriority to set a request. You can also extend ImageLoader and override makeImageRequest to return your custom ImageRequest which overrides getPriority. None of this should require too much duplicated/custom code.

What APIs are you proposing to add?

@xp-vit
Copy link
Author

xp-vit commented Feb 1, 2017

I was thinking on extending:
public ImageContainer get(String requestUrl, ImageListener imageListener, int maxWidth, int maxHeight, ScaleType scaleType)

to

public ImageContainer get(String requestUrl, ImageListener imageListener, int maxWidth, int maxHeight, ScaleType scaleType, Priority priority) {

And then add appropriate convenience methods with smaller number of parameters in the same time supporting API backward compatibility, passing Priority.LOW when priority not passed.

@jpd236
Copy link
Collaborator

jpd236 commented Feb 2, 2017

I think that approach might run into issues with makeImageRequest(), which is protected and thus can't have its signature changed. Open to ideas on how to resolve that, but I'm not seeing a simple one at a glance. It's not as simple as adding an overloaded copy like you propose for get(), because get() needs to know which version of makeImageRequest to call, and it can't know whether there's an overridden version of makeImageRequest. But you presumably need some way to propagate the priority down to it.

Also, for what it's worth, I think the semantics are a bit hard to define when it comes to the request batching that ImageLoader does. If a request is in the queue for an image at one priority, and a new one comes in with a higher priority, what should happen? We don't really have a way to promote the priority of an in-flight request. That makes me a bit hesitant to explicitly add this as an API, because you might call get with a high priority and not realize that it would have no impact if a low priority request is already in flight.

I don't know how an app might decide to prioritize requests, but it still seems to me like subclassing ImageLoader would be the way to go here if you accept the above caveat and still want custom priorities, maybe because you can guarantee in your application that there's a unique URL -> priority mapping. But in that case, it should be possible to just override makeImageRequest and apply that mapping on the given requestUrl and return an ImageRequest with the given priority.

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

No branches or pull requests

2 participants