mapbox / vector-tile-spec

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

proto 3 #139

Closed gertcuykens closed 5 years ago

gertcuykens commented 5 years ago

https://github.com/mapbox/vector-tile-spec/issues/90

joto commented 5 years ago

Not sure what your patch has to do with #90. That issue was about protobuf 3 vs. protobuf 2. Your patch merges the vt3 spec into master which isn't finalized and therefore lives in a branch.

gertcuykens commented 5 years ago

Didn't know there was a other branch, its related with that issue because I believe this is how the current spec looks like in proto3 and is I think compatible with current tiles. So basically I didn't change the spec only the proto and put it in v3 because it's still a significant change for applications using the proto file

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;
}
joto commented 5 years ago

I am seeing something different when I look into the changes this patch makes.

gertcuykens commented 5 years ago

Most is indeed just a copy paste from v2.1 into a v3.0 folder because spec itself didn't require change but if you scroll all the way down you should see the new proto3 file?

https://github.com/mapbox/vector-tile-spec/pull/139/files

image

joto commented 5 years ago

It seems we are still talking about different things. Version 3 of the vector tile spec has nothing to do with protobuf2 or 3. Please don't try to put these things together. The patch doesn't make any sense as it is. We have a version 3 of the vector tile spec in the making.

gertcuykens commented 5 years ago

It was indeed based on the fact that I assumed there was no v3 in the making. That said I don't think it can be added as a intermediate v2.2 change either because it's technically backward compatible but in reality allot of assumptions are made, like default values. And that dropping in the new proto file requires code change for creating basically the same pbf tiles.