Plan View: Fix polyline/polygon vertex dragging freezing the UI

[mrschick]

Description

I've noticed that when dragging the vertices of long polylines or large polygons, the UI would often freeze up completely (to the point of "program is not responding") after a few seconds of lag, and only sometimes recover from that state after tens of seconds.
Managed to fix it by debouncing "pathChanged" emissions to occour on every event loop, instead of every frame, which prevents dragging of such polylines/polygons from clogging up the UI code with too many path emissions.

Test Steps

Try creating Corridor Scan items with many polyline vertices or Survey scans (both basic and circular) covering a large area. Dragging of any polyline/polygon vertex won't cause the UI to freeze up completely, it will at worst lag a bit but always recover when releasing the vertex (unlike before).

Checklist:

• Review Contribution Guidelines.
• Review Code of Conduct.
• I have tested my changes.

Related Issue

Couldn't find any, but it seems more likely than not that this was reported at some time in the past.

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

[HTRamsey]

Can you take a look at the unit tests?

[mrschick]

I've tried, but don't really understand from the action log which one is failing. EDIT: nvm, was looking at the log all wrong 🤦🏻‍♂️

Also can't really get my local environment to run the tests, the documentation seems to be outdated, from a time when deploy/qgroundcontrol-start.sh was still a thing.

[DonLakeFlyer]

Ah, interesting. Didn't realize there were perf problems here. The issue is that the pathChanged signal is getting emitted over and over again as you drag. These signals then stack up in the queue causing a massive amount of redraw. There is already code in QGC to handle cases like that. It's used a bunch during mission planning recalc of the waypoint lines.

The two methods are in QGCApplication:

    /// Registers the signal such that only the last duplicate signal added is left in the queue.
    void addCompressedSignal(const QMetaMethod &method);

    void removeCompressedSignal(const QMetaMethod &method);

We should try using addCompressedSignal to register the pathChanged signal as "compressed". By doing this only a single instance of the signal will be in the queue. That would limit the redraws to only when it gets back to the main event loop. Not quite sure if it will work as well as the timer though. Will have to try.

@mrschick When the pull that I linked to this finishes building can you try a build from there and see how it works for you in comparison?

[DonLakeFlyer]

Nope the compress signals thing doesn't work because they all get pushed on the main thread and aren't queued. I'll modify it.

[DonLakeFlyer]

The pathChanged signal needs to be a queued connection

[mrschick]

The pathChanged signal needs to be a queued connection

Too bad 😅
Could a framework that works with a timer instead be of more use?

[mrschick]

I've gotten the unit test to run on my machine now. I believe the issue that causes them to fail is directly caused by the changes, since the tests expect the signal to be sent synchronously, deferring it breaks that expectation.
Could the test be deferred too somehow?

[DonLakeFlyer]

I'll take a look at all of this and figure something out.

[DonLakeFlyer]

To fix the unit tests you need to add:

    QTest::qWait(100); // Let event loop process so queued signals flow through

After the calls which end up queueing the signals to the next from adjustVertext. This way the signals will flow through before you hit the test code.

I'm going to merge this in with the unit test failure and then I'll fix up the test with a separate pull.

The current state of this code works better than what we have now. but this will sequence needs a full review to be made to work much better in 5.1.

[DonLakeFlyer]

Thanks for the pull

[mrschick]

To fix the unit tests you need to add:

    QTest::qWait(100); // Let event loop process so queued signals flow through

Neat!

Thanks for the support.