Finer-grained UI updates, move UI interface to boost::signals #1205

laanwj commented at 3:43 PM on May 5, 2012: member

Gets rid of MainFrameRepaint in favor of specific update functions that tell the UI exactly what changed.

This improves the efficiency of various handlers (no unnecessary balance recalculations).
Also fixes the problem with mined transactions not showing up until restart.
As this finally makes it possible for the UI to know when a new alert arrived, it can be shown as OS notification.
New addresses are selected again in address book (was a regression in 0.6.1)

The following notifications were added:

NotifyBlocksChanged: Block chain changed
NotifyKeyStoreStatusChanged: Wallet status (encrypted, locked) changed.
NotifyAddressBookChanged: Address book entry changed.
NotifyTransactionChanged: Wallet transaction added, removed or updated.
NotifyNumConnectionsChanged: Number of connections changed.
NotifyAlertChanged: New, updated or cancelled alert.

These notifications could also be useful for RPC clients. However, currently, they are ignored in bitcoind.

Also brings back polling with timer for numBlocks in ClientModel. This value updates so frequently during initial download that the number of signals clogs the UI thread and causes heavy CPU usage. And after initial block download, the value changes so rarely that a delay of half a second until the UI updates is unnoticable.

Second commit converts UI interface to boost::signals:

Core no longer links to any UI functions directly, but the UI subscribes to notifications through boost::signals.
Signals now go directly from the core to WalletModel/ClientModel.
- WalletModel subscribes to signals on CWallet: Prepares for multi-wallet support, by no longer assuming an implicit global wallet.
Gets rid of noui.cpp, the few lines that were left are merged into init.cpp
Rename wxXXX message flags to MF_XXX, to make them UI indifferent.
ThreadSafeMessageBox converted to void, no longer returns the value 4 which was never used

Diapolo commented at 9:32 AM on May 7, 2012: none

I looked around in your changes and there is much code in, I can't really say anything to most parts ^^. Does it compile fine on Windows? This takes more time for me to check this, sorry.

laanwj commented at 9:49 AM on May 7, 2012: member

It mostly needs sanity testing. The code should compile find on windows, as there's no OS specific changes. There should be no user-visible changes to behaviour (except for the small fixed problems) but this improves efficiency, prepares for multiple-wallet support and makes it easier to split the UI into a separate process at some point.

Diapolo commented at 3:51 PM on May 10, 2012: none

This should help with lowering the current CPU usage of utilizing a full core, while initial block chain download is in progress, right? Is this an issue on non-Windows versions, too and have you verified this?

laanwj commented at 4:48 PM on May 10, 2012: member

Well, this is not really meant as a performance fix, although it probably has that effect. AFAIK there are no pressing UI performance issues in 0.6.2.

The idea is to further formalize the interface between the UI and the core.

Instead of a sledgehammer "mainFrameRepaint" event, the UI gets messages such as AddressBookChanged, TransactionChanged that tell the UI directly what has changed, so it can update its internal model (and display) without further checks. This has a positive effect on performance and lock contention.

Eventually I want to remove all locks and direct access in the UI to the core memory space, and handle everything through function calls. This is much safer, and from that point, it is easy(ish) to completely separate the processes by using message passing.

sipa commented at 6:57 PM on May 17, 2012: member

ACK on changed to core

luke-jr commented at 1:13 AM on May 19, 2012: member

test_bitcoin fails to link:

obj/main.o: In function `InvalidChainFound':
/home/luke-jr/Projects/Education/Tonal/BitCoin/bitcoin/src/main.cpp:949: undefined reference to `uiInterface'
obj/main.o: In function `CBlock::AddToBlockIndex(unsigned int, unsigned int)':
/home/luke-jr/Projects/Education/Tonal/BitCoin/bitcoin/src/main.cpp:1650: undefined reference to `uiInterface'
obj/main.o: In function `CheckDiskSpace(unsigned long long)':
/home/luke-jr/Projects/Education/Tonal/BitCoin/bitcoin/src/main.cpp:1861: undefined reference to `uiInterface'
/home/luke-jr/Projects/Education/Tonal/BitCoin/bitcoin/src/main.cpp:1862: undefined reference to `uiInterface'
obj/main.o: In function `CAlert::ProcessAlert()':
/home/luke-jr/Projects/Education/Tonal/BitCoin/bitcoin/src/main.cpp:2207: undefined reference to `uiInterface'
obj/main.o:/home/luke-jr/Projects/Education/Tonal/BitCoin/bitcoin/src/main.cpp:2213: more undefined references to `uiInterface' follow

luke-jr commented at 5:13 AM on May 19, 2012: member

Fix for test_bitcoin issue in 315fa37

in src/ui_interface.h:None in a47c2286aa outdated

  76 | +    MF_ICON_ASTERISK         = MF_ICON_INFORMATION,
  77 | +    MF_ICON_MASK             = (0x00000100|0x00000200|0x00000400|0x00000800),
  78 | +    MF_FORWARD               = 0x00001000,
  79 | +    MF_BACKWARD              = 0x00002000,
  80 | +    MF_RESET                 = 0x00004000,
  81 | +    MF_HELP                  = 0x00008000,

luke-jr commented at 5:14 AM on May 19, 2012:

http://msdn.microsoft.com/en-us/library/windows/desktop/ms647581(v=vs.85).aspx MF_HELP = 0x00004000L -- Indicates that the menu item has a vertical separator to its left.

laanwj commented at 7:40 AM on May 19, 2012: member

Thanks for testing sipa and luke-jr.

I've pushed a commit that should resolve the test build and windows build issues.

Fine-grained UI updates

Gets rid of `MainFrameRepaint` in favor of specific update functions that tell the UI exactly what changed.

This improves the efficiency of various handlers. Also fixes problems with mined transactions not showing up until restart.

The following notifications were added:

- `NotifyBlocksChanged`: Block chain changed
- `NotifyKeyStoreStatusChanged`: Wallet status (encrypted, locked) changed.
- `NotifyAddressBookChanged`: Address book entry changed.
- `NotifyTransactionChanged`: Wallet transaction added, removed or updated.
- `NotifyNumConnectionsChanged`: Number of connections changed.
- `NotifyAlertChanged`: New, updated or cancelled alert. As this finally makes it possible for the UI to know when a new alert arrived, it can be shown as OS notification.

These notifications could also be useful for RPC clients. However, currently, they are ignored in bitcoind (in noui.cpp).

Also brings back polling with timer for numBlocks in ClientModel. This value updates so frequently during initial download that the number of signals clogs the UI thread and causes heavy CPU usage. And after initial block download, the value changes so rarely that a delay of half a second until the UI updates is unnoticable.

fe4a655042

Convert UI interface to boost::signals2.

- Signals now go directly from the core to WalletModel/ClientModel.
  - WalletModel subscribes to signals on CWallet: Prepares for multi-wallet support, by no longer assuming an implicit global wallet.
- Gets rid of noui.cpp, the few lines that were left are merged into init.cpp
- Rename wxXXX message flags to MF_XXX, to make them UI indifferent.
- ThreadSafeMessageBox no longer returns the value `4` which was never used, converted to void.

ab1b288fa7

Process address book updates incrementally

- No longer invalidates selection model, thus retains selection on address book changes
- Fixes selection of new address when added

0832c0d166

Make testcases build, prevent windows symbol collision 239c11d0dd

laanwj referenced this in commit 5a8398e55a on May 20, 2012

laanwj merged this on May 20, 2012

laanwj closed this on May 20, 2012

coblee referenced this in commit 87549bcc14 on Jul 17, 2012

laanwj deleted the branch on Apr 9, 2014

suprnurd referenced this in commit 711a5fbf20 on Dec 5, 2017

lateminer referenced this in commit cc695d696c on Jan 22, 2019

lateminer referenced this in commit 4f19cd0dfa on Dec 25, 2019

DrahtBot locked this on Sep 8, 2021