Bug 460007

Summary: Forecasting graph on the home page has random quality problems
Product: [Applications] kmymoney Reporter: Ralf Habacker <ralf.habacker>
Component: reportsAssignee: KMyMoney Devel Mailing List <kmymoney-devel>
Status: RESOLVED FIXED    
Severity: normal CC: me
Priority: NOR    
Version First Reported In: 5.1.3   
Target Milestone: ---   
Platform: Other   
OS: Microsoft Windows   
Latest Commit: https://invent.kde.org/office/kmymoney/commit/a7e8343fd9397587f5dbb7433b2af7be40ecfd80 Version Fixed/Implemented In: 4.8.5,5.2
Sentry Crash Report:
Bug Depends on:    
Bug Blocks: 426400    
Attachments: testfile.kmy
bigger test case

Description Ralf Habacker 2022-10-05 11:35:09 UTC 
Created attachment 152596 [details]
testfile.kmy

Phil Richardson reported this bug at https://mail.kde.org/pipermail/kmymoney/2022-September/003991.html

STEPS TO REPRODUCE
1. Download portable binary for KMyMoney on Windows from https://download.kde.org/stable/kmymoney/5.1.3/
2. unpack 7z file and run start.bat
3. open the appended file (testfile.kmy)
4. inspect the home page

OBSERVED RESULT
The forecasting graph on the home page has random quality problems. One
moment it renders in full quality, other times it's very blocky, as if it's
consuming 100% of the same width/height, but with only 10% of the quality.

EXPECTED RESULT
The graph should always be displayed in full quality

SOFTWARE/OS VERSIONS
Windows: 10
KDE Frameworks Version:  5.76.0
Qt Version: 5.12.12

ADDITIONAL INFORMATION
After a deactivation/activation cycle of the report view plugin, the graphic is displayed in full quality. This suggests that there is an initialization problem when creating the graph.
Comment 1 Ralf Habacker 2022-10-05 14:47:31 UTC 
The size of graph, which is calculated in

void KHomeViewPrivate:;showNetWorthGraph()
    {
        Q_Q(KHomeView);

        // Adjust the size
        QSize netWorthGraphSize = q->size();
        netWorthGraphSize -= QSize(80, 30);
        m_netWorthGraphLastValidSize = netWorthGraphSize;
        qDebug() << "netWorthGraphSize" << netWorthGraphSize;

contains small values after starting the application

> netWorthGraphSize QSize(414, 275)

and increases after applying any change in the settings menu (for example changing the state of the splash screen)

> netWorthGraphSize QSize(1694, 1172)
.
Comment 2 Ralf Habacker 2022-10-06 07:41:08 UTC 
(In reply to Ralf Habacker from comment #1)
> contains small values after starting the application

The problem with this is that the size of the generated image depends directly on the size of the associated widget, so if you have a small app window, for example, the image will be of poor quality. Subsequently enlarging the app window does not update the graph. Additionally, the problem is reproducible with a maximized application window when the application window size was previously small.

All the above cases can be reproduced with msvc and linux builds.
Comment 3 Dawid Wróbel 2022-10-06 07:51:59 UTC 
Unless it’s an easy fix, it would be best to invest that time into replacing the Home view with a WML one. A proof of concept MR is already available in GitLab.
Comment 4 Ralf Habacker 2022-10-06 08:12:07 UTC 
Git commit 614548a93eb61e76b1c60bd0ce73af2e972cbce9 by Ralf Habacker.
Committed on 06/10/2022 at 08:11.
Pushed by habacker into branch 'master'.

Fix 'Forecasting graph on the home page has random quality problems'

M  +7    -0    kmymoney/views/khomeview.cpp
M  +1    -0    kmymoney/views/khomeview.h

https://invent.kde.org/office/kmymoney/commit/614548a93eb61e76b1c60bd0ce73af2e972cbce9
Comment 5 Ralf Habacker 2022-10-06 08:13:07 UTC 
Git commit 7599ae1798e7410c628b53971933af3c8ed84f00 by Ralf Habacker.
Committed on 06/10/2022 at 08:12.
Pushed by habacker into branch '5.1'.

Fix 'Forecasting graph on the home page has random quality problems'
FIXED-IN: 5.1.4

(cherry picked from commit 614548a93eb61e76b1c60bd0ce73af2e972cbce9)

M  +7    -0    kmymoney/views/khomeview.cpp
M  +1    -0    kmymoney/views/khomeview.h

https://invent.kde.org/office/kmymoney/commit/7599ae1798e7410c628b53971933af3c8ed84f00
Comment 6 Ralf Habacker 2022-10-06 08:34:58 UTC 
(In reply to Dawid Wróbel from comment #3)
> Unless it’s an easy fix,

This was easy to fix, see associated commits.

>  it would be best to invest that time into replacing the Home view with a WML one. A proof of concept MR is already available in
> GitLab.

Thanks for this hint, but is not a solution for the stable 5.1 branch.
Comment 7 Ralf Habacker 2022-10-07 08:40:01 UTC 
Git commit 897ccf9e4163c1f42e254bd81dd914827938a12d by Ralf Habacker.
Committed on 07/10/2022 at 08:39.
Pushed by habacker into branch 'master'.

Reduce the number of home page rebuilds for multiple resize events

Suggested by Thomas Baumgart

M  +8    -3    kmymoney/views/khomeview.cpp

https://invent.kde.org/office/kmymoney/commit/897ccf9e4163c1f42e254bd81dd914827938a12d
Comment 8 Ralf Habacker 2022-10-07 08:41:06 UTC 
Git commit 8910ed3bbf19f2d23a93a7b49445dc5747a532cb by Ralf Habacker.
Committed on 07/10/2022 at 08:40.
Pushed by habacker into branch '5.1'.

Reduce the number of home page rebuilds for multiple resize events

Suggested by Thomas Baumgart
(cherry picked from commit 897ccf9e4163c1f42e254bd81dd914827938a12d)

M  +8    -3    kmymoney/views/khomeview.cpp

https://invent.kde.org/office/kmymoney/commit/8910ed3bbf19f2d23a93a7b49445dc5747a532cb
Comment 9 Ralf Habacker 2022-10-07 08:41:16 UTC 
Git commit 8910ed3bbf19f2d23a93a7b49445dc5747a532cb by Ralf Habacker.
Committed on 07/10/2022 at 08:40.
Pushed by habacker into branch '5.1'.

Reduce the number of home page rebuilds for multiple resize events

Suggested by Thomas Baumgart
(cherry picked from commit 897ccf9e4163c1f42e254bd81dd914827938a12d)

M  +8    -3    kmymoney/views/khomeview.cpp

https://invent.kde.org/office/kmymoney/commit/8910ed3bbf19f2d23a93a7b49445dc5747a532cb
Comment 10 Ralf Habacker 2022-10-07 08:51:56 UTC 
Git commit a7e8343fd9397587f5dbb7433b2af7be40ecfd80 by Ralf Habacker.
Committed on 07/10/2022 at 08:51.
Pushed by habacker into branch '4.8-origin'.

Fix 'Forecasting graph on the home page has random quality problems'
FIXED-IN: 4.8.5,5.1.4

(cherry picked from commit 614548a93eb61e76b1c60bd0ce73af2e972cbce9)

M  +7    -0    kmymoney/views/khomeview.cpp
M  +5    -0    kmymoney/views/khomeview.h

https://invent.kde.org/office/kmymoney/commit/a7e8343fd9397587f5dbb7433b2af7be40ecfd80
Comment 11 Ralf Habacker 2022-10-07 08:51:59 UTC 
Git commit a7e8343fd9397587f5dbb7433b2af7be40ecfd80 by Ralf Habacker.
Committed on 07/10/2022 at 08:51.
Pushed by habacker into branch '4.8-origin'.

Fix 'Forecasting graph on the home page has random quality problems'
FIXED-IN: 4.8.5,5.1.4

(cherry picked from commit 614548a93eb61e76b1c60bd0ce73af2e972cbce9)

M  +7    -0    kmymoney/views/khomeview.cpp
M  +5    -0    kmymoney/views/khomeview.h

https://invent.kde.org/office/kmymoney/commit/a7e8343fd9397587f5dbb7433b2af7be40ecfd80
Comment 12 Ralf Habacker 2022-10-07 08:52:04 UTC 
Git commit d1387581d25a5bcff117f056d0fb23403913aacc by Ralf Habacker.
Committed on 07/10/2022 at 08:51.
Pushed by habacker into branch '4.8-origin'.

Reduce the number of home page rebuilds for multiple resize events

Suggested by Thomas Baumgart
(cherry picked from commit 897ccf9e4163c1f42e254bd81dd914827938a12d)

M  +8    -3    kmymoney/views/khomeview.cpp

https://invent.kde.org/office/kmymoney/commit/d1387581d25a5bcff117f056d0fb23403913aacc
Comment 13 Ralf Habacker 2022-10-07 08:52:07 UTC 
Git commit d1387581d25a5bcff117f056d0fb23403913aacc by Ralf Habacker.
Committed on 07/10/2022 at 08:51.
Pushed by habacker into branch '4.8-origin'.

Reduce the number of home page rebuilds for multiple resize events

Suggested by Thomas Baumgart
(cherry picked from commit 897ccf9e4163c1f42e254bd81dd914827938a12d)

M  +8    -3    kmymoney/views/khomeview.cpp

https://invent.kde.org/office/kmymoney/commit/d1387581d25a5bcff117f056d0fb23403913aacc
Comment 14 Ralf Habacker 2022-10-07 09:15:28 UTC 
Git commit b9104064eeb64d90c7acc1fbcddced551b053a5d by Ralf Habacker.
Committed on 07/10/2022 at 09:15.
Pushed by habacker into branch 'master'.

Fix building with Qt 5.15

Fixup for commit 897ccf9e4.

M  +1    -0    kmymoney/views/khomeview.cpp

https://invent.kde.org/office/kmymoney/commit/b9104064eeb64d90c7acc1fbcddced551b053a5d
Comment 15 Dawid Wróbel 2022-10-14 08:02:37 UTC 
What this does on my end is that every resize event it re-calculates the data several time, which effectively makes the app completely hang-up after an attempt to resize the window:

Processed home view section 8 in 9145 ms
Processed home view section 1 in 47 ms
Processed home view section 2 in 1 ms
Processed home view section 3 in 1993 ms
Processed home view section 4 in 0 ms
Processed home view section 5 in 2 ms
Processed home view section 6 in 5308 ms
Processed home view section 9 in 0 ms
Processed home view section 10 in 5 ms
Processed home view section 7 in 0 ms
Processed home view section 8 in 9180 ms
Processed home view section 1 in 46 ms
Processed home view section 2 in 1 ms
Processed home view section 3 in 1993 ms
Processed home view section 4 in 0 ms
Processed home view section 5 in 2 ms
Processed home view section 6 in 5179 ms
Processed home view section 9 in 0 ms
Processed home view section 10 in 4 ms
Processed home view section 7 in 0 ms

The above is from one resize event, took almost a minute
Comment 16 Ralf Habacker 2022-10-14 08:26:51 UTC 
(In reply to Dawid Wróbel from comment #15)
> What this does on my end 

you are using master or 5.1 ? 

> is that every resize event it re-calculates the
> data several time which effectively makes the app completely hang-up after
> an attempt to resize the window:
> Processed home view section 8 in 9145 ms

Hmmh, I've checked this with a loaded file, but did not get such huge numbers, only msecs. May this be related to the loaded file size ?
Comment 17 Dawid Wróbel 2022-10-14 08:29:54 UTC 
(In reply to Ralf Habacker from comment #16)
> you are using master or 5.1 ? 
master

> Hmmh, I've checked this with a loaded file, but did not get such huge
> numbers, only msecs. May this be related to the loaded file size ?

Possibly, my XML book file is 1.5MB compressed
Comment 18 Ralf Habacker 2022-10-14 08:51:39 UTC 
I changed the code on the 5.1 branch to 

void KHomeView::refresh()
{
    Q_D(KHomeView);
    if (isVisible()) {
        auto t_start = std::chrono::high_resolution_clock::now();
        d->loadView();
        auto t_end = std::chrono::high_resolution_clock::now();
        qDebug() << __FUNCTION__ << std::chrono::duration<double, std::milli>(t_end-t_start).count() << " ms";
        d->m_needsRefresh = false;
    } else {
        d->m_needsRefresh = true;
    }
}

void KHomeView::resizeEvent(QResizeEvent* event)
{
    static QTimer timeout;

    static auto t_now = std::chrono::high_resolution_clock::now();
    auto t_old = t_now;
    t_now = std::chrono::high_resolution_clock::now();
    qDebug() << __FUNCTION__ << "time to last resize event" << std::chrono::duration<double, std::milli>(t_now-t_old).count() << " ms";

    // refresh only if no other resize event occurs within the specified time
    timeout.stop();
    connect(&timeout, &QTimer::timeout, this, &KHomeView::refresh, Qt::UniqueConnection);
    timeout.setSingleShot(true);
    timeout.start(100);
    KMyMoneyViewBase::resizeEvent(event);
}

and  got with the appended testfile:

- app start (Qt5.12)  
resizeEvent time to last resize event 8.1e-05  ms (initial call)
refresh 336.841  ms
refresh 185.522  ms ( triggered by other event)
resizeEvent time to last resize event 690.682  ms
refresh 305.591  ms

- moving to another screen 
resizeEvent time to last resize event 53915.6  ms
refresh 172.17  ms

- resizing the window
resizeEvent time to last resize event 21941.2  ms
resizeEvent time to last resize event 35.1883  ms
resizeEvent time to last resize event 29.0735  ms
resizeEvent time to last resize event 37.9509  ms
resizeEvent time to last resize event 25.2898  ms
resizeEvent time to last resize event 31.865  ms
resizeEvent time to last resize event 24.6573  ms
resizeEvent time to last resize event 30.3148  ms
resizeEvent time to last resize event 20.7432  ms
resizeEvent time to last resize event 23.4848  ms
resizeEvent time to last resize event 19.4617  ms
resizeEvent time to last resize event 21.0152  ms
resizeEvent time to last resize event 18.9487  ms
resizeEvent time to last resize event 20.2506  ms
resizeEvent time to last resize event 20.7011  ms
resizeEvent time to last resize event 17.4308  ms
resizeEvent time to last resize event 29.7552  ms
resizeEvent time to last resize event 24.0264  ms
resizeEvent time to last resize event 16.496  ms
resizeEvent time to last resize event 16.8137  ms
resizeEvent time to last resize event 17.4071  ms
resizeEvent time to last resize event 23.0246  ms
resizeEvent time to last resize event 23.6899  ms
resizeEvent time to last resize event 16.7566  ms
resizeEvent time to last resize event 23.7812  ms
resizeEvent time to last resize event 17.9087  ms
resizeEvent time to last resize event 30.9371  ms
resizeEvent time to last resize event 13.8725  ms
resizeEvent time to last resize event 15.2983  ms
resizeEvent time to last resize event 15.8863  ms
resizeEvent time to last resize event 16.1352  ms
resizeEvent time to last resize event 15.8075  ms
resizeEvent time to last resize event 16.1257  ms
resizeEvent time to last resize event 16.1365  ms
resizeEvent time to last resize event 16.9236  ms
resizeEvent time to last resize event 23.0767  ms
resizeEvent time to last resize event 16.533  ms
resizeEvent time to last resize event 16.6189  ms
resizeEvent time to last resize event 16.8857  ms
resizeEvent time to last resize event 29.1898  ms
resizeEvent time to last resize event 16.5146  ms
resizeEvent time to last resize event 16.106  ms
resizeEvent time to last resize event 16.6079  ms
resizeEvent time to last resize event 16.2147  ms
resizeEvent time to last resize event 16.5906  ms
resizeEvent time to last resize event 16.1275  ms
resizeEvent time to last resize event 16.351  ms
resizeEvent time to last resize event 16.8019  ms
resizeEvent time to last resize event 16.6459  ms
resizeEvent time to last resize event 18.7296  ms
resizeEvent time to last resize event 18.1929  ms
resizeEvent time to last resize event 15.7177  ms
resizeEvent time to last resize event 16.428  ms
resizeEvent time to last resize event 16.1611  ms
refresh 118.69  ms

-> only one refresh
Comment 19 Ralf Habacker 2022-10-14 09:11:57 UTC 
The same with kmymoney from master branch on openSUSE Tumbleweed

- app startup
resizeEvent time to last resize event 0.000102  ms  (initial call)
refresh 1.13677  ms
resizeEvent time to last resize event 5.09184  ms
Open file QUrl("file:///home/admin/Downloads/testfile.kmy")
Start verifying account hierarchy
End verifying account hierarchy
Model for accounts loaded with 98 items in 0 ms
Model for parameters loaded with 2 items
Model for currencies loaded with 185 items
Model for prices loaded with 23 items in 0 ms
refresh 4.29173  ms
refresh 9.44309  ms
Start calculating balances: 0 splits
End calculating balances
refresh 4.53986  ms
resizeEvent time to last resize event 112.891  ms
resizeEvent time to last resize event 13.5078  ms
Processed home view section 8 in 0 ms
Processed home view section 1 in 0 ms
Processed home view section 2 in 0 ms
Processed home view section 3 in 0 ms
Processed home view section 4 in 0 ms
Processed home view section 5 in 2 ms
Processed home view section 6 in 107 ms
Processed home view section 7 in 1 ms
Processed home view section 10 in 0 ms
refresh 218.909  ms

- resizing the window
resizeEvent time to last resize event 47463.8  ms
resizeEvent time to last resize event 43.4126  ms
resizeEvent time to last resize event 44.4866  ms
resizeEvent time to last resize event 43.9953  ms
resizeEvent time to last resize event 32.2256  ms
resizeEvent time to last resize event 50.639  ms
resizeEvent time to last resize event 43.0902  ms
resizeEvent time to last resize event 31.8694  ms
resizeEvent time to last resize event 37.3693  ms
resizeEvent time to last resize event 29.984  ms
resizeEvent time to last resize event 34.5287  ms
resizeEvent time to last resize event 37.4108  ms
resizeEvent time to last resize event 40.4525  ms
resizeEvent time to last resize event 37.6339  ms
resizeEvent time to last resize event 37.2994  ms
resizeEvent time to last resize event 36.7078  ms
resizeEvent time to last resize event 49.4  ms
Processed home view section 8 in 0 ms
Processed home view section 1 in 0 ms
Processed home view section 2 in 0 ms
Processed home view section 3 in 0 ms
Processed home view section 4 in 0 ms
Processed home view section 5 in 2 ms
Processed home view section 6 in 83 ms
Processed home view section 7 in 0 ms
Processed home view section 10 in 0 ms
refresh 189.042  ms

-> only one refresh
Comment 20 Ralf Habacker 2022-10-14 09:47:20 UTC 
Created attachment 152801 [details]
bigger test case

With the bigger test case loaded I get the following values from kmymoney debug build from 5.1 branch  with displaying "Processed home view section ...  ms"  added from master branch.

- app start
resizeEvent time to last resize event 7.1e-05  ms (initial call)
Processed home view section 8 in 1256 ms
Processed home view section 1 in 15 ms
Processed home view section 2 in 1 ms
Processed home view section 3 in 84 ms
Processed home view section 4 in 0 ms
Processed home view section 5 in 1 ms
Processed home view section 6 in 1600 ms
Processed home view section 7 in 0 ms
Processed home view section 10 in 3 ms
refresh 3005.2  ms

Processed home view section 8 in 202 ms
Processed home view section 1 in 15 ms
Processed home view section 2 in 1 ms
Processed home view section 3 in 1 ms
Processed home view section 4 in 0 ms
Processed home view section 5 in 1 ms
Processed home view section 6 in 209 ms
Processed home view section 7 in 0 ms
Processed home view section 10 in 5 ms
refresh 470.117  ms (triggered by other event) 

- moving to another screen 
resizeEvent time to last resize event 3607.35  ms
Processed home view section 8 in 191 ms
Processed home view section 1 in 14 ms
Processed home view section 2 in 0 ms
Processed home view section 3 in 1 ms
Processed home view section 4 in 0 ms
Processed home view section 5 in 1 ms
Processed home view section 6 in 211 ms
Processed home view section 7 in 0 ms
Processed home view section 10 in 4 ms
refresh 459.127  ms

- resizing window
resizeEvent time to last resize event 81037  ms
Processed home view section 8 in 188 ms
Processed home view section 1 in 14 ms
Processed home view section 2 in 1 ms
Processed home view section 3 in 1 ms
Processed home view section 4 in 0 ms
Processed home view section 5 in 1 ms
Processed home view section 6 in 224 ms
Processed home view section 7 in 0 ms
Processed home view section 10 in 3 ms
refresh 470.032  ms
Comment 21 Ralf Habacker 2022-10-20 08:27:20 UTC 
(In reply to Dawid Wróbel from comment #15)
> Processed home view section 8 in 9145 ms

This issue, which was present before, is now traceable due to the changes made to this ticket, but is independent of the adjustments made for this ticket. A separate ticket should be created for this.
Comment 22 Dawid Wróbel 2022-10-20 11:12:36 UTC 
> This issue, which was present before, is now traceable due to the changes made to this ticket, but is independent of the adjustments made for this ticket. A separate ticket should be created for this.

I am not convinced, tbh. Yes, the underlying issue is slowness of the home view processing, but the frequency of rewriting the window increased to the point that resizing renders the app unusable is, in my opinion, a side effect that is not otherwise easily fixable and, arguably, a no lesser of an issue than the graph being rendered improperly. 

This fix effectively creates another big issue and I stand by my initial point that it's best not to touch home view altogether, as it is full of spaghetti code of which any modification can cause several other side effects that are hard to trace. I am saying as the author of its migration form WebEngine and if I was to do this again, I would instead just rewrite it from scratch using QML and never look back :)
Comment 23 Dawid Wróbel 2022-10-20 11:19:18 UTC 
(to clarify, I don't have a problem with this bug being fixed the way it was, just making sure we are on the same page regarding how badly that home page code needs rewriting)