Skip to content

Fix: Ghosting effect on the flight path when dragging the takeoff point. #13810

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
Eureak-zon wants to merge 3 commits into
base: master
Choose a base branch
from
+5 −2
Open

Fix: Ghosting effect on the flight path when dragging the takeoff point. #13810

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
ee1923f
Fix: Ghosting effect on the flight path when dragging the takeoff point.
Eureak-zon Dec 24, 2025
0aad72d
Merge branch 'master' into fix-takeoff-ghosting
Eureak-zon Dec 24, 2025
d4555c4
Merge branch 'master' into fix-takeoff-ghosting
Eureak-zon Dec 25, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/MissionManager/MissionController.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1882,7 +1882,7 @@ void MissionController::_initAllVisualItems(void)
}
}

connect(_settingsItem, &MissionSettingsItem::coordinateChanged, this, &MissionController::_recalcAll);
//connect(_settingsItem, &MissionSettingsItem::coordinateChanged, this, &MissionController::_recalcAll);
Copy link

Copilot AI Dec 25, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This commented-out code should be removed entirely rather than left as a comment. The disconnect for this connection in _deinitAllVisualItems (line 1916) should also be removed to maintain consistency.

Suggested change
//connect(_settingsItem, &MissionSettingsItem::coordinateChanged, this, &MissionController::_recalcAll);

Copilot uses AI. Check for mistakes.
Copy link
Collaborator

@DonLakeFlyer DonLakeFlyer Dec 31, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this can be disconnected. I need to dig into the recalc code a bunch more. But the location of the home position drives things like mission status lengths/times and so forth. By disconnecting there is some info down deep in the bowels of MissionController that will be wrong now I believe. Gonna take a pile more review on my part to verify.

Copy link
Collaborator

@DonLakeFlyer DonLakeFlyer Jan 3, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Getting a bit closer to re-educating myself on all of this. I think the coordinateChanged signal needs to be connected to the signal which recalcs just mission flight status instead. Not 100% sure yet. After being away from MissionController it's so dense to get back into! Could use a full re-write to make it more understandable.

connect(_settingsItem, &MissionSettingsItem::coordinateChanged, this, &MissionController::plannedHomePositionChanged);

for (int i=0; i<_visualItems->count(); i++) {
Expand Down
5 changes: 4 additions & 1 deletion src/MissionManager/TakeoffMissionItem.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,10 @@ void TakeoffMissionItem::_init(bool forLoad)
{
_editorQml = QStringLiteral("qrc:/qml/QGroundControl/Controls/SimpleItemEditor.qml");

connect(_settingsItem, &MissionSettingsItem::coordinateChanged, this, &TakeoffMissionItem::launchCoordinateChanged);
connect(_settingsItem, &MissionSettingsItem::coordinateChanged, this, [=](const QGeoCoordinate& coordinate) {
Copy link

Copilot AI Dec 25, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The lambda capture [=] is deprecated in C++20 when capturing this implicitly. Use [this] instead to be explicit about capturing the object pointer. This matches the pattern used elsewhere in the codebase (see GeoFenceController.cc:108 and RallyPointController.cc:69 for examples).

Suggested change
connect(_settingsItem, &MissionSettingsItem::coordinateChanged, this, [=](const QGeoCoordinate& coordinate) {
connect(_settingsItem, &MissionSettingsItem::coordinateChanged, this, [this](const QGeoCoordinate& coordinate) {

Copilot uses AI. Check for mistakes.
SimpleMissionItem::setCoordinate(coordinate);
Copy link

Copilot AI Dec 25, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The lambda unconditionally updates the SimpleMissionItem coordinate when the settings item coordinate changes, without checking the _launchTakeoffAtSameLocation flag. This may cause the takeoff point to follow the launch point even when the user has configured them to be at different locations. Consider checking _launchTakeoffAtSameLocation before calling SimpleMissionItem::setCoordinate(coordinate), similar to the logic in TakeoffMissionItem::setCoordinate at lines 113-114.

Suggested change
SimpleMissionItem::setCoordinate(coordinate);
if (_launchTakeoffAtSameLocation) {
SimpleMissionItem::setCoordinate(coordinate);
}

Copilot uses AI. Check for mistakes.
Copy link
Collaborator

@DonLakeFlyer DonLakeFlyer Jan 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This isn't correct either. In the fly view the home position which is kept in the settings items can differ from the takeoff coordinate. In the plan view it's the takeoff item job to drag the launch position along with it. So the user doesn't have the ability to move the home position if they are using a takeoff item. It's all ridiculously complicated.

Copy link
Collaborator

@DonLakeFlyer DonLakeFlyer Jan 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And then for a fixed wing on PX4 it's all different!

emit launchCoordinateChanged(coordinate);
});

if (_flyView) {
_initLaunchTakeoffAtSameLocation();
Expand Down
Loading