jugstalt / gview-gis

gView GIS - Userfriendly open source GI framework
Apache License 2.0
8 stars 4 forks source link

WKBToGeometry in OGC.cs #17

Open charga opened 1 year ago

charga commented 1 year ago

I found a bug with the WKB conversion:

// Get the type of this geometry. uint type = (uint)ReadUInt32(reader, (WkbByteOrder)byteOrder); int newSrid = (type & 0x20000000) != 0 ? reader.ReadInt32() : -1; <-- only if interestested on SRID

        bool hasZ = false, hasM = false;
        if (type >= 1000 && type < 2000)
        {
            hasZ = true;
            type -= 1000;
        }
        else if (type >= 2000 && type < 3000)
        {
            hasM = true;
            type -= 2000;
        }
        else if (type >= 3000 && type < 4000)
        {
            hasZ = hasM = true;
            type -= 3000;
        }
        **type = (type & 0xffff) % 1000;** <-- can type correctly to Enum
        if (!Enum.IsDefined(typeof(WKBGeometryType), type))
        {
            throw new ArgumentException("Geometry type not recognized");
        }
jugstalt commented 1 year ago

Thanks for the tip. The problem seems a bit complicated though. In addition to Stand WKB, there are also Extended WKB and ISO WKB.

In Extended WKB, optionally returns SRID if GeometryType is 0x20000000. However, Z and M values are also treated differently there.

My previous implementation only applies to standard WKB and ISO WKB (with Z and M values). I will add the extended implementation. Your suggestion would mix Extended and ISO and also provide incorrect results.

I used this site as a source for my research: https://libgeos.org/specifications/wkb/

Let me know if you don't agree with the standard described there and have a better source.

jugstalt commented 1 year ago

I think this implementation would be correct

private static IGeometry WKBToGeometry(BinaryReader reader, WkbByteOrder byteOrder)
{
    // Read geometry type
    uint geometryType = (uint)ReadUInt32(reader, byteOrder);

    bool hasZ, hasM, hasSRID;
    uint baseGeometryType;

    // Check if the higher bits are set (EWKB)
    if ((geometryType & 0xE0000000) != 0)
    {
        // Extract Z, M and SRID flags from geometry type
        hasZ = (geometryType & 0x80000000) != 0;
        hasM = (geometryType & 0x40000000) != 0;
        hasSRID = (geometryType & 0x20000000) != 0;
        baseGeometryType = geometryType & 0x1FFFFFFF;

        if (hasSRID)
        {
            // Read SRID
            uint srid = ReadUInt32(reader, byteOrder);
        }
    }
    else
    {
        // Assume ISO WKB format

        // Extract Z and M values from geometry type
        hasZ = (geometryType / 1000) % 2 != 0;
        hasM = (geometryType / 2000) % 2 != 0;
        baseGeometryType = geometryType % 1000;

        // ISO WKB does not store SRID
        hasSRID = false;
    }

   if (!Enum.IsDefined(typeof(WKBGeometryType), baseGeometryType ))
    {
        throw new ArgumentException("Geometry type not recognized");
    }

    switch((WKBGeometryType)baseGeometryType) ....
}
charga commented 1 year ago

Thanks for the advice, I proceeed to test