Improve tooltips for torrent progress bar #4936

zeule · 2016-03-10T04:57:36Z

A little improvements for the progress bars in the "General" tab of the torrent properties are proposed.

Show files (and their sizes) for a piece under cursor in tooltip.
If the cursor points to a single file which extends over several pieces, highlight those pieces.

This works as follows: the following phrase is added to the tooltip "Hold Shift key for detailed information":
Here is an illustration:

And if this modifier (Shift) is pressed when tooltip is requested, detailed information about a pieces of a file under cursor is shown:

The detailed tooltip shows a table with sizes and names of the files in the piece under cursor.

In both images one can see highlighted (actually darkened) part of the torrent, occupied by the file under cursor.

This PR extends (and based on) PR #4867. The work continues in PR #4994

sledgehammer999 · 2016-03-10T10:45:52Z

You should probably close [#4867] and use this one instead.

No! Don't do that.
You'll just rebase this after merge the other PR.

zeule · 2016-03-10T11:35:49Z

@LordNyriox , thank you! I always forget to replace those Q_DECL_OVERRIDE inserted by Qt Creator...

Danny3 · 2016-03-12T14:59:18Z

@evsh
Since you're working on this...
Could you maybe add the the actual progress bar that exists in μTorrent but it's missing here?
It's 4 pixels tall surrounded by a 1 pixel pixel grey border as far as I can tell.

This exist in the older version of μTorrent (1.8.5) that I'm using.
This matches the percentage and can be seen how it's moving when downloading a torrent fast or when the torrent is rechecked.

It's a little unexpected for me that the progress bar doesn't really show the progress of downloading torrent, but the downloaded parts of it and the real progress bar is just the small one shown int the Done column.

zeule · 2016-03-12T15:33:26Z

@Danny3 to me this "progress line" looks like a separate widget. Why does the progress percentage not satisfy you? Do you want just a graphical representation or there is something more in this progress bar?

Danny3 · 2016-03-14T14:35:17Z

@evsh

I found the "progress line" to be useful in 2 cases:
1. A little trick that I found myself:
When I'm taking a break, like going o eat or something, I just leave the pointer of my mouse at the end of it.
When I get back, I look how much the end of line has moved from where my mouse pointer is.
If it didn't move, it's clear to me that it didn't download anything more.
I don't think I'll remember the percentange, especially if it's a floating point number and this method seems faster than writing the number in a text file for later comparison.
2. Good to detect slowdowns on hard drives.
If i remember right, I think that once I detected that the antivirus was running an automated scan because I was looking at the progress line after doing a force rechek on a torrent and it seemed to me that the line was moving too slow compared to other times.
I think that I wouldn't have noticed anything looking at the percentage number.
Maybe it's my brain that likes graphs more than numbers...
And it's easier for it to see a grahical overview than the exact number.

I thought that qBittorrent tried to offer a simmilar UI to μTorrent keeping as much as possible of the good features in μTorrent.
This seems to me like a good feature which takes little vertical space.
If you can't add it, don't wory, it's ok.
Thanks for asking why do I need it for.
BTW, about this PR, showing the files in the tooltip looks like a nice feature.

zeule · 2016-03-14T14:49:36Z

@Danny3, thank you for the explanations! Thinking about this progress bar, may I ask just one more question? Why don't you use the ETA column in the torrent list? Is it because the column relies on instantaneous speed? What do you think about a small addition to this progress line: to show completed in blue and active (donwloading) in green if the width of this addition is big enough (> 1px?)?

Chocobo1 · 2016-03-14T16:49:49Z

Just tried it out, it's cool!

Here's some bugs I observed:

I start qbt with empty jobs, the bars looks like it's floating, but it should be flat as before, see pics below:
Before:

After I hover mouse over Progress bar, it becomes normal:

This floating bug is also observed when I have only 1 jobs and then I remove it, then the bar starts floating.
About the tooltips text, maybe it's better to switch the legends and pieces info and use the string "This piece contains:" as title.

White: .....
(and other legends)

This piece contains:

in tooltips, use only filename, leave out it's path.

What do you think?

zeule · 2016-03-14T17:01:30Z

@Chocobo1,thank you for the comments! I will definitely investigate the first problem, because the strange thing is I changed the bar drawing code to avoid the same "hovered" impression on my system. Perhaps this is just because of the difference between light and dark backgrounds.
I think that the path in the tooltip is useful. Think about a torrent with series of something. Than each subdirectory may contain files with names like 001, 002, 003, .. Without path information they all will be indistinguishable.

zeule · 2016-03-14T17:13:28Z

I implemented this feature for the following situation. Suppose I want to download a book. I launch a search and (see PR #3989) obtain a number of book collections where this particular book is present. Then I start to download a torrent and find that it is incomplete with current peers. But do they have the book I need? Now I can overview the torrent and check. If no, I can try another one.
And here is the question: perhaps I should instead add an "Availability" column to the "Contents" tab? This column might show instantaneous availability for the torrent files.

Chocobo1 · 2016-03-14T17:15:37Z

Without path information they all will be indistinguishable.

yeah, then it's better to have path included.
But I just felt too much text crammed together (not sure how to describe it precisely).

Another idea, is it better to move file size to the start of a line? that way I can quickly know what file (the piece contains) by simply looking from the end (of a string).

Danny3 · 2016-03-14T17:15:38Z

@evsh
Most of the times the ETA column shows ∞ (Infinity symbol)
The tracker could be down or the only one seeder could be offline at that moment.
IMO, not showing anything it would be the same, either way, it doesn't help me at all.
When it works it's just a smart guess based on the current speed.
Unfortunately it has a major problem that's impossible to fix.
It assumes that the speed will remain constant, (tracker will stay online and peers will remain online and upload at the same speed).
Anyway, it gives you an idea by predicting the future, it's good enough, it's nice.
I will rephrase my example with taking a break from the computer:
Let's say I add a tutorial torrent in the evening.
ETA or no ETA, this will take a while and I put my mouse pointer to the end of the progress line and go to bed.
When I wake up and come to computer I take a look how much progress the torrent made over the night from where I left the mouse.
I don't care about the time, because I know it, it's from when I went to bed until now when I'm looking.
So I don't care that much about the future that ETA shows, but the progress in the past, that has already happened and it's accurate.
I think the progress it's useful for multiple days ETA torrents too, if you want to quickly see how much progress the torrent made each night. leaving the mouse there, for curiosity.

Normally I like simple straight progress bar (one color), but your idea seems interesting and good for showing more detailed information without requiring extra space and I will like to give it a try.
I assume making both versions with a settings toggle would be too much work.
BTW, are you sure that having the second color "green" wouldn't be a problem for color blind people like the ones that have Deuteranopia (problem seeing green)?
http://www.color-blindness.com/deuteranopia-red-green-color-blindness/
Maybe they will see only the first part (blue) and not the the second (green), so they will see half a line...

Chocobo1 · 2016-03-14T17:27:47Z

@evsh
One last thing for this round :p
were you using tabs for indentation? IMO it took too much (precious) spaces, maybe just 2~3 spaces is enough?

zeule · 2016-03-14T17:33:16Z

No, this is the default indentation from HTML <ul> tag.

zeule · 2016-03-21T01:59:25Z

Can I mark somehow the PR as not yet ready for merge? There is a bug in it: if I select a torrent with downloaded metadata and after that a torrent without it, the progress bar shows the progress of the former one after mouse moved out of it.

Chocobo1 · 2016-03-21T05:04:30Z

Can I mark somehow the PR as not yet ready for merge?

just add something like [don't merge] in the title or in the first post

zeule · 2016-03-21T14:57:42Z

Thank you all for the suggestion! I've worked around the bug by cleaning piece information in clear(). Perhaps it is better to do that anyway.

zeule · 2016-03-21T15:24:02Z

Fixed drawing error, now with light theme it should be correct. Tried to remove indentation from the list, but without success (can someone help me, please? How to change indentation in <ul> element rendered in tooltip?). Tried to typeset the files information in a table. The code is there, under TYPESET_FILELIST_AS_TABLE define. However, I got strange alignment with hieroglyphs in file names: file names were shifted downwards for approx 0.1-0.2 ex. Maybe somebody can tell what's wrong there? Moved file size information to the beginning of the list items.

ngosang · 2016-03-22T11:02:47Z

I'd prefer the Legend at the top of the tooltip. Also remove some spaces.
Example:

White: ...
Green: ...
Blue: ...
Files in these pieces:
- Image....

zeule · 2016-03-22T11:40:51Z

I decided to put the legend at the bottom because otherwise the tooltip text looks like a filled with water balloon, pinned by mouse pointer, which is not aesthetic. Could you, maybe, suggest how to avoid that look? Which spaces do you want to remove?

ngosang · 2016-03-22T16:27:32Z

Could you, maybe, suggest how to avoid that look?

No, but most relevant/important info should be first. For users it is more important to understand the graphic.

Which spaces do you want to remove?

Before:

Files in these pieces:

   *  Image....

White: ...
Green: ...
Blue: ...

After:

White: ...
Green: ...
Blue: ...
Files in these pieces:
- Image....

zeule · 2016-03-22T20:57:02Z

@ngosang, @Chocobo1 What do you think about the following: we can show the "simple" tooltip (only legend), but if one of the modifier keys is pressed, we show another tooltip which contains only the file list. Additionally, we can hide file size with another modifier (or this is too much?)
So, the simple tooltip contains the following:
White: ...
Green: ...
Blue: ...
Hold Shift for detailed information

And the detailed tooltip does not contain the legend at all. This saves us from ugly text pyramid and from annoying spaces, but leaves the tooltips clear and readable.

Chocobo1 · 2016-03-23T07:52:30Z

@evsh
Good idea!

we can show the "simple" tooltip (only legend), but if one of the modifier keys is pressed, we show another tooltip which contains only the file list.

agree.

we can hide file size with another modifier (or this is too much?)

too cumbersome.

better add parentheses: (Hold Shift for detailed information)

zeule · 2016-03-23T15:35:38Z

Implemented the tooltips split. Activated table rendering by default for the detailed tooltip, because I think that vertically aligned cells look good. I did not add the suggested parentheses because, as to me, now the simple tooltip is clean and nice.

zeule · 2016-03-28T22:48:16Z

Detailed tooltip caption is left-aligned now

zeule · 2016-05-03T20:28:08Z

I think this is ready for merging.

ngosang · 2016-05-07T11:49:59Z

Sometimes I see the size in two lines but I think it should be always in one line. Maybe using a non-breaking space...

zeule · 2016-05-14T01:50:33Z

@ngosang: I've added 'white-space:nowrap' to the style of the size table cell. Seems to be working here. Could you test, please? But I agree with you that there has to be the thin unbreakable space between the number and the unit.

glassez · 2016-05-31T15:42:03Z

src/base/bittorrent/indexrange.h

+namespace BitTorrent
+{
+    // Interval is defined via [first;last]
+    template <class Index>


I don't like the use class keyword in this context.

class → typename

sledgehammer999 · 2016-06-01T23:24:19Z

I think the code is ok for merging. I did a test run and I noticed something different from a previous version I had tested. I don't know if this is intentional or not. When hovering over the progress/availability bars I don't see the relevant piece being highlighted! Or was it the file you were highlighting? In any case nothing gets highlighted now.

zeule · 2016-06-01T23:55:36Z

When hovering over the progress/availability bars I don't see the relevant piece being highlighted!

This is a bug, which I can't reproduce. Could you look what happens inside of PiecesBar::highlightFile(int imagePos)? In particular, what are the values of pieceIndex, filePieces, and imageRange?

zeule · 2016-06-02T00:05:19Z

@sledgehammer999 there was a bug on one of the previous iterations: integer overflow which broke highlighting if position was far than 2GB from the beginning of a torrent. Do you observe something similar?

zeule · 2016-06-02T00:18:43Z

and in that case imageRange contains negative values.

sledgehammer999 · 2016-06-02T00:58:52Z

src/base/indexrange.h

+
+    constexpr IndexType last() const
+    {
+        return m_first + m_size - 1;


Question: since begin() is the same as first() shouldn't end() be the same as last()?
I am under the impression you offer those for compatibility, right?

You are right, this id for compatibility, and as such, end() must point to one after the last element.

I suppose you mean last() and end() are both correct as they are now, right?

sledgehammer999 · 2016-06-02T01:17:52Z

The issue is that inside PieceIndexToImagePos::imagePos() the torrentInfo is invalid so pieceLength is -1. I cannot understand why it is invalid. You are initializing it in the constructor.

On another note:
2 negative side-effects of curly brace initializations

When using them in the initialization list of a constructor QtCreator gets confused about the rest of the class. For example take class PieceIndexToImagePos and go inside imagePos(). Hit enter after const qlonglong pieceLength = m_torrentInfo.pieceLength();. The newline gets indented too far to the right.
In QRect newHighlitedRegion {imageRange.first(), 0, imageRange.size(), m_image.height()}; hover over newHighlitedRegion. The tooltip doesn't show the constructor signature.

sledgehammer999 · 2016-06-02T01:25:30Z

There's something very wrong here.
If I put in PiecesBar::highlightFile() this:

PieceIndexToImagePos transform {m_torrent->info(), m_image};
qCritical() << m_torrent->info().isValid();

it works!!!

Vanilla code(without qCritical()) doesn't work.

zeule · 2016-06-02T08:26:44Z

OMG, I thought that TorrentHandle::info() returns const TorrentInfo&...

In addition to the current tooltip, which shows color legend, if user holds the Shift key during hovering we show another tooltip which contains a table of contents for the piece under the moue cursor. The table lists file sizes and names. If the cursor points to a part of a file which spans several pieces, those pieces are highlighted.

…eces()

zeule · 2016-06-02T08:32:44Z

There's something very wrong here.

OMG, I thought that TorrentHandle::info() returns const TorrentInfo&...

Fixed.

zeule · 2016-06-02T08:45:08Z

BTW, GCC with -Wextra should warn about this error. Unfortunately, we do not pass neither -Wextra nor other warning options to it.

glassez · 2016-06-02T09:24:08Z

OMG, I thought that TorrentHandle::info() returns const TorrentInfo&...

@evsh So what was the problem? I was lost in recent code changes...

zeule · 2016-06-02T09:28:23Z

@glassez PieceIndexToImagePos was keeping const reference to a temporary TorrentInfo object, obtained via TorrentHandle::info()

sledgehammer999 · 2016-06-02T10:42:34Z

I'll be able to check in a couple of hours. In the meantime, please take a look at my inline comment in constexpr IndexType last() const.

PS: Aren't you able to define extra CXXFLAGS before compiling?

zeule · 2016-06-02T13:18:42Z

PS: Aren't you able to define extra CXXFLAGS before compiling?

If I modify cmake files, each git command will ask me to commit or stash those changes.

sledgehammer999 · 2016-06-02T14:54:08Z

Thanks for this.

If I modify cmake files, each git command will ask me to commit or stash those changes.

Not true. It asks about that edited file only when you are rebasing or pulling a branch that modifies that same file. But this is solved easily. You do your necessary changes and stash them. Each time you clean your working dir you just need "git stash apply" to reapply them. The "apply" verb keeps the stash and doesn't delete it.
This may not work so well if you want to keep multiple stashes. You'll need to remember the index too then.

zeule · 2016-06-02T18:00:28Z

Thank you for merging, and I very much thankful to all those who took part in the discussion!

@sledgehammer999, I hope that you ran it one more time and that highlighting work. Does it?

It asks about that edited file only when you are rebasing or pulling a branch that modifies that same file.

And rebasing appears to be the most frequent operation :( Especially to amend a HEAD^n commit.

Chocobo1 · 2016-06-02T18:23:55Z

Was just playing this, found a minor issue:

This happens when I point to the very start (and end) of the progress bar.

zeule · 2016-06-02T18:25:45Z

@Chocobo1: I can't understand what is in your image. What is the dark rectangle? What is supposed to be there?

Chocobo1 · 2016-06-02T18:27:44Z

Forgot to say, the box is supposed to show detailed information (holding down shift).
I was holding down shift and moving mouse around.

zeule · 2016-06-02T18:29:13Z

So the detailed tooltips for the very first piece and the very last ones are empty. Is it correct?

Chocobo1 · 2016-06-02T18:32:04Z

Not sure, it only happens on/near the border of the box.

zeule · 2016-06-02T18:55:53Z

Fixed in #5339

zeule mentioned this pull request Mar 10, 2016

Show downloading files in peers list. Attempt #2 #4867

Merged

zeule mentioned this pull request Mar 22, 2016

Add availability column to torrent contents #4994

Merged

zeule mentioned this pull request Apr 15, 2016

Preview File to use piece_deadline instead of sequential_download #182

Open

glassez reviewed May 31, 2016
View reviewed changes

Follow project coding style. Issue #2192.

f9c2bd3

sledgehammer999 reviewed Jun 2, 2016
View reviewed changes

zeule added 2 commits June 2, 2016 10:30

Replace TorrentHandle::fileExtremityPieces() with TorrentInfo::filePi…

5f2362d

…eces()

sledgehammer999 merged commit 95fbff3 into qbittorrent:master Jun 2, 2016

zeule mentioned this pull request Jan 13, 2017

Number of pieces #2956

Closed

Improve tooltips for torrent progress bar #4936

Improve tooltips for torrent progress bar #4936

Conversation

zeule commented Mar 10, 2016

sledgehammer999 commented Mar 10, 2016

zeule commented Mar 10, 2016

Danny3 commented Mar 12, 2016

zeule commented Mar 12, 2016

Danny3 commented Mar 14, 2016

zeule commented Mar 14, 2016

Chocobo1 commented Mar 14, 2016

zeule commented Mar 14, 2016

zeule commented Mar 14, 2016

Chocobo1 commented Mar 14, 2016

Danny3 commented Mar 14, 2016

Chocobo1 commented Mar 14, 2016

zeule commented Mar 14, 2016

zeule commented Mar 21, 2016

Chocobo1 commented Mar 21, 2016

zeule commented Mar 21, 2016

zeule commented Mar 21, 2016

ngosang commented Mar 22, 2016

zeule commented Mar 22, 2016

ngosang commented Mar 22, 2016

zeule commented Mar 22, 2016

Chocobo1 commented Mar 23, 2016

zeule commented Mar 23, 2016

zeule commented Mar 28, 2016

zeule commented May 3, 2016

ngosang commented May 7, 2016

zeule commented May 14, 2016 • edited Loading

glassez May 31, 2016

Choose a reason for hiding this comment

zeule May 31, 2016

Choose a reason for hiding this comment

sledgehammer999 commented Jun 1, 2016 • edited Loading

zeule commented Jun 1, 2016

zeule commented Jun 2, 2016

zeule commented Jun 2, 2016

sledgehammer999 Jun 2, 2016

Choose a reason for hiding this comment

zeule Jun 2, 2016

Choose a reason for hiding this comment

sledgehammer999 Jun 2, 2016

Choose a reason for hiding this comment

zeule Jun 2, 2016 via email

Choose a reason for hiding this comment

sledgehammer999 commented Jun 2, 2016

sledgehammer999 commented Jun 2, 2016 • edited Loading

zeule commented Jun 2, 2016

zeule commented Jun 2, 2016

zeule commented Jun 2, 2016

glassez commented Jun 2, 2016

zeule commented Jun 2, 2016

sledgehammer999 commented Jun 2, 2016

zeule commented Jun 2, 2016

sledgehammer999 commented Jun 2, 2016

zeule commented Jun 2, 2016

Chocobo1 commented Jun 2, 2016

zeule commented Jun 2, 2016

Chocobo1 commented Jun 2, 2016 • edited Loading

zeule commented Jun 2, 2016

Chocobo1 commented Jun 2, 2016 • edited Loading

zeule commented Jun 2, 2016

zeule commented May 14, 2016 •

edited

Loading

sledgehammer999 commented Jun 1, 2016 •

edited

Loading

sledgehammer999 commented Jun 2, 2016 •

edited

Loading

Chocobo1 commented Jun 2, 2016 •

edited

Loading

Chocobo1 commented Jun 2, 2016 •

edited

Loading