mapbox / vector-tile-spec

Mapbox Vector Tile specification
https://www.mapbox.com/vector-tiles/specification/
Other
891 stars 210 forks source link

proto3 support #90

Open kiambogo opened 7 years ago

kiambogo commented 7 years ago

I am wanting to use a newer protobuf compiler (https://github.com/tiziano88/elm-protobuf) which only supports proto3 syntax. Is an update to proto3 in scope at this time?

flippmoke commented 7 years ago

Currently the specification still uses proto2, but it is possible that it could be done as part of a future 3.0. I believe that if you manually enforce the defaults then using a modified .proto file should work just fine for proto3 though?

kiambogo commented 7 years ago

Thanks for the fast reply! I'm quite new to protobufs, so I wanted to check whether this was a change that was going to be implemented soon before modifying the file myself.

I'll take a crack at updating it myself then.

joto commented 7 years ago

Proto3 has a few differences which might make some things not work:

strk commented 5 years ago

Is the WARNING emitted by the protobuf compiler something that could be fixed by being explicit about using "proto2" syntax ? I'm talking about this one:

/usr/bin/protoc-c --c_out=. vector_tile.proto [libprotobuf WARNING google/protobuf/compiler/parser.cc:547] No syntax specified for the proto file: vector_tile.proto. Please use 'syntax = "proto2";' or 'syntax = "proto3";' to specify a syntax version. (Defaulted to proto2 syntax.)

It would be nice to get rid of that. Reference: https://trac.osgeo.org/postgis/ticket/3831

joto commented 5 years ago

Yes, we could get rid of the warning with an explicit setting. On the other hand older protoc implementations don't understand that syntax, so to change this would introduce a needless incompatibility.

strk commented 5 years ago

For the record, do you know how would such syntax look ? So that I can test if it would fix the warning with my version of protoc and in case I'll want to inject such syntax based on protoc version, from our build scripts (see https://trac.osgeo.org/postgis/ticket/3831)

gertcuykens commented 5 years ago

This should generate the exact same pbf tile as far as I can tell assuming you don't depend on defaults. Hope the next spec is recommending proto3 for its simplicity.

syntax = "proto3";
package pb;

option optimize_for = LITE_RUNTIME;

message Tile {

        // GeomType is described in section 4.3.4 of the specification
        enum GeomType {
                UNKNOWN = 0;
                POINT = 1;
                LINESTRING = 2;
                POLYGON = 3;
        }

        // Variant type encoding
        // The use of values is described in section 4.1 of the specification
        message Value {
                // Exactly one of these values must be present in a valid message
        oneof type {
                string string_value = 1;
                float float_value = 2;
                double double_value = 3;
                int64 int_value = 4;
                uint64 uint_value = 5;
                sint64 sint_value = 6;
                bool bool_value = 7;
        }
        }

        // Features are described in section 4.2 of the specification
        message Feature {
                uint64 id = 1;

                // Tags of this feature are encoded as repeated pairs of
                // integers.
                // A detailed description of tags is located in sections
                // 4.2 and 4.4 of the specification
                repeated uint32 tags = 2;

                // The type of geometry stored in this feature.
                GeomType type = 3;

                // Contains a stream of commands and parameters (vertices).
                // A detailed description on geometry encoding is located in 
                // section 4.3 of the specification.
                repeated uint32 geometry = 4;
        }

        // Layers are described in section 4.1 of the specification
        message Layer {
                // Any compliant implementation must first read the version
                // number encoded in this message and choose the correct
                // implementation for this version number before proceeding to
                // decode other parts of this message.
                uint32 version = 15;

                string name = 1;

                // The actual features in this tile.
                repeated Feature features = 2;

                // Dictionary encoding for keys
                repeated string keys = 3;

                // Dictionary encoding for values
                repeated Value values = 4;

                // Although this is an "optional" field it is required by the specification.
                // See https://github.com/mapbox/vector-tile-spec/issues/47
                uint32 extent = 5;
        }

        repeated Layer layers = 3;
}
strk commented 5 years ago

Yes, we could get rid of the warning with an explicit setting. On the other hand older protoc implementations don't understand that syntax, so to change this would introduce a needless incompatibility.

Do you know what's the first protoc version that does understand the syntax variable ?

joto commented 5 years ago

No. It was years ago when I saw this, so probably not an issue any more. But would be good if somebody checked with, say, old CentOS versions.

bumling commented 5 years ago

This worked for me. Mapbox-gl-js complained using the version above in the comments.

syntax = "proto3";
package vector_tile;

option optimize_for = LITE_RUNTIME;
option php_namespace = "Protobuf";

message Tile {

    enum GeomType {
        UNKNOWN = 0;
        POINT = 1;
        LINESTRING = 2;
        POLYGON = 3;
    }

    message Value {
        string string_value = 1;
        float float_value = 2;
        double double_value = 3;
        int64 int_value = 4;
        uint64 uint_value = 5;
        sint64 sint_value = 6;
        bool bool_value = 7;
    }

    message Feature {
        uint64 id = 1;
        repeated uint32 tags = 2 [ packed = true ];
        GeomType type = 3;
        repeated uint32 geometry = 4 [ packed = true ];
    }

    message Layer {
        uint32 version = 15;
        string name = 1;
        repeated Feature features = 2;
        repeated string keys = 3;
        repeated Value values = 4;
        uint32 extent = 5;
    }

    repeated Layer layers = 3;
}