shawnbot / topogram

Continuous area cartograms with d3 and TopoJSON
MIT License
326 stars 131 forks source link

Figure out a more elegant way to link projected arc and feature coordinates #1

Closed shawnbot closed 11 years ago

shawnbot commented 11 years ago

The part of the cartogram() function that I'm most uncomfortable with is where I insert projected arc coordinate references into each of the projected GeoJSON feature geometries. This is a performance optimization that allows me to just iterate over the arc coordinates in the main loop and avoid having to modify the feature geometries each time. It's done with a dictionary in which each coordinate is hashed as its string representation (because the native String() is much faster than anything I could cook up), but it still feels like a hack. And though I haven't done any in-depth profiling yet, I'm pretty sure that this is one of the biggest time sinks in the whole process.

I couldn't think of any other way to do it, though, without ripping the guts out of TopoJSON's unpacking code and re-implementing it to produce an intermediary data structure. I'm wondering if this is a use case that TopoJSON should support. The intermediary structure would be like a topology object, but contain projected arc coordinates and references to those in the feature geometries.

@mbostock or @jasondavies, thoughts?

jasondavies commented 11 years ago

See #2. The issue is that your projected arcs are no longer delta-encoded. So when reconstructing them you need to ignore this.

jasondavies commented 11 years ago

@mbostock, perhaps TopoJSON should allow object reconstruction for non-delta-encoded arcs, e.g. when there is no "transform" property?

shawnbot commented 11 years ago

Yep, this would do it. I almost just copied the code from topojson and left out the delta encoding.