specialized layer for a grid of points

[tschaub]

The attached patch adds a PointGrid layer for dynamically generating a grid of features with point geometries. The patch includes examples that demonstrate how grid origin, rotation, and maxFeatures can be changed and how the layer can be used with snapping. Tests pass and examples work in Chrome 12 and IE 6.

Thanks for any review.

[bartvde]

Reviewing this one now.

[bartvde]

Very nice work Tim with great examples, a few questions/remarks wrt lib/OpenLayers/Layer/PointGrid.js, mostly wrt the docs:

• why do we add fromBounds if we already have OpenLayers.Bounds.toGeometry? I personally don't see a need for both.
• why is origin initialized to 0 and not to null? From the docs it's not entirely clear to me where the origin is located (bottom-left)?
• maxFeatures and setMaxFeatures: is maxFeatures not an {Integer} always, so why use {Number} in docs?
• make more explicit in docs that dx and dy are in map units and not in pixel units?
• in getOrigin shouldn't we protect against getExtent returning null?
• updateGrid misses docs for force
• make it more explicit in the docs that rotation is negative in the clockwise direction

Other than this it's good to go.

[tschaub]

Replying to bartvde:

* why do we add fromBounds if we already have OpenLayers.Bounds.toGeometry? I personally don't see a need for both.

Can you believe I forgot about Bounds.toGeometry (r5625 was a long time ago)? Actually, I think toGeometry should be deprecated and fromBounds should be added, but we don't need the bloat at this point. It makes more sense that LonLat and Bounds (lower level) don't know about Geometry (higher level). Whenever we get around to 3.0, this should be reworked. For now, I'll rework the patch to use toGeometry.

* why is origin initialized to 0 and not to null? From the docs it's not entirely clear to me where the origin is located (bottom-left)?

I'll update the initial value to be null. The grid is infinite, so there is no bottom/left etc. The grid lattice will always be aligned with the origin. I'll update the docs to clarify this. Let me know if you think there is a better way to describe it (we could say (Math.sqrt(Math.pow(xp-xo, 2) + Math.pow(yp-yo, 2)) * Math.cos(Math.atan2(yp-yo, xp-xo) - r)) % dx === 0 and similarly for dy for any point (xp, yp) rotation (r) and origin (xo, yo)).

* maxFeatures and setMaxFeatures: is maxFeatures not an {Integer} always, so why use {Number} in docs?

I go back and forth on this one. There's nothing restricting this value to an integer, and we only have Number in js anyway.

* make more explicit in docs that dx and dy are in map units and not in pixel units?

Good point, will do.

* in getOrigin shouldn't we protect against getExtent returning null?

Internally, getOrigin is only called on map moveend, so at that point we should have an extent. If someone calls setSpacing or other before adding the layer to a map, things will fail. I'll add docs to clarify this.

* updateGrid misses docs for force

Added, thanks.

* make it more explicit in the docs that rotation is negative in the clockwise direction

Clarified this.

Other than this it's good to go.

Thanks for the careful review.

[tschaub]

dynamic point grid

[tschaub]

Updated patch based on review.

[tschaub]

(In [12099]) Adding layer for generating dynamic point grids. r=bartvde (closes #3344)

[bartvde]

Replying to tschaub:

Replying to bartvde:

* why do we add fromBounds if we already have OpenLayers.Bounds.toGeometry? I personally don't see a need for both.

Can you believe I forgot about Bounds.toGeometry (r5625 was a long time ago)? Actually, I think toGeometry should be deprecated and fromBounds should be added, but we don't need the bloat at this point. It makes more sense that LonLat and Bounds (lower level) don't know about Geometry (higher level). Whenever we get around to 3.0, this should be reworked. For now, I'll rework the patch to use toGeometry.

Agreed, I've opened up #3369 so that we don't forget about this whenever it comes to 3.0.