move grid_renderer feature keys to counter (or rendering buffer to 32bit ints) #1196

Closed
springmeyer opened this Issue Apr 27, 2012 · 7 comments

Comments

Projects
None yet
3 participants
@springmeyer
Member

springmeyer commented Apr 27, 2012

Early revisions of the grid_renderer used an incrementing counter for each feature processed, and burned that integer into the grid buffer during rendering. During encoding these keys were re-mapped back to the feature's actual id.

After #753 was finished for all key datasources, I moved to burning the feature.id() directly into the rendering buffer because they were now promised to be unique and because it simplified the encoding boilerplate.

This was short-sided because we need to support at least 32 bit integers (and in the future perhaps 64 bit) for both feature.ids(). Currently now its possible for feature ids to overflow when pushed into the unsigned short rendering buffer which is limited to 65535 and only positive numbers.

@springmeyer

This comment has been minimized.

Show comment
Hide comment
@springmeyer

springmeyer Jun 6, 2012

Member

note to anyone watching - yes, we need to consider moving to 64 bit integers, but since feature.id() is 32 that was my target for now.

Member

springmeyer commented Jun 6, 2012

note to anyone watching - yes, we need to consider moving to 64 bit integers, but since feature.id() is 32 that was my target for now.

@strk

This comment has been minimized.

Show comment
Hide comment
@strk

strk Jul 17, 2012

Contributor

was this backported to the 2.0 branch ?

Contributor

strk commented Jul 17, 2012

was this backported to the 2.0 branch ?

@springmeyer

This comment has been minimized.

Show comment
Hide comment
Member

springmeyer commented Jul 17, 2012

no.

@cgendreau

This comment has been minimized.

Show comment
Hide comment
@cgendreau

cgendreau Jul 25, 2012

On mapnik 2.0, could I reduce the impact of the issue if I reduce the density of overlapping points?

On mapnik 2.0, could I reduce the impact of the issue if I reduce the density of overlapping points?

@springmeyer

This comment has been minimized.

Show comment
Hide comment
@springmeyer

springmeyer Jul 25, 2012

Member

@cgendreau - no, the only workaround is to ensure that your feature ids are less than 65,535.

Member

springmeyer commented Jul 25, 2012

@cgendreau - no, the only workaround is to ensure that your feature ids are less than 65,535.

@cgendreau

This comment has been minimized.

Show comment
Hide comment
@cgendreau

cgendreau Jul 25, 2012

@springmeyer You mean a maximum of 65,535 features or 65,535 as the maximum value of a feature?

@springmeyer You mean a maximum of 65,535 features or 65,535 as the maximum value of a feature?

@springmeyer

This comment has been minimized.

Show comment
Hide comment
@springmeyer

springmeyer Jul 25, 2012

Member

It depends. Most directly I mean the maximum value of the feature id. In the case of the the Mapnik postgis plugin (which I think is what you are using) by default the feature.id() is incremented per feature - so that would mean the overflow could be avoided by rendering less than 65k features per tile. The postgis plugin has a key_field option which allows the user to control which value is used for the feature.id() and in that case overflow would only occur based on that fields value.

Member

springmeyer commented Jul 25, 2012

It depends. Most directly I mean the maximum value of the feature id. In the case of the the Mapnik postgis plugin (which I think is what you are using) by default the feature.id() is incremented per feature - so that would mean the overflow could be avoided by rendering less than 65k features per tile. The postgis plugin has a key_field option which allows the user to control which value is used for the feature.id() and in that case overflow would only occur based on that fields value.

@cgendreau cgendreau referenced this issue in Canadensys/canadensys-explorer Nov 15, 2012

Closed

Problem with the interactivity on the map #1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment