modified block DL progressbar to be more informative and precise #1025

Diapolo · 2012-04-02T06:03:30Z

I added a % left value to the tooltip, the progressbar text is now displayed in it's middle centered and uses real block values instead of only a % value ("x of y blocks (z %)"). The progressbar is hidden, if there is no network connection and get's hidden, if the connection is lost.

Edit: Removed the dynamic component of the idea ;).

gmaxwell · 2012-04-02T06:09:33Z

NAK. Most confusing possibility yet proposed. :(

The problem with relative progress is that people restart while syncing and freak out when it goes to zero... thinking that it's started from the start again. Some people have even given up on the reference client as a result of this (or so they say on IRC).

Behavior that changes in ways that would take a paragraph to explain based on state which is hidden from the user is not intuitive at all.

However, I like showing real block values— but UI clueful people seem to think this is a bad idea.

Diapolo · 2012-04-02T06:40:55Z

I'm new in that discussion, but think about it for a while ... the ugly thing is not really the full resync, but if you didn't start the client for a few days and it has to catch up let's say 1000 blocks. This would be displayed relative and does not seem to stuck for so long on 99%. I even added the strings "abs. display" / "rel. display" as a hint.

But okay, so if the consens would be to not use a dynamic display at all, what do you think of the other changes (including only displaying the bar if there is a bc network connection)?

laanwj · 2012-04-02T06:42:29Z

src/qt/bitcoingui.cpp

-        setNumBlocks(clientModel->getNumBlocks());
-        connect(clientModel, SIGNAL(numBlocksChanged(int)), this, SLOT(setNumBlocks(int)));
+        // don't display the sync. message, if we are not connected to the network
+        if (clientModel->getNumConnections() > 0)


Do not make this check in the setModel function but in setNumBlocks.

Background: in Qt, a model is assigned to a widget at most once (there are exceptions, for example when reading a new file and everything should be reset), so it must not make judgements based on temporary state.

Also this does not handle the case where number of connections goes back to 0.

Good point, will change that behaviour.

laanwj · 2012-04-02T06:59:57Z

We've already tried the "show relative progress" thing once. It didn't fly.

Unless someone develops an elegant widget that shows both absolute and relative information in a neat, intuitive way (and cross-validates this with the complainers on the bitcoin forums and here) I'm reluctant to try it again.

I do agree with only showing "catching up" and the progress bar when the network connection is up. See my implementation comment though :)

Diapolo · 2012-04-02T08:04:41Z

So if a relative display didn't fly what about displaying "x of y blocks (z %)" as text on the progressbar? I think it's very nice, because you see progress per block and not per % (which is ver imprecise).

Diapolo · 2012-04-02T08:27:29Z

Updated to be not relative anymore and changed network detection part as laanwj suggested.

laanwj · 2012-04-02T08:41:46Z

The problem is that the source information (the YY in "XX of YY blocks") is
imprecise too. Showing that number only gives a false sense of precision.

On the other hand I agree with you. If you show a progress bar, the % is
redundant as it is already visually conveyed.

Diapolo · 2012-04-02T08:46:46Z

Can you explain, why YY is imprecise? That are the current all time blocks in the network (claimed by the nodes), but even if the real value is not correct, the new text display is clear.

If you start the client and it sits on 99% for minutes you think fuck what's up. If it reads 9900 of 10000 blocks (99%) and keeps moving block for block you see it's working and it progresses further.

sipa · 2012-04-02T10:01:25Z

I'd say use an absolute percentage chain always, but put in readable text on it "est. X blocks left".

Diapolo · 2012-04-02T10:13:58Z

Last commit makes it absolute again, so that's fine and I won't touch it anymore ^^.

So we have the coice if we keep "%", use my suggestion "x of y blocks (z%) or something like sipa suggested above. As I said everything is better than a simple % because users want to see at least a real progress.

Edit: Tried it and a "x blocks remaining (y % done)" message looks nice and clean, too.

laanwj · 2012-04-02T10:43:13Z

Thanks for making it absolute again.

I'm convinced. I think we should drop the percentage from the text and show
"xx of estimated yy". Percentage is implicit in the progess bar visual and
doesnt add anything.

Sipa's suggestion to show only the estimated number of blocks left is also
good. Advantage is that it shows only one number, disadvantage is that the
derived number is less useful for problem diagnostics.

Diapolo · 2012-04-02T11:13:32Z

The more I think about it, the better I like sipas idea of only showing the remaining blocks. And for problem diagnostic we have a tooltip, which shows the real numbers.

So "~xx blocks remaining" as text? Centered on the progressbar (perhaps hard to read as this uses the native OS style for the bar and text) or to the right of the bar?

laanwj · 2012-04-02T12:04:36Z

I'd say show it on the progress bar. That's one less level of
indirection. If it looks weird on some OS we can always change it.

Diapolo · 2012-04-02T13:36:02Z

Was a bit tricky to count backwards (blocks remaining) and keep track of the right value (scale) of the progressbar, but now it's ready for testing / review :).

Diapolo · 2012-04-02T13:37:46Z

src/qt/bitcoingui.cpp

@@ -451,20 +451,33 @@ void BitcoinGUI::setNumConnections(int count)

 void BitcoinGUI::setNumBlocks(int count)
 {
-    if(!clientModel)
+    // don't show / hide progressBar and it's label if we have no connection(s) to the network
+    if (!clientModel || clientModel->getNumConnections() == 0)


@laanwj Will this interfere with status bar warnings or do they only work if there is a connection?

Yes, this interferes with the status bar warnings. But those do only work if the client was connected to the network at one point or another.

sipa · 2012-04-02T14:28:27Z

Looks good so far, but

small blocks for the progress percentage are not nice when combined with text
at least here, it's completely unreadable: dark blocks for progress with black text; is there a way to get something like this: http://stackoverflow.com/questions/6258690/is-there-any-way-to-change-the-color-of-text-halfway-through-a-character-on-a ?

Diapolo · 2012-04-02T14:35:09Z

@sipa The style/design of the progressbar comes from the OS and can only be changed by some form of Qt stylesheets afaik (see: http://harmattan-dev.nokia.com/docs/library/html/qt4/qprogressbar.html#details).

2 options, put the text right to the progressbar or search the web for howto edit the style of the bar (which is a hard work, because of the different OS GUIs and color schemes :-/).

Diapolo · 2012-04-02T16:42:06Z

Okay, I found a way to apply stylesheets to the bar and it can be very well customized ... will add a commit so you could try it out @sipa.

laanwj · 2012-04-02T16:50:20Z

Ok, let us know how it goes. Sounds like something that is a nightmare to get right on all OSes. Otherwise, putting the text in the progressBarLabel to the left of the progress bar is fine too.

Diapolo · 2012-04-02T17:11:20Z

OMFG what a mess, but now I'm happy :-D. So on Windows the bar looks equal, no matter what theme I activate and even if I chose the classic theme.

I moved some one-time functions so that they don't get called everytime setNumConnections() is called.

To the style itself, I chose an orange as bar color, because of the Bitcoin icon and I think it ROCKS! But hey let's vote or you can make own suggestions :).

Diapolo · 2012-04-02T17:13:02Z

src/qt/bitcoingui.cpp

@@ -138,17 +138,21 @@
    frameBlocksLayout->addWidget(labelBlocksIcon);
    frameBlocksLayout->addStretch();

-    // Progress bar for blocks download
-    progressBarLabel = new QLabel(tr("Synchronizing with network..."));


I removed that one, because we set the text below, so that was not needed. The same was true for the progressBar tooltip a few lines below.

laanwj · 2012-04-02T17:27:15Z

So on Windows the bar looks equal, no matter what theme I activate and even if I chose the classic theme.

This does mean we lose platform-native styling for the progress bar. Not that that is necessarily a problem, I think it looks nice (also on Ubuntu 11.10).

…ction code to setNumBlocks()

…ons used on the bar to a better code location

…f a sync)

Diapolo · 2012-04-02T21:59:36Z

I had to edit the code once more, because in the end of a sync the bar was rather jerky and jumped a bit too much. Because of that I had to seperate the "~x blocks remaining" text from the real progress bar values, to be not linked. Now the displayed text is one thing and the progressbar progress display another one.

Edit: I will rebase this after I got some final ACKs :).

sipa · 2012-04-03T01:26:01Z

It looks nice, visual ACK. I didn't check the source code changes.

laanwj · 2012-04-03T06:05:12Z

That last commit did certainly make it better. It seemed like the number never changed while I was looking :-)

Edit: ACK

…wed plurals in translation for "x block(s) remaining"

Diapolo · 2012-04-03T06:31:57Z

@laanwj Could you take a short look over the last commit. If you are fine with that, I will rebase!

laanwj · 2012-04-03T06:45:11Z

src/qt/bitcoingui.cpp

    progressBar->setVisible(false);

    statusBar()->addWidget(progressBarLabel);
    statusBar()->addWidget(progressBar);
    statusBar()->addPermanentWidget(frameBlocks);

+    // define OS independent progress bar style (has to be placed after addWidget(), otherwise we crash)


maybe also add an explanation why we need OS independent progress bar style, so that people in the future will understand :-)

Like: "we did this, because with some OSes default style, text on the progress bar is unreadable" ;)?

…in the tr() call

Diapolo · 2012-04-04T05:00:16Z

So we have 1 "visual" ACK and laanwjs ACK, further dev votes :)?

laanwj · 2012-04-04T06:29:27Z

I don't think there's much more to be done, except determine whether we want this for 0.6.1 or 0.7.0. The code change is small and simple enough to make it end up in 0.6.1 IMO, the only hindrance is that some translation messages have been changed and it usually takes a while to update them.

Diapolo · 2012-04-04T06:51:46Z

As for the translations, I would push the new master file (en) early to transifex, so that translators have enough time to keep up. So I guess there will be RCs for 0.6.1 and I suggest let's integrate this one here.

Remember, there is that Wallet-HTML-thing from #945, which has to do with translations, too (even if we would do that).

laanwj · 2012-04-04T06:58:27Z

Yes, agreed. I'm for merging this as soon as possible.
(and also #1002, depending on whether we fix URL handling for 0.6.1)

#945 is for 0.7.0 as it is pretty low-priority (nearly no visual difference) but a lot of hassle for translators.

sipa · 2012-04-04T10:38:37Z

Fine for 0.6.1 for me.

modified block DL progressbar to be more informative and precise

…mit) (bitcoin#1025) * simplify CMasternodePayments::IsEnoughData (always using GetStorageLimit)

… requested. 1a8ed6e [Startup] Don't continue loading the wallet if the shutdown was requested. (furszy) Pull request description: Auto-explainable title. ACKs for top commit: Fuzzbawls: ACK 1a8ed6e random-zebra: ACK 1a8ed6e and merging... Tree-SHA512: bf8952699d1acda591a5cd9dfdca142a29c1c2c6aea5f6b9ede938a053dcc5194e8b691d606a4bdd684ce376a34aa3f14f759cddc438602dae4877ee024068d2

laanwj reviewed Apr 2, 2012
View reviewed changes

Diapolo reviewed Apr 2, 2012
View reviewed changes

Philip Kaufmann added 6 commits April 2, 2012 23:48

modified block DL progressbar to be dynamic and more precise

9ceae8a

removed relative progressbar display and moved re-worked network dete…

068ed1e

…ction code to setNumBlocks()

changed progressbar text to "~n blocks remaining"

5519660

implemented OS independent progress bar style / moved one-time functi…

c7c0c93

…ons used on the bar to a better code location

color update for progress bar

e9de46c

polished code and fixed progress display (was very jerky at the end o…

a7a69cd

…f a sync)

changed percentage done in tooltip to float to be more precise / allo…

ec9a490

…wed plurals in translation for "x block(s) remaining"

laanwj reviewed Apr 3, 2012
View reviewed changes

clarified comment why we use an own progressbar style / included "~" …

853a4a8

…in the tr() call

dooglus mentioned this pull request Apr 3, 2012

Show "N blocks remaining" rather than a percentage. #1030

Closed

laanwj added a commit that referenced this pull request Apr 4, 2012

Merge pull request #1025 from Diapolo/GUI-BlockDL

cadae35

modified block DL progressbar to be more informative and precise

laanwj merged commit cadae35 into bitcoin:master Apr 4, 2012

dooglus mentioned this pull request Apr 9, 2012

blockchain download status is hard to read #1071

Closed

coblee pushed a commit to litecoin-project/litecoin that referenced this pull request Jul 17, 2012

Merge pull request bitcoin#1025 from Diapolo/GUI-BlockDL

018cfa8

modified block DL progressbar to be more informative and precise

suprnurd pushed a commit to chaincoin-legacy/chaincoin that referenced this pull request Dec 5, 2017

simplify CMasternodePayments::IsEnoughData (always using GetStorageLi…

d6637c2

…mit) (bitcoin#1025) * simplify CMasternodePayments::IsEnoughData (always using GetStorageLimit)

bitcoin locked as resolved and limited conversation to collaborators Sep 8, 2021

modified block DL progressbar to be more informative and precise #1025

modified block DL progressbar to be more informative and precise #1025

Conversation

Diapolo commented Apr 2, 2012

gmaxwell commented Apr 2, 2012

Diapolo commented Apr 2, 2012

Choose a reason for hiding this comment

Choose a reason for hiding this comment

laanwj commented Apr 2, 2012

Diapolo commented Apr 2, 2012

Diapolo commented Apr 2, 2012

laanwj commented Apr 2, 2012

Diapolo commented Apr 2, 2012

sipa commented Apr 2, 2012

Diapolo commented Apr 2, 2012

laanwj commented Apr 2, 2012

Diapolo commented Apr 2, 2012

laanwj commented Apr 2, 2012

Diapolo commented Apr 2, 2012

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sipa commented Apr 2, 2012

Diapolo commented Apr 2, 2012

Diapolo commented Apr 2, 2012

laanwj commented Apr 2, 2012

Diapolo commented Apr 2, 2012

Choose a reason for hiding this comment

laanwj commented Apr 2, 2012

Diapolo commented Apr 2, 2012

sipa commented Apr 3, 2012

laanwj commented Apr 3, 2012

Diapolo commented Apr 3, 2012

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Diapolo commented Apr 4, 2012

laanwj commented Apr 4, 2012

Diapolo commented Apr 4, 2012

laanwj commented Apr 4, 2012

sipa commented Apr 4, 2012