Train widgets updating continuously, whether they are visible or not

40 views
Skip to first unread message

Peret

unread,
Mar 6, 2025, 7:27:55 AM3/6/25
to golden-cheetah-developers
Debugging code I have detected an undesired behavior in ElevationChartWindow::telemetryUpdate():

isHidden() is not working as expected. That function always returns false.
The easy solution is to change it for !isVisible(), that is proven to work.
It is recommended, I think, just to avoid computing things if not used at all.

So, I am proposing a new PR with that change, as it is obvious.

With that in mind, I have noticed that any other widget or video widget included in any layout, is being called for its telemetryUpdate() slot.
In particular, LiveMap widget, or even the video widget, are being updated continuously, despite they are not in the current layout.
There can be generic solutions, maybe implementing QWidget::showEvent() and hideEvent() at GcChartWindow. Those events are correctly matched, I have checked that.
But perhaps it is not recommended to generalize, as some widgets are computing averages, or whatever they need to do (I have not investigated), and logic to preserve all of functionalities already working may be complex.

I am proposing to review some of them, the most cpu/network demanding: starting by video and live map, by myself, and sharing ideas before requesting PR. I am newbie at Qt. If you agree with that.

Ale Martinez

unread,
Mar 6, 2025, 10:33:07 AM3/6/25
to golden-cheetah-developers
El jueves, 6 de marzo de 2025 a la(s) 9:27:55 a.m. UTC-3, Peret escribió:
Debugging code I have detected an undesired behavior in ElevationChartWindow::telemetryUpdate():

isHidden() is not working as expected. That function always returns false.
The easy solution is to change it for !isVisible(), that is proven to work.
It is recommended, I think, just to avoid computing things if not used at all.

So, I am proposing a new PR with that change, as it is obvious.

I merged your PR, thank you.
 
With that in mind, I have noticed that any other widget or video widget included in any layout, is being called for its telemetryUpdate() slot.
In particular, LiveMap widget, or even the video widget, are being updated continuously, despite they are not in the current layout.
There can be generic solutions, maybe implementing QWidget::showEvent() and hideEvent() at GcChartWindow. Those events are correctly matched, I have checked that.
But perhaps it is not recommended to generalize, as some widgets are computing averages, or whatever they need to do (I have not investigated), and logic to preserve all of functionalities already working may be complex.

I am proposing to review some of them, the most cpu/network demanding: starting by video and live map, by myself, and sharing ideas before requesting PR. I am newbie at Qt. If you agree with that.

It seems a good idea, please comment your findings, thank you. 

Peret

unread,
Mar 8, 2025, 11:23:44 AM3/8/25
to golden-cheetah-developers
I have had a look at videowindow widget. In the process of analyzing its workflow, prior to optimize its behavior, if possible, I have found some minor fixes, not fully related to its optimization, although as well,  that makes the user experience more robust, in my opinion. Due to complexity of the workflow, some weird behaviors remained unfixed:
- VideoWindow::startPlayback() was called twice from TrainSidebar::Start() I have not found any reason. Not relevant, but unneccessary
- Stopping and starting a workout made the video to be deselected, due to the complex workflow to take into account a change or deselection of workout. So, the video was not seen afterwards. If you wanted to stop and start again the same workout, you needed to stop/start an even number of times.
- The most relevant: when swicthing the layout to a one without video, video widgets remained visible, overlapping the layout. This is very annoying when starting a training session with no video: odd number of starts show videow widgets overlapped (in particular, the first start)
This is due to: all widgets of all train layouts are created when slecting train view, none of the widgets are hidden when they are not necessary, and the odd number of starts required is due to previous point

Some of them (hiding/showing widgets) is relevant for the optimization of the widgets, but can be implemented beforehand. I think these changes are useful, so I am asking for a PR, and you can tell me whether it is ok, can be revised, or rejected if not necessary.
I will continue with hiding/showing and not computing widgets when not necessary, if possible.

Peret

unread,
Mar 13, 2025, 4:55:48 AM3/13/25
to golden-cheetah-developers
I have added the optimization of widgets as charts as part of the PR, as they are related, and changes are not that big to create different PRs, in my opinion; let me know if it is ok or prefer another way.

Ale Martinez

unread,
Mar 13, 2025, 10:28:54 AM3/13/25
to golden-cheetah-developers
El jueves, 13 de marzo de 2025 a la(s) 5:55:48 a.m. UTC-3, Peret escribió:
I have added the optimization of widgets as charts as part of the PR, as they are related, and changes are not that big to create different PRs, in my opinion; let me know if it is ok or prefer another way.

I thibk it is Ok provided they are related changes
 
El sábado, 8 de marzo de 2025 a las 17:23:44 UTC+1, Peret escribió:
I have had a look at videowindow widget. In the process of analyzing its workflow, prior to optimize its behavior, if possible, I have found some minor fixes, not fully related to its optimization, although as well,  that makes the user experience more robust, in my opinion. Due to complexity of the workflow, some weird behaviors remained unfixed:
- VideoWindow::startPlayback() was called twice from TrainSidebar::Start() I have not found any reason. Not relevant, but unneccessary
- Stopping and starting a workout made the video to be deselected, due to the complex workflow to take into account a change or deselection of workout. So, the video was not seen afterwards. If you wanted to stop and start again the same workout, you needed to stop/start an even number of times.

This is unrelated and it should be removed, I cannot reproduce it, it is not a common workflow and mostly cosmetic, besides de proposed solutions is too convoluted.

Peret

unread,
Mar 13, 2025, 1:54:11 PM3/13/25
to golden-cheetah-developers
Ok, thanks for the comments. I have reverted the changes related to those two issues.
Reply all
Reply to author
Forward
0 new messages