Bug fixes and additions in Sharpmap 1.0 Ogr provider

Topics: Data Access, SharpMap v0.9 / v1.x
Dec 18, 2013 at 9:53 PM
Hi,

I don't know your process for handling bug fixes so I'll submit it here.
I found some bugs in the Ogr provider in SharpMap.Extensions when trying to use it to read S-57 maps.
Code included below. If anyone is interested in the full source file, let me know.

A brief summary of the fixes;

1. LayerName property - set operation
The _ogrDataSource.GetLayerByName call will not throw an exception if the specified layer name does not exist (as assumed by the current implementation).
Instead, the call returns null, which will cause _ogrLayer and ConnectionID to be set to invalid/inconsistent values.
My fix checks the result for null before setting _ogrLayer and ConnectionID;
        public string LayerName
        {
            get { return _ogrLayer.GetLayerDefn().GetName(); }
            set
            {
                try
                {
                    OgrLayer layer = _ogrDataSource.GetLayerByName(value);

                    //*** FIX: Must check for null since GetLayerByName returns null if layer name does not exist.
                    if (layer != null)
                    {
                        _ogrLayer = layer;
                        ConnectionID = string.Format("Data Source={0};Layer{1}", _ogrDataSource.name, value);
                    }
                }
                catch { }
            }
        }
OgrFeatureToFeatureDataRow
The data row column index (fdrIndex) will be out-of-sync if there are columns without a value i.e. if ogrFeature.IsFieldSet returns false.
My fix increments fdrIndex in this case unless the field is of an unsupported type;
private static FeatureDataRow OgrFeatureToFeatureDataRow(FeatureDataTable table, OSGeo.OGR.Feature ogrFeature, GeoAPI.Geometries.IGeometryFactory factory)
{
    FeatureDataRow fdr = table.NewRow();
    Int32 fdrIndex = 0;
    for (int iField = 0; iField < ogrFeature.GetFieldCount(); iField++)
    {
        //*** BEGIN FIX: Ensure fdrIndex is incremented if field value is not set and field is of a supported type.
        bool valueAvailable = ogrFeature.IsFieldSet(iField);
        OgrFieldType fieldType = ogrFeature.GetFieldType(iField);

        // No need to get field value if there's no value available...
        if (!valueAvailable)
        {
            // We need to increment the data row column index if this is a supported field type
            // otherwise the next field will be written to the wrong column...
            // We support all types except OFTBinary...
            // Supported field types must be consistent with the code in ReadColumnDefinition and the switch case below...
            if (fieldType != OgrFieldType.OFTBinary)
            {
                fdrIndex++;
            }

            continue;
        }
        //*** END FIX

        // Process field value...
        switch (fieldType)
        {
            case OgrFieldType.OFTString:
            case OgrFieldType.OFTWideString:
            {
                fdr[fdrIndex++] = ogrFeature.GetFieldAsString(iField);
                break;
            }
            case OgrFieldType.OFTInteger:
            {
                fdr[fdrIndex++] = ogrFeature.GetFieldAsInteger(iField);
                break;
            }
            case OgrFieldType.OFTReal:
            {
                fdr[fdrIndex++] = ogrFeature.GetFieldAsDouble(iField);
                break;
            }
            case OgrFieldType.OFTDate:
            case OgrFieldType.OFTDateTime:
            case OgrFieldType.OFTTime:
            {
                Int32 y, m, d, h, mi, s, tz;
                ogrFeature.GetFieldAsDateTime(iField, out y, out m, out d, out h, out mi, out s, out tz);
                        
                try
                {
                    if (y == 0 && m == 0 && d == 0)
                    {
                        fdr[fdrIndex++] = DateTime.MinValue.AddMinutes(h*60 + mi);
                    }
                    else
                    {
                        fdr[fdrIndex++] = new DateTime(y, m, d, h, mi, s);
                    }
                }
                catch { }
                break;
            }

            //*** NEW: Added support for OFTStringList, OFTWideStringList, OFTIntegerList and OFTRealList
            case OgrFieldType.OFTStringList:
            case OgrFieldType.OFTWideStringList:
            {
                fdr[fdrIndex++] = ogrFeature.GetFieldAsStringList(iField);
                break;
            }
            case OgrFieldType.OFTIntegerList:
            {
                int count = 0;
                fdr[fdrIndex++] = ogrFeature.GetFieldAsIntegerList(iField, out count);
                break;
            }
            case OgrFieldType.OFTRealList:
            {
                int count = 0;
                fdr[fdrIndex++] = ogrFeature.GetFieldAsDoubleList(iField, out count);
                break;
            }
            default:
            {
                Debug.WriteLine(string.Format("Cannot handle Ogr DataType '{0}'", fieldType));
                break;
            }
        }
    }

    using (var gr = ogrFeature.GetGeometryRef())
    {
        fdr.Geometry = ParseOgrGeometry(gr, factory);
        gr.Dispose();
    }
    return fdr;
}
I have also done some additions to be able to properly process my S-57 data;

Added support for additional field types
Support added for OFTStringList, OFTWideStringList, OFTIntegerList and OFTRealList to ReadColumnDefinition and OgrFeatureToFeatureDataRow methods.
For OgrFeatureToFeatureDataRow see code snipped above.
private static void ReadColumnDefinition(FeatureDataTable fdt, OgrLayer oLayer)
{
    using (OgrFeatureDefn ogrFeatureDefn = oLayer.GetLayerDefn())
    {
        int iField;

        for (iField = 0; iField < ogrFeatureDefn.GetFieldCount(); iField++)
        {
            using (OgrFieldDefn ogrFldDef = ogrFeatureDefn.GetFieldDefn(iField))
            {
                OgrFieldType type= ogrFldDef.GetFieldType();
                switch (type)
                {
                    case OgrFieldType.OFTInteger:
                        fdt.Columns.Add(ogrFldDef.GetName(), typeof(Int32));
                        break;
                    case OgrFieldType.OFTReal:
                        fdt.Columns.Add(ogrFldDef.GetName(), typeof(Double));
                        break;
                    case OgrFieldType.OFTString:
                        fdt.Columns.Add(ogrFldDef.GetName(), typeof(String));
                        break;
                    case OgrFieldType.OFTWideString:
                        fdt.Columns.Add(ogrFldDef.GetName(), typeof(String));
                        break;
                    case OgrFieldType.OFTDate:
                    case OgrFieldType.OFTTime:
                    case OgrFieldType.OFTDateTime:
                        fdt.Columns.Add(ogrFldDef.GetName(), typeof(DateTime));
                        break;

                    //*** NEW: Added support for OFTStringList, OFTWideStringList, OFTIntegerList and OFTRealList
                    case OgrFieldType.OFTStringList:
                    case OgrFieldType.OFTWideStringList:
                        fdt.Columns.Add(ogrFldDef.GetName(), typeof(String[]));
                        break;
                    case OgrFieldType.OFTIntegerList:
                        fdt.Columns.Add(ogrFldDef.GetName(), typeof(Int32[]));
                        break;
                    case OgrFieldType.OFTRealList:
                        fdt.Columns.Add(ogrFldDef.GetName(), typeof(Double[]));
                        break;
                    default:
                    {
                        //fdt.Columns.Add(_OgrFldDef.GetName(), System.Type.GetType("System.String"));
                        Debug.WriteLine("Not supported type: " + type + " [" + ogrFldDef.GetName() + "]");
                        break;
                    }
                }
            }
        }
    }
}
2. Added helper method ContainsLayer.
This is to allow user to check whether a layer name is defined in the active file;
//*** NEW: Added helper method to check if layer name exists...
public Boolean ContainsLayer(string layerName)
{
    using (OgrLayer layer = _ogrDataSource.GetLayerByName(layerName))
    {
        return (layer != null);
    }
}
Coordinator
Dec 19, 2013 at 7:45 AM
Edited Dec 19, 2013 at 7:49 AM
This discussion has been copied to a work item. Click here to go to the work item and continue the discussion.

Thanks for this very useful addition.