Ability to tweak multi threaded image loading

[tsuijten]

First of all, thanks for this great library!

I'm using TileView to display large paintings. The tiles are downloaded from the internet. I have setShouldRenderWhilePanning set to true because it offers the best user experience. Sometimes while flinging the animation gets jaggy.

I feel this might be because there are multiple thread working on downloading the images. Maybe when using a single thread, the tiles might be loaded a little less fast, but would also have less overhead and allow more resources for Android to smoothly show the animations.

Right now the TileRenderPoolExecutor is tightly coupled with the TileView, it would be great if there was some way to tweak the TileRenderPoolExecutor or add an abstraction layer (interface) for the class in charge of loading the tile-set (with TileRenderPoolExecutor as the default implementation)

Additionally I use TileView inside a ViewPager which allows the user to browse through an art collection. Each individual TileView creates and stops a PoolExecutor which also adds overhead. It would make more sense to share a PoolExecutor between TileView's

What are your thoughts on this? Of course I'm willing to help by creating a pull request :)

[moagrius]

My initial thoughts are that this sounds reasonable and could be useful for fine-grain control. Feel free to issue a PR and we'll take a look.

FWIW - and take this with a grain of salt - it used only a single background thread (AsyncTask, actually) until 2.1, and performance is much worse. That said, I definitely wouldn't warn you off from taking a shot.

Good luck - look forward to seeing what you come up with.

[p-lr]

I also saw that but only under certain particular circumstances. I did some investigations and lead me to the conclusion that the cleanup procedure is sometimes too aggressive. But then, it may depend on the device's performance. I don't think this is a multithread vs one thread issue.

More precisely, when the scale is a little greater than 1 / 2^n, the tiles from the n-1 detail level are loaded and down scaled by a factor of about 2. In that context, the TileView has twice more tiles to load/cleanup than when the scale is exactly 1 / 2^(n - 1). So then, i sometimes see a fling have a "jaggy" ending animation. I am not 100% sure, but looking at the memory usage and making some logging lead me to the conclusion mentioned earlier. Maybe too much Canvas.remove calls in the same frame? idk. Unfortunately, i don't have time right now to fix it (if its possible). But at least you may have a better clue of what's going on.

[tsuijten]

I agree that it mostly occurs when at a specific zoom level when there are many tiles in view. It might also have to do with the cleanup indeed. It think it would be a good idea to decouple the TileRenderPoolExecutor maybe I'll go ahead with that so I can play around with a different implementation. Of course I could also start by forking and playing with the hardcoded threading variables.

I theory I would also think that having to create a ExecutorService every time a user goes to the next page in the ViewPager is a lot of overhead (having to create threads and so on) No clue if you would really notice this in practice.

[tsuijten]

I've been playing around some more. At first I wanted to use Fresco to load the Bitmap but this did not seem trivial. Then I used Glide to load the images instead of Picasso.

Now things are much smoother when flinging the image! 🎉 Don't really know what happend there, maybe because Glide accounts for loading images inside an async job. Any way, for now I'm pleased with the result!

[p-lr]

Interesting. I presume you used it in your BitmapProvider implementation. Could you post it here?

[tsuijten]

These are the BitmapProvider's I tested with (Kotlin code btw)

class BitmapProviderPicasso : BitmapProvider {
    override fun getBitmap(tile: Tile, context: Context): Bitmap? {
        return (tile.data as? Map<TilePosition, String>)?.get(TilePosition(tile.column, tile.row))?.let {
            try {
                Picasso.with(context)
                        .load(it)
                        .memoryPolicy(MemoryPolicy.NO_CACHE, MemoryPolicy.NO_STORE)
                        .get()
            } catch (e: IOException) {
                null
            }
        }
    }
}

class BitmapProviderGlide : BitmapProvider {
    override fun getBitmap(tile: Tile, context: Context): Bitmap? {
        return (tile.data as? Map<TilePosition, String>)?.get(TilePosition(tile.column, tile.row))?.let {
            try {
                Glide.with(context)
                        .load(it)
                        .asBitmap()
                        .skipMemoryCache(true)
                        .into(tile.width, tile.height)
                        .get()
            } catch (e: IOException) {
                null
            }
        }
    }
}

[p-lr]

Thanks! Also, i'd like to know whether the tiles you load are local or on the web/distant server.

[tsuijten]

They are loaded from a webserver

[p-lr]

Ok. I will try Glide on local tiles as soon as i get time, to see if there is a difference. Anyway, thanks for the feedback :+1:

[tsuijten]

Okay, I found what was making Glide more smooth. By default Glide loads images as RGB_565 while Picasso loads them as ARGB_8888.

When I changed Picasso to load the images as RGB_565 (which takes half the memory) it was also running much more smoothly.

[p-lr]

From the doc (RGB_565) :

Each pixel is stored on 2 bytes and only the RGB channels are encoded: red is stored with 5 bits of precision (32 possible values), green is stored with 6 bits of precision (64 possible values) and blue is stored with 5 bits of precision. This configuration can produce slight visual artifacts depending on the configuration of the source. For instance, without dithering, the result might show a greenish tint. To get better results dithering should be applied. This configuration may be useful when using opaque bitmaps that do not require high color fidelity.

The colors fidelity may be altered. Tough the fact that is uses twice less memory explains the performance gains.

[moagrius]

fwiw I always use 565

[p-lr]

I can confirm this has a huge impact on performance, even for local tiles! No more stutter even in worse conditions. This is getting better and better :dango:

[tsuijten]

That's great! I know it may have some impact on te quality of the image. But I don't think the users will notice this. For me the extra performance and decreased memory pressure is definitely worth it.

Might this be noteworthy in the docs somewhere?

[p-lr]

Although BitmapProviderAssets already uses RGB_565, i believe this could be mentioned somewhere that this is likely to have a positive impact on performance over ARGB_8888 which is btw, the default when using BitmapFactory.decodeStream( inputStream ).

[p-lr]

After some tests with different maps, i agree with @tsuijten that users would unlikely notice the difference (i personally don't see any, but i'm not a specialist..).

[moagrius]

do i need to do anything on this or do you have it covered peter?

[p-lr]

Well I intend to mention this in the readme, or in the wiki. As you wish.

[moagrius]

go for it 👍

[p-lr]

Ok right now I can't but I will think about it.

[moagrius]

np, or I can, just lmk if I should do something... thx On Jun 8, 2016 4:48 PM, "peterLaurence" notifications@github.com wrote:

Ok right now I can't but I will think about it.

— You are receiving this because you commented.

Reply to this email directly, view it on GitHub https://github.com/moagrius/TileView/issues/315#issuecomment-224739765, or mute the thread https://github.com/notifications/unsubscribe/AAqzoKVzAXoYqTEWLmdUR14GewDvZHLBks5qJziagaJpZM4IuE9F .

[p-lr]

Ok np.

[p-lr]

I have created a new topic in the wiki, called "Performance hints". I talked about RGR_565. I will add other hints later.

[tsuijten]

Great! Thanks for your help!.

[moagrius]

can i close this, or do we still want to DI the executor?

[tsuijten]

You can close this issue, maybe I'll make a pullrequest for DI-ing the executor because I still feel this will reduce overhead in a viewpager. But no need to keep this issue open for that IMO