Skip to content

Rework handling of panning to identified features - #4591

Open
uclaros wants to merge 11 commits into
masterfrom
bugfix/better-jump-to
Open

uclaros wants to merge 11 commits into
masterfrom
bugfix/better-jump-to

Conversation

@uclaros

@uclaros uclaros commented Jul 10, 2026 •

Copy link
Copy Markdown
Contributor

Fixes #3845
This PR is a rework on how the map jumps to geometries.
Right now, a double animated pan happens is some cases while the feature form preview drawer is opening.
This is one thing handled by this PR: a single animation is performed while the drawer is opening.

The main change in behavior though is the following:

  • We calculate the effective map canvas size with the form drawer open. This can change from layer to layer based on the attributes shown on the preview.
  • We calculate a safe extent roughly 82% of that effective map size.
  • When tapping on a feature on the map, highlight the feature and apply the following logic based on the location of the identified geometry:
    • If the geom is already within the safe extent, then the map canvas does not pan.
    • If the geom (or part of it) is outside the safe extent but does fit within it, then pan the map canvas so that the geom is centered to the safe extent.
    • If the geom (or part of it) is outside the safe extent and would not fit within it, then pan the map canvas so that the tapped map location is centered to the safe extent.
Peek 2026-09-24 18-11 Peek 2026-09-24 18-14 Peek 2026-09-24 18-16

In all of the above the map canvas scale is unchanged and the pan is animated.

  • When selecting a feature from a list of features instead of the map canvas, then:
    • Animate zooming to, until the selected feature geom fits centered inside the safe extent
Peek 2026-09-24 18-18

The advantages of this logic are that:
Identifying points or small geometries near the center of the (effective) screen does not modify the extent and the user does not need to re-orient himself (google maps does the same for POIs)
Scale is always preserved while navigating the map.
Identifying large geometries does not modify the map extent too much.

Testing notes:

  • need to test all Geometries that we support (Points, Lines, Polygons and their multi variants)

uclaros added 7 commits July 10, 2026 11:44
We will use it to jump to the highlighted feature once we know
the available area of the map that is not covered by the panel
- Use the renamed previewPanelHeight property instead of signal params
- Move all highlighting and form opening to a identifyFeature()
- Don't pan if whole feature is near visible map center
- Center feature to visible map if it fits
- Center clicked location to visible map if whole does not fit
- Add method to calculate the map extent required for a geometry to
  fit the visible part of the map canvas when covered by a drawer
- Use animated zoom when identifying features from a list and not on map
@Withalion

Copy link
Copy Markdown
Collaborator

might help resolve #3845 as well

@Withalion Withalion left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Nice! It would be great to rebase on current master as well since some time passed.

Comment thread app/qml/map/MMMapCanvas.qml Outdated
Comment thread app/qml/map/MMMapCanvas.qml Outdated
Comment thread app/qml/map/MMMapCanvas.qml
Comment thread app/qml/map/MMMapController.qml Outdated
// Holds the map coordinates of the point the user identified. NaN if identify was triggered from list of features
property point identifyLocation: Qt.point(NaN, NaN)

signal featureIdentified( var pair, var point )

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Suggested change
signal featureIdentified( var pair, var point )
signal featureIdentified( FeatureLayerPair pair, qgsPoint clickedPoint )

It's about time we expose FeatureLayerPair & QgsPoint to QML properly

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Well yes, but do you think this PR is the proper place to do it?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

QgsPoint is already exposed, I believe FeatureLayerPair can be exposed with just a few lines, so why not. You don't have to apply the refactoring everywhere just here is enough

Comment thread app/test/testutilsfunctions.cpp
Comment thread app/test/testutilsfunctions.cpp
}
}

void TestUtilsFunctions::testDrawerCompensatedExtent()

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

As well here I would have case for:

  • when the map is just panned
  • when feature is bigger then current extent, so the map zooms out
  • when feature is smaller then current extent

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This only tests the extent calculation when compensating for the drawer height.

Testing the actual panning to and zooming to would be a completely different case.

Comment thread app/test/testutilsfunctions.cpp Outdated
Comment thread app/test/testutilsfunctions.cpp Outdated
@Withalion Withalion modified the milestones: 2026.4.0, 2026.5.0 Aug 25, 2026
@Withalion

Copy link
Copy Markdown
Collaborator

@uclaros can you please imrpove the description of this issue. What's happening, what's the issue. Mostly for testers to understand the issue better.

@Withalion Withalion removed this from the 2026.5.0 milestone Aug 27, 2026
@github-actions

Copy link
Copy Markdown

📦 Build Artifacts Ready

OS Status Build Info Workflow run
macOS Build 📬 Mergin Maps 72391 dmg Expires: 25/11/2026 #7239
linux Build ❌ Build failed or not found. #7265
win64 Build 📬 Mergin Maps 64411 win64 Expires: 25/11/2026 #6441
Android Build 📬 Mergin Maps 854911 APK [armeabi-v7a] Expires: 25/11/2026 #8549
📬 Mergin Maps 854911 APK [armeabi-v7a] Google Play Store #8549
Android Build 📬 Mergin Maps 854951 APK [arm64-v8a] Expires: 25/11/2026 #8549
📬 Mergin Maps 854951 APK [arm64-v8a] Google Play Store #8549
iOS Build 📬 Build number: 26.08.949111 #9491

@github-actions

Copy link
Copy Markdown

Coverage Report for CI Build 36013951345

Coverage increased (+0.1%) to 59.636%

Details

  • Coverage increased (+0.1%) from the base build.
  • Patch coverage: No coverable lines changed in this PR.
  • 369 coverage regressions across 1 file.

Uncovered Changes

No uncovered changes found.

Coverage Regressions

369 previously-covered lines in 1 file lost coverage.

File Lines Losing Coverage Coverage
mm/app/inpututils.cpp 369 55.77%

Coverage Stats

Coverage Status
Relevant Lines: 15764
Covered Lines: 9401
Line Coverage: 59.64%
Coverage Strength: 93.96 hits per line

💛 - Coveralls

@github-actions

Copy link
Copy Markdown

📦 Build Artifacts Ready

OS Status Build Info Workflow run
macOS Build 📬 Mergin Maps 73331 dmg Expires: 23/12/2026 #7333
linux Build 📬 Mergin Maps 73591 x86_64 Expires: 23/12/2026 #7359
win64 Build 📬 Mergin Maps 65351 win64 Expires: 23/12/2026 #6535
Android Build 📬 Mergin Maps 864551 APK [arm64-v8a] Expires: 23/12/2026 #8645
📬 QR code Google Play Store #8645
Android Build 📬 Mergin Maps 864511 APK [armeabi-v7a] Expires: 23/12/2026 #8645
📬 QR code Google Play Store #8645
iOS Build 📬 Build number: 26.09.958511 #9585

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

New feature highlight behaviour design

2 participants