Fixed zoom levels review

[tschaub]

Can someone (crshmidt or euzuro) shed a little insight on MIN_ZOOM_LEVEL, MAX_ZOOM_LEVEL, and RESOLUTIONS on layers with fixed zoom levels? Is there any reason to have the min/max constants? Seems only necessary if there is a commercial api we want to support that doesn't use zero based zoom. If possible, I like the idea of just having RESOLUTIONS.

Thanks for any insight.

[elemoine]

Backing Tim here. I've also audited this code and have found no obvious need for MIN_ZOOM_LEVEL and MAX_ZOOM_LEVEL. Moveover, I saw people on the list that set these variables instead of maxZoomLevel and minZoomLevel.

[euzuro]

sounds like this should be addressed.

[euzuro]

Hello dearest fans of the MIN_ and MAX_ constants.

Unfortunately, I don't think that we can get rid of these constants right now.

The reason for their existence is because (in theory at least) we don't always know what the resolutions (zoom levels) look like for our third party layers.

It happens that right now, we have hammered in RESOLUTIONS arrays for all of our third party layers, but there may come a day in which we add a new one for which we *don't* know all the resolutions/scales.

The reason for the MIN_ZOOM_LEVEL and MAX_ZOOM_LEVEL constants are for explicitly telling OpenLayers what the boundaries are for the specific 3rd party's API. As you mention above, some APIs do not accept a zoom level '0', so we have to deal with that (MIN_ZOOM_LEVEL).

The MAX_ZOOM_LEVEL, then, is used in order to determine the total number of zoom levels that are available for use in the layer.

Now, yes, in the case where a RESOLUTIONS array is hard-coded into the layer, we *could* just use the length of that array instead of making the calculation based on the MAX_ZOOM_LEVEL.

That change wouldn't, however, take care of the case mentioned above where we do not know the resolutions array beforehand.

Note that VE is one of the APIs that does not 'do' zoom level 0.

[euzuro]

The patch that I have attached makes the whole MIN_, MAX_, min, max zoom levels and num zoom levels logic more clear.

It also deals with the fact that the map *always* has a 'numZoomLevels' value set by default (16).... allowing the setting of 'maxZoomLevel' directly on the layer to override the map's default 'numZoomLevels' value.

[euzuro]

I will run tests on this to make sure all is square, then I will add some more info to the bottom of the SettingZoomLevels wiki. Finally, I will take a look into making one more addition to the patch... perhaps something along the lines of the 'alwaysInRange' property we added with #987.

...seems like for one of these layers, if the user doesn't specify a 'minZoomLevel' or 'maxZoomLevel' or 'numZoomLevels', that is probably an indication that s/he wants them all... and we might ought to make our code behave that way.

Thoughts appreciated. Patch forthcoming tomorrow morning or afternoon.

[euzuro]

OK. Original patch passes all tests ff2 and ie7.

See: SettingZoomLevels#a3rdPartyLayers for updated info on how it all works, including many examples with explanations.

Finally, regarding the idea of an 'alwaysInRange' sort of modification to the patch... I thought it over some more and just really don't think it's quite right. The correct way to solve this problem is by removing the default value of 16 from the map object and moving it down to Layer.WMS or wherever it is that it actually belongs. Clearly 16 is *not* a reasonable default for all of the myriad layers we now have in OpenLayers library.

Changing it, however, would be a clear API break and I think it has a high likelihood of breaking existing applications build on OL. I have made a lengthy remark to this effect in the code, flagged with the '3.0' keyword so we will be sure to keep this in mind for the rewrite. (said patch with added comment to be added immediately following this comment)

AFAIC, this ticket/patch is ready for a review.

[euzuro]

Added comment regarding proposed changes in 3.0. NO FUNCTIONAL CHANGE

[euzuro]

Please review.

Note to reviewer: I recommend actually downloading and applying the patch in order to make any sense of it. The mere colored trac diff I think does not really clarify things. To get a real sense of the change, you really want to look at the FixedZoomLevels.js file before and after the patch is applied. The only function modified is initResolutions()

[euzuro]

chris, if you can take a look at these two, i would appreciate

[crschmidt]

Okay. Patch is clearly explainedin comments, fixes old regression in behavior, and it's clear why it was a problem. Erik, please commit.

[euzuro]

(In [7948]) fix a somewhat broken FixedZoomLevels.js functionality -- the 'maxZoomLevels' property if set on a layer was not getting processed correctly. This patch fixes that issue, as well as adding a cole-slaw-sized blerb of comments explaining the rules of precedence wrt resolution setting for 3rd party layers (those who subclass FixedZoomLevels). Thanks to tschaub for poking me to clarify this previously messy territory, and for cr5 for kindly taking the time to review my patch. (Closes #1182)