reachpoint level recalculated when reachpoint moves

[urskaufmann]

Describe the bug
Reach with correct levels (I1 = 2736.51). I move the reachpoint with snapping, then the level is recalculated to 0.00 if there is snapping to a detail geometry or recalculated to the point-level if it snaps to a point (example: the cover).

To Reproduce
Exact steps to reproduce the behavior:

1. Use the Vertex tool
2. Move the end-point of a reach (reach-point) to a place, where it snaps
3. Control the reach-point level

Expected behavior
automatic levels if there exists already a level for a reachpoint will be often not wanted. If the new level is 0, then there shoul be never a change. If there is already a level and the new level changes, then the user should confirm the new level.

Screenshots / data
Start with I1=2736.51

after moving on the detail geometry: level is set to 0.00!

after moving to symbol = node=cover: level is equal to cover level (without confirm!)

If I move without snapping, the level does not change (as expected).

Desktop (please complete the following information):

• TWW version 0.0.0
• QGIS Version 3.34.4
• OS Win 10

[urskaufmann]

It also recalculates, if I move another vertex of the reach!

• the to-reachpoint is snapped on the detail geometry
• the reach point level is reset to 2736.51
• I move the vertex of the reach
• the to-reachpoint level that snaps to the detail geometry is set to 0.00
• the from-reachpoint level does not change.

But if I change also snaping on the from reachpoint side, also the from level is reset.

If I move to to-reachpoint away from the detail geometry and let let it snap on his own reach (shorten the line), the there is a very strange now reachpoint level (could be interpolated from vertex level = 0 and old to-reachpoint-level = 2736.51).
HELP - do not interpolated or calculate if there is a 0 value!!

[ponceta]

There's something broken in the synchronisation between reach points altitudes and reach geometries.

If you edit the altitude of the reach point the label is correctly recalculated.

[cymed]

The problem is that the progression3d_geometry snaps in 3d, which in turn is used in in tww_app.ft_vw_tww_reach_update(). We update the reach's progression3d_geometry as follows

      -- Synchronize geometry with level
      IF NEW.rp_from_level <> OLD.rp_from_level OR (NEW.rp_from_level IS NULL AND OLD.rp_from_level IS NOT NULL) OR (NEW.rp_from_level IS NOT NULL AND OLD.rp_from_level IS NULL) THEN
        NEW.progression3d_geometry = ST_ForceCurve(ST_SetPoint(ST_CurveToLine(NEW.progression3d_geometry),0,
        ST_MakePoint(ST_X(ST_StartPoint(NEW.progression3d_geometry)),ST_Y(ST_StartPoint(NEW.progression3d_geometry)),COALESCE(NEW.rp_from_level,'NaN'))));
      ELSE
        IF ST_Z(ST_StartPoint(NEW.progression3d_geometry)) <> ST_Z(ST_StartPoint(OLD.progression3d_geometry)) THEN
          NEW.rp_from_level = NULLIF(ST_Z(ST_StartPoint(NEW.progression3d_geometry)),'NaN');
        END IF;
      END IF;

    [same procedure with rp_to_level]

    UPDATE tww_od.reach_point SET
          [...]
          , situation3d_geometry = ST_StartPoint(NEW.progression3d_geometry)
        WHERE obj_id = OLD.rp_from_obj_id;


   [same procedure with rp_to_obj_id]

So if the rp_level did not change and the progession3d_geometry snapped to a wrong 3d Z coordinate, that Z coordinate is transferred to rp_level and rp_situation3d_geometry. The question is in which cases we want that z matching

1. If the rp_level is zero
2. If the rp_level is NULL
3. If the rp_level is distinct from the reach's Z value in general

I suggest either 1 and 2

      -------------------------------------
      -- Synchronize geometry with level --
      -------------------------------------

      -- Start point
      IF NEW.rp_from_level IS NULL OR NEW.rp_from_level=0 THEN
        NEW.rp_from_level = NULLIF(ST_Z(ST_StartPoint(NEW.progression3d_geometry)),NEW.rp_from_level);
      ELSE NULL;
      END IF;
      IF NEW.rp_from_level IS DISTINCT FROM ST_Z(ST_StartPoint(NEW.progression3d_geometry)) THEN
        NEW.progression3d_geometry = ST_ForceCurve(ST_SetPoint(ST_CurveToLine(NEW.progression3d_geometry),0,
        ST_MakePoint(ST_X(ST_StartPoint(NEW.progression3d_geometry)),ST_Y(ST_StartPoint(NEW.progression3d_geometry)),COALESCE(NEW.rp_from_level,'NaN'))));
      ELSE NULL;
      END IF;

      -- End point
     IF NEW.rp_to_level IS NULL OR NEW.rp_to_level=0 THEN
        NEW.rp_to_level = NULLIF(ST_Z(ST_EndPoint(NEW.progression3d_geometry)),NEW.rp_to_level);
     ELSE NULL;
     END IF;
     IF NEW.rp_to_level IS DISTINCT FROM ST_Z(ST_EndPoint(NEW.progression3d_geometry)) THEN
        NEW.progression3d_geometry = ST_ForceCurve(ST_SetPoint(ST_CurveToLine(NEW.progression3d_geometry),0,
        ST_MakePoint(ST_X(ST_EndPoint(NEW.progression3d_geometry)),ST_Y(ST_EndPoint(NEW.progression3d_geometry)),COALESCE(NEW.rp_from_level,'NaN'))));
      ELSE NULL;
      END IF;

or not at all

      -------------------------------------
      -- Synchronize geometry with level --
      -------------------------------------

      -- Start point
      IF NEW.rp_from_level IS DISTINCT FROM ST_Z(ST_StartPoint(NEW.progression3d_geometry)) THEN
        NEW.progression3d_geometry = ST_ForceCurve(ST_SetPoint(ST_CurveToLine(NEW.progression3d_geometry),0,
        ST_MakePoint(ST_X(ST_StartPoint(NEW.progression3d_geometry)),ST_Y(ST_StartPoint(NEW.progression3d_geometry)),COALESCE(NEW.rp_from_level,'NaN'))));
      ELSE NULL;
      END IF;

      -- End point
     IF NEW.rp_to_level IS DISTINCT FROM ST_Z(ST_EndPoint(NEW.progression3d_geometry)) THEN
        NEW.progression3d_geometry = ST_ForceCurve(ST_SetPoint(ST_CurveToLine(NEW.progression3d_geometry),0,
        ST_MakePoint(ST_X(ST_EndPoint(NEW.progression3d_geometry)),ST_Y(ST_EndPoint(NEW.progression3d_geometry)),COALESCE(NEW.rp_from_level,'NaN'))));
      ELSE NULL;
      END IF;

Does anyone use snapping on 3d nodes to set the z coordinate of reach points?

[ponceta]

In Pully we snap on nodes and construction points to get the 3D information.

[cymed]

Ok, so we need to switch it (overriding 0 and NaN elevations on start and End points).

Suggestion:

• We alter the geometry z level to the new rp_level if
  • the rp_level was altered
  • the new z level of the geometry is NaN or 0
• We alter the rp_level to the geometry level after that if the geometry Z value has changed (either by overriding NaN or 0 or by snapping to a 3d point

      -- Synchronize geometry with level
      IF 
        NEW.rp_from_level IS DISTINCT FROM OLD.rp_from_level  OR -- update when changing manually
        NULLIF(ST_Z(ST_StartPoint(NEW.progression3d_geometry)),'NaN') IS NULL OR
        ST_Z(ST_StartPoint(NEW.progression3d_geometry)) = 0  
      THEN
        NEW.progression3d_geometry = ST_ForceCurve(ST_SetPoint(ST_CurveToLine(NEW.progression3d_geometry),0,
       ST_MakePoint(ST_X(ST_StartPoint(NEW.progression3d_geometry)),ST_Y(ST_StartPoint(NEW.progression3d_geometry)),COALESCE(NEW.rp_from_level,'NaN'))));
      ELSE
        NULL;
      END IF;
 
      IF ST_Z(ST_StartPoint(NEW.progression3d_geometry)) <> ST_Z(ST_StartPoint(OLD.progression3d_geometry)) THEN
          NEW.rp_from_level = NULLIF(ST_Z(ST_StartPoint(NEW.progression3d_geometry)),'NaN');
      ELSE
        NULL;
      END IF;

    [same procedure with rp_to_level]

    UPDATE tww_od.reach_point SET
          [...]
          , situation3d_geometry = ST_StartPoint(NEW.progression3d_geometry)
        WHERE obj_id = OLD.rp_from_obj_id;


   [same procedure with rp_to_obj_id]

In @urskaufmann 's example, the rp_level would not be set to 0 after snapping to the detail_geometry3d_geometry, but it would take the cover elevation (we have to leave it to the user to know which layer he is snapping to - snapping to the wastewater node instead can make sense)

@urskaufmann @ponceta are you ok with that?

[urskaufmann]

Means this: if I want to correct my data, which is "Pickellochmodell" (the reachpoint.level is 492.00) and I draw a detail-geometry, and I now snap the reach to the detail-geometry (no more Pickelloch, the reach ends at the wall of the structure), then the reachpoint.level will be set to the cover.level (496.2)? Without remark or question to the user? I do not agree.
I think if a value other than 0 or NaN will be recalculated, then the user should decide, if the level changes or not.

[cymed]

Means this: if I want to correct my data, which is "Pickellochmodell" (the reachpoint.level is 492.00) and I draw a detail-geometry, and I now snap the reach to the detail-geometry (no more Pickelloch, the reach ends at the wall of the structure), then the reachpoint.level will be set to the cover.level (496.2)? Without remark or question to the user? I do not agree. I think if a value other than 0 or NaN will be recalculated, then the user should decide, if the level changes or not.

No.

Reachpoint.level was set to cover.level in your test because you snapped an existing reach to to a cover, which is a PointZ that had a Z value.
If you snap to a detail geometry segment, QGIS snaps to it in 2d and sets z=0. (although that might change upstream in the future as it is an undesired snapping behaviour)
If you snap to a detail geometry vertex, QGIS snaps to it in 3d and sets z to the node's Z.

With my change, Z=0 and Z=NaN from the reach geometry are ignored on the new trigger.

@ponceta do you snap on PointZ on creation only or also on update? The simplest solution would be to use the reach Z values on insert only and once the reach is created, we control the start and end point elevation from the rp_level fields

[sjib]

@urskaufmann Can you re-test this with 2024.0.2 pre-release and close if ok?

[cymed]

There has been no fix so far, #351 was closed

[ponceta]

I don't understand, it should have been reviewed and merged, not closed.

[ponceta]

@ponceta do you snap on PointZ on creation only or also on update? The simplest solution would be to use the reach Z values on insert only and once the reach is created, we control the start and end point elevation from the rp_level fields

We usually snap on creation and get the Z coordinates from construction points (Points Z) and wastewater_nodes (Points Z) if they exist.

When we want to get the Z when doing an update, we turn on the snapping topology tool which should enable QGIS to gather the Z on update geometrically speaking.

We disabled snapping on covers or wastewater structures (2d or curvepolygonz) to avoid problems.

[urskaufmann]

snapping on the detail-geometry still sets the level to 0.00

Still a no-go!

[ponceta]

This is true with QGEP too.

The simplest thing is to set an action on snapping which will check the altitude of the main node of the wastewater. Or snap on a construction pointZ (From GPS)

We could set a default altitude from the main wastewater node to the detail geometry egdes altitudes if they are 0? This would enable the snapping on edges (with QGIS topology activated) and nodes of the detail wastewater sturcture :