Bug report #4079

(Some) features with invalid geometries not labelled

Added by Alister Hood almost 13 years ago. Updated about 5 years ago.

Status:Closed
Priority:Normal
Assignee:-
Category:Labelling
Affected QGIS version:master Regression?:No
Operating System: Easy fix?:No
Pull Request or Patch supplied:No Resolution:fixed/implemented
Crashes QGIS or corrupts data:No Copied to github as #:14062

Description

I had the attached "SW overland flow catchments" shape file in a project and found that when I tried to label it one of the largest catchments was not labelled.
I right-clicked on it and did save-as to create a copy. Testing I found that the same label was aways missing in the "SW..." file, but never missing in the other file.
I deleted the attributes from the "Descriptio" field in both files, then saved them and the project.
Now when I test with this project or with a fresh project, they have swapped around: the same label is always missing in the "temp" shape file, but not in the "SW..." file.
What is causing this? (Answer = invalid geometries; refer to Larry's comment)

no_label.JPG (56.5 KB) Alister Hood, 2011-07-14 02:19 AM

strange_label_locations.JPG (54.5 KB) Alister Hood, 2011-07-14 02:19 AM

temp.zip - demonstration shape files and project file (8.08 KB) Alister Hood, 2011-07-14 02:19 AM

History

#1 Updated by Alister Hood almost 13 years ago

N.B. also these files are quite good to demonstrate another problem with the label placement. Turn both layers on: why do the labels end up in such strange locations?

This is resolved by enabling the option "features don't act as obstacles for labels"; I was misunderstanding something before ;)

#2 Updated by Alister Hood almost 13 years ago

An interesting thing which might be related:

Using the "shapefile viewer" plugin, the shape with the missing label does not show up, although it does show up for the other file where the label isn't missing. But one of the other shapes doesn't actually show up for either file...

#3 Updated by Alister Hood almost 13 years ago

Also, strangely, if I select the object with the node tool, the layer where the label does show up says there is one geometry error (although there is no green cross indicating where it is), but the layer where the label doesn't show up says there is not.

#4 Updated by Alister Hood almost 13 years ago

And if I copy and paste the object that supposedly has a geometry error into the other file, then the label does show up. But then it disappears when I save the changes to the layer.

#5 Updated by Alister Hood almost 13 years ago

And if I copy and paste the object that supposedly has a geometry error into the other file, then the label does show up. But then it disappears when I save the changes to the layer.

Except I have now managed to copy it in and save the layer without the label disappearing... and I don't know what I did differently!

#6 Updated by Alister Hood almost 13 years ago

Another interesting thing: the file which does show the label (and which is supposed to have an invalid geometry) crashes globe plugin if the labels are turned on and I zoom right in :)

#7 Updated by Alister Hood almost 13 years ago

  • Pull Request or Patch supplied set to No

Another interesting thing: the file which does show the label (and which is supposed to have an invalid geometry) crashes globe plugin if the labels are turned on and I zoom right in :)

Actually: forget about the globe plugin. With labels turned on both files cause a crash when zooming in close in QGIS itself :(

I didn't think this was happening previously (I've updated my trunk build this afternoon)... but I guess I was probably wrong.

#8 Updated by Alister Hood almost 13 years ago

Using the VCexpress debugger I get the error message "Unhandled exception at 0x014d3ac3 (qgis_core.dll) in qgis.exe: 0xC0000005: Access violation reading location 0x0000000d.", and if I click Break it goes to this line in qgsgeometry.cpp:
if ( mDirtyGeos )

#9 Updated by Alister Hood almost 13 years ago

Oh, and there's this in the output window:
First-chance exception at 0x7c812afb in qgis.exe: Microsoft C++ exception: geos::util::TopologyException at memory location 0x0131ca68..
First-chance exception at 0x7c812afb in qgis.exe: Microsoft C++ exception: geos::util::TopologyException at memory location 0x0131ca5c..
First-chance exception at 0x7c812afb in qgis.exe: Microsoft C++ exception: geos::util::TopologyException at memory location 0x0131c9f4..
First-chance exception at 0x7c812afb in qgis.exe: Microsoft C++ exception: geos::util::TopologyException at memory location 0x0131cdc8..

#10 Updated by Alister Hood almost 13 years ago

  • Priority changed from Normal to High

I guess this should be "High" priority, since it causes a crash (and perhaps especially since I managed to create the offending shape files in QGIS :) )

#11 Updated by Alister Hood over 12 years ago

For the record
I believe commit 05cdd5247a3435f84be06a27a4b4e213ed861e7d fixed the crashes, although I didn't test in the globe.
The label is still missing.

#12 Updated by Paolo Cavallini over 12 years ago

  • Category changed from Symbology to Labelling

#13 Updated by Giovanni Manghi over 12 years ago

  • Target version set to Version 1.7.4

#14 Updated by Giovanni Manghi over 12 years ago

  • Crashes QGIS or corrupts data set to No
  • Status changed from Open to Feedback
  • Affected QGIS version set to master

Hi,
this is issue appears to be the same of #4634. Can you check it so we can eventually close that ticket as duplicate?

#15 Updated by Alister Hood over 12 years ago

I can't reproduce #4634, but I can still reproduce this one.

#16 Updated by Alister Hood about 12 years ago

I suspect this is the same issue as #3517 and #2949

#17 Updated by Paolo Cavallini about 12 years ago

  • Target version changed from Version 1.7.4 to Version 1.8.0

#18 Updated by Paolo Cavallini over 11 years ago

  • Target version changed from Version 1.8.0 to Version 2.0.0

#19 Updated by Giovanni Manghi over 11 years ago

  • Resolution set to fixed

With the lastest changes in labelling code this seems to be fixed, at least the "strange label location" issue. The "missing" labels are easy to explain: that polygons missing the label (in the attached picture) are not features of their own, but are "parts" of other polygons in the layer.

#20 Updated by Giovanni Manghi over 11 years ago

  • Status changed from Feedback to Closed

#21 Updated by Alister Hood over 11 years ago

  • Status changed from Closed to Reopened
  • Priority changed from High to Normal

The "missing" labels are easy to explain: that polygons missing the label (in the attached picture) are not features of their own, but are "parts" of other polygons in the layer.

No, I'm not complaining about the small parts of multipart polygons not being labelled.
One of the polygons is not labelled at all (in one of the files). This polygon consists of two parts which are touching each other.

#22 Updated by Alister Hood over 11 years ago

I see that now it is labelled when labelling with "around centroid", just not with "free".

#23 Updated by Larry Shaffer over 11 years ago

Hi Alister,

With recent changes, 'around' and 'offset' have their centroids converted to points (via GEOS) before being processed by PAL. This fixed some issues with how PAL was figuring centroids, but also means (apparently) that GEOS is somewhat adept at handling 'GEOS-invalid' geometries for some operations. The options Free, Horizontal, and Perimeter are still figured in PAL using the polygon if it is valid.

If you select the geometry in question that is not producing a label and run the following in Python Console:

 from qgis.utils import iface
 f = iface.activeLayer().selectedFeatures()[0]
 f. geometry().isGeosValid()

It returns False, i.e. an invalid GEOS geometry (as you noted above). Even though invalid, GEOS can generate a valid centroid for it. However, the following line in qgspallabeling.cpp will essentially skip running the polygon geometry through PAL (i.e. it's not PAL ignoring the geometry):

https://github.com/qgis/Quantum-GIS/blob/master/src/core/qgspallabeling.cpp#L723

What's needed here is a more robust means of trying to either fix invalid geometries on-the-fly, or for those geometries to be replaced with a temporary valid proxy via some other operation (like the 'buffer 0' trick) for labeling. Geometries are already duplicated (then deleted) for creating PAL labels.

More info on fixing geometries #4634-17

Update: as a test, I commented out qgspallabeling.cpp#L725-6, to see if PAL could also deal with the invalid geometry (or crash), and it did not, i.e. the label was not shown. So fixing invalid geometry before processing with PAL seems the thing to do.

I also tried a simple fix for invalid geometries with:


  QgsGeometry* geom = f.geometry();
  if ( !geom )
  {
    // try simple fix invalid polygons
    if ( geom->type() == QGis::Polygon )
    {
      geom = geom->buffer( 0.0, 8 ); // try simple buffer 0 fix

      if ( !geom )
      {
        return; // invalid geometry
      }
    }
    else
    {
      return; // invalid geometry
    }
  }

and later when converting to GEOS

  if ( geos_geom == NULL )
  {
    // try simple fix invalid polygons
    if ( geom->type() == QGis::Polygon )
    {
      geos_geom = geom->buffer( 0.0, 8 )->asGeos(); // try simple buffer 0 fix

      if ( geos_geom == NULL )
      {
        return; // invalid geometry
      }
    }
    else
    {
      return; // invalid geometry
    }
  }

But, this did not fix the 'temp' polygon. The only reason I can think of for why your 'SW...' file is labeling the invalid polygon, is that while QgsGeometry reports it is invalid, for whatever reason, it can still be converted to a GEOS geometry and used by PAL.

#24 Updated by Alister Hood almost 11 years ago

  • Subject changed from Mysterious missing label to (Some) features with invalid geometries not labelled

#25 Updated by Alvaro Huarte about 10 years ago

  • Resolution deleted (fixed)

I have implemented the ST_MakeValid function for QgsGeometry class to fix the related issues #9861 and #9777. The code is a fork from the original code in postgis implementation with slight changes:

https://github.com/postgis/postgis/blob/svn-trunk/liblwgeom/lwgeom_geos_clean.c

Also, the user can configure automatic validation of the fetched geometries of a layer and convert them to valid when needed.

https://github.com/qgis/QGIS/pull/1241

Best regards
Alvaro

#26 Updated by Jürgen Fischer almost 10 years ago

  • Target version changed from Version 2.0.0 to Future Release - Lower Priority

#27 Updated by Giovanni Manghi almost 7 years ago

  • Regression? set to No
  • Easy fix? set to No

#28 Updated by Giovanni Manghi about 5 years ago

  • Status changed from Reopened to Closed
  • Resolution set to end of life

#29 Updated by Alister Hood about 5 years ago

  • Description updated (diff)
  • Resolution changed from end of life to fixed/implemented

I believe https://github.com/qgis/QGIS/pull/1617 is what fixed it.

Also available in: Atom PDF