[UI] Ghost In some columns

[Doadin]

As can be seen in the image there is a eta on a paused torrent this eta shows/changes whenever hovering a combination of the downloading/paused torrent and will show for multiple paused torrents. Its almost as if it is looping all torrents from the downloading torrent and lower.

It happens when hovering blank space below paused torrent also if you add another torrent no matter the state it doesn't happen, seems to be a very rare case in a certain condition.

[Cas]

Yeah I've seen this before and likely to do with the caching for the cells

[Doadin]

Doesn't cache just compare values from cells? From what I have seen the torrent row that the ghosting happens on was never started so the value for the whole session should be blank not sure how caching would think other wise unless its comparing wrong cells? I don't know much about it so you could be 100% right just more so clarifying that the row aka torrent was never in a state to have a value for the ghosting cell.

[gazpachoking]

Saw this happening today. It was only happening on the ETA column for me, even if I re-arranged or hid other columns. It would copy the ETA of the bottom most downloading torrent onto completed ones below it when I moused over the active torrent and moved down. Moving the mouse back up from below would clear the ghost values.

[gazpachoking]

So, it's certainly something to do with the caching for the cells. If I comment this out: ​https://github.com/deluge-torrent/deluge/blob/24a3987c3af69a835fb9f600ea71f4512ec7a8e5/deluge/ui/gtk3/torrentview_data_funcs.py#L222-L223 The problem is resolved (at least for ETA, which is the only column I've seen it happen.) The issue I'm having is that I don't understand how that cache is supposed to work yet, so I'm not sure what the right fix is.

[Doadin]

@gazpachoking I thought I saw it happen with other columns back when I was first trying to convert to gtk3 but recently now that I started using deluge again I have only noticed it on eta column. So depending on how this is fixed might want consider applying it to other/all cached columns.

[Doadin]

I'm really not sure how caching works either but should it not be more like

func_last_value[row]['cell_data_statusicon'] == state:

Im not sure how it knows what row to compare if we are not storing the row? Maybe I'm just completely missing something I don't see in github history where this change was made. Anyways I'm sure you guys will probably figure it out.

[Doadin]

I do also wonder is caching why theres like that delay when you pause a torrent and it will still shows download speed and it slowly goes down even though its not transfering any data which can be seen even by the global speed value. I find it slightly ocd triggering that it does that lol.

[gazpachoking]

Replying to Doadin:

I do also wonder is caching why theres like that delay when you pause a torrent and it will still shows download speed and it slowly goes down even though its not transfering any data which can be seen even by the global speed value. I find it slightly ocd triggering that it does that lol.

That's a different thing. ;D

I did some more digging in to this, and it seems like the cell renderer funcs assume that when they are fed the same value as last time, the cell will already contain the same text property they put there last time they were called. This just doesn't always seem to be the case though. On the initial render, that assumption holds true. But when moving the mouse around the screen, it redraws certain rows, and the cell passed to the data func may not already contain the expected output for the given data. I'm not sure if there is a better way other than disabling the optimization, and I'm also not sure why I don't see the same issue on other columns that also use a similar optimization.

[gazpachoking]

Okay, here's my final theory. The reason the other columns with optimizations don't have the same problem as ETA is that the issue only arises when cells are being redrawn both from the model data changing, and the mouse moving around (and when there are multiple cells in a column that have the same value.) The other columns that have the optimization are things that don't change frequently. My suggestion: Just rip out the offending lines and hope whatever performance impact it was having wasn't big. ​https://github.com/deluge-torrent/deluge/blob/24a3987c3af69a835fb9f600ea71f4512ec7a8e5/deluge/ui/gtk3/torrentview_data_funcs.py#L222-L223

I suspect every other data func in that file that has a return statement also has the bug, but the cases when it can be triggered are suuuuuper rare.

[Cas]

For reference the original changes by bro occurred in [8ecc0e11a79d92020] which fixed #1885 and added simple caching

I thought I had seen it in other columns that just ETA but since that is the most reported issue and shouldn't need caching we can go with this fix.

[gazpachoking]

I fully believe it can happen on other columns, but I couldn't fully understand the exact conditions triggering it. Replicating on ETA was really easy though.

[Cas]

Milestone renamed

[Cas]

Ticket retargeted after milestone closed