Vertex Tool uses old cached data resulting in corrupted data in db

[MorriganR]

Description of the bug

Vertex Tool uses old cached data resulting in corrupted data in db. This issue is very similar to closed one:
New node tool snapping index out of sync for transaction groups (and triggers in the DB) #25142 (Oct 4, 2017)

Scenario in order to reproduce the issue with video timestamps:

1. Create PostGIS layer with simple LineString geometry. (00:00)
2. Start QGIS twice. (00:02)
3. Open the same layer in both QGIS and start editing with Vertex Tool. (00:06)
4. If we change the geometry of LineString in one window, those changes are only displayed correctly. BUT Vertex Tool cached old/incorrect value. As result we see ghosts of previous versions of geometry. (01:00)
5. Move Feature on the oher hand works correctly. (01:35)
6. The same behaviour can be reproduced in one QGIS window. It is needed to open the same layer twice. (02:08)

QGIS and OS versions
QGIS-dev

Additional context
TestDataSet.zip

RTwwDDVnC5.mp4

[uclaros]

Could you check if snapping works for the correct updated line or for the wrong outdated line? ie. after moving and saving from window2 go to window1 and create a new line, trying to snap on the new and old existing lines.
The vertex tool uses QgsPointLocator for point selection which is basically how snapping is also handled.

[MorriganR]

I am using QGIS only for a few days, so I am new to it. But i believe that this is what you wanted.

tvwNcuAekt.mp4

[uclaros]

But i believe that this is what you wanted.

Yes, indeed, snapping also misbehaves.
So the problem is that the layer's QgsPointLocator is only aware of changes made to the layer itself and completely unaware of changes made to the datasource but not to the layer (ie other layer, database trigger etc)

You could resync things by changing the project CRS in order to force a rebuild to all layers' locators, but this is hacky as hell!

[nyalldawson]

This is exactly the situation that the notify support added for postgres a few years back was designed to help with.

See
https://oslandia.com/en/2017/10/07/refresh-your-maps-from-postgresql/
https://kartoza.com/en/blog/using-pgnotify-to-automatically-refresh-layers-in-qgis/

[uclaros]

I don't think so, notify is only to avoid having to refresh the map manually.
The locator snapping index though has no way to be refreshed manually, it is only updated by signals on the layer (featureAdded, featureDeleted, geometryChanged).

I have hit another related bug where the layer's locator thinks the layer is empty, and it is stuck like this, unable to snap on the layer or use vertex tool, unless I changed the CRS to force a rebuild.

[nyalldawson]

Hm - are you sure about that? I believe handling multi-user snapping was one of the main drivers behind the notify work.

In any case, I think it's the correct approach to handle this situation. Maybe we are just missing the code to invalidate the snapping cache when notified by postgres.

[uclaros]

This is not a postgres thing, it's provider independent.
I think what we need is a way for the snapping cache to be always in sync with the data, just like the map canvas is. We already do the expensive procedure of getFeatureRequesting on canvas update, so we already have the updated geometries. We could fire a signal after a successful getFeatureRequest (that does not skip geometry ofc) and connect it to a slot on the layer's point locator to update the geometry in the index.
Would it be too expensive?

[nyalldawson]

You probably want to check with @wonder-sk there. That was the approach really old qgis versions used before @wonder-sk reworked snapping, and it didn't work well in practice.

[MorriganR]

This issue is a duplicate of #31251

[MorriganR]

I have this problem's solution for my data set. I've made simple plugin which makes indexing strategy IndexExtent and connects some signals together.
MorriganR/index_updater@53e5ccf
Maybe it will be useful for someone.

[troopa81]

The same behaviour can be reproduced in one QGIS window. It is needed to open the same layer twice. (02:08)

For this use case, defining data dependencies between your two layers should do just fine (see here and data dependencies in here )

In any case, I think it's the correct approach to handle this situation. Maybe we are just missing the code to invalidate the snapping cache when notified by postgres.

I agree. For now, we just trigger repaint, if we fire a dataSourceChanged or dataChanged it should fix the issue. I'm gonna make some tests and propose a PR.

I think what we need is a way for the snapping cache to be always in sync with the data, just like the map canvas is. We already do the expensive procedure of getFeatureRequesting on canvas update, so we already have the updated geometries.

Building the snapping index is expensive, you can't do it every time you render the canvas, people are already complaining about the time it takes when they want to edit large dataset. Hence the tip to change the indexing strategy with IndexExtent, but that's a workaround, not a fine way to deal with this.

My thoughts on how to properly manage indexing:

• keep a feature rendered state while rendering to avoid re-rendering when indexing
• Merge IndexHybrid and IndexExtent and have only one strategy that index the current extent and update the index when panning/zooming/moving instead of rebuilding it completely
• Automatically disable snapping when there is too many feature to index (large scale for instance, but there is already a feature to disable it with some parameters).

I just have to find time to POC this and create a QEP if it works.

[MorriganR]

The same behaviour can be reproduced in one QGIS window. It is needed to open the same layer twice. (02:08)

For this use case, defining data dependencies between your two layers should do just fine (see here and data dependencies in here )

@troopa81 thank you for your time. I tried data dependencies in my project, but this functionality is blocked in last QGIS releases by this PR #37475. I proposed a PR #41463 that is supposed to fix this data dependencies issue.