Ticket #1207 (closed bug: fixed)

Opened 5 years ago

Last modified 5 years ago

map.updateSize causes all WMS Untiled baselayers to load images from WMS

Reported by: bartvde Owned by:
Priority: major Milestone: 2.6 Release
Component: general Version: SVN
Keywords: Cc:
State: Complete

Description

we've encountered a potential problem with updateSize in OL 2.5.

The updateSize function causes all invisible baselayers to request an image from their WMS service. At least for WMS untiled, I haven't checked other types.

I've created an example which can be used to reproduce, the easiest is to watch the Net console of Firebug. 

<html xmlns="http://www.w3.org/1999/xhtml">
<head>
<style type="text/css">
#map {
width: 400px;
height: 500px;
border: 1px solid black;
}
</style>
<script src="../lib/OpenLayers.js"></script>
<script type="text/javascript">
var map;

function resize() {
document.getElementById('map').style.width = '600px';
document.getElementById('map').style.height = '800px';
map.updateSize();
}

function init(){
OpenLayers.DOTS_PER_INCH = 90.714;
map = new OpenLayers.Map( 'map', { 'projection': 'EPSG:28992',
'units':'m', 'maxExtent': new
OpenLayers.Bounds(0,300000,300000,600000),'scales': [2000000, 1500000,
1000000, 750000, 500000, 375000, 250000, 100000, 50000, 25000, 10000, 5000,
2500, 1000]});

var layer = new OpenLayers.Layer.WMS( "Provincies",
"http://www.rijkswaterstaat.nl/services/geoservices/overzichtskaartnl?",
{layers: 'NL-prov,bebouwing', 'format':'image/gif', 'transparent':'TRUE'},
{'ratio':1, singleTile: true, isBaseLayer: true} );
map.addLayer(layer);

var layer = new OpenLayers.Layer.WMS( "Dijkringgebieden",
"http://www.rijkswaterstaat.nl/services/geoservices/basispakket/grenzen?",
{layers: 'AAA205', 'format':'image/gif', 'transparent':'TRUE'}, {'ratio':1,
singleTile: true, isBaseLayer: true, visibility: false} );
map.addLayer(layer);
if (!map.getCenter()) map.zoomToMaxExtent();

map.addControl( new OpenLayers.Control.LayerSwitcher() );
}
</script>
</head>
<a href="javascript:resize()">Resize</a>
<body onload="init()">
<div id="map"></div>
</body>
</html>

Attachments

ticket1207.patch Download (0.5 KB) - added by bartvde 5 years ago.
Patch fixing this issue
resize.html Download (1.5 KB) - added by bartvde 5 years ago.
example to reproduce
patch-1207-r5925-A0.diff Download (382 bytes) - added by elemoine 5 years ago.

Change History

Changed 5 years ago by bartvde

Patch fixing this issue

  Changed 5 years ago by elemoine

Can't we do the visibility check at the layer level as opposed to the tile level (to do 1 time instead on n)? I should definitely look more closely into the issue but that's the thing popping up into my mind when I look at the patch.

  Changed 5 years ago by crschmidt

I think that this should actually be even more different than that: it should be happening at the map level. We shouldn't be calling moveTo on layers that aren't visible, and layers that aren't called moveTo on shouldn't be drawing anything...

I think the patch in question is definitely just going to hide the situation (but is a fine hack for you if you need something to work for now.)

  Changed 5 years ago by elemoine

  • state set to Needs More Work
  • version changed from 2.5 to SVN

crschmidt, Pierre's patch for #937 actually does what you're suggesting - have setCenter not call moveTo on invisible layer. I'm currently working on Pierre's patch for #937. So let's wait to see what happens with #937 before tackling that ticket.

  Changed 5 years ago by crschmidt

Well, I thought we already didn't call moveTo on invisible layers -- only on ones which had *just* changed from one state to the other. For updateSize, this isn't the case, so I'm not 100% convinced that your point is salient -- but I'm happy to wait on 937 either way.

follow-up: ↓ 7   Changed 5 years ago by bartvde

  • milestone set to 2.6 Release

Using this morning's trunk (which has the fix for ticket:937) this is still happening. On updateSize, all WMS layers (even the invisible ones) load (whether they are a baselayer or not).

  Changed 5 years ago by bartvde

Okay, looking at this again. Seeing then above comments, my first try was to eliminate this at the Map level, something like: 

                if (this.layers[i].visibility) {
                    this.layers[i].onMapResize();                
                }

Unfortunately this won't work if you set a layer to visible first, then change it to invisible, and do the resize. The grid will be incorrectly after the draw.

Next level is Grid.js, and there the check works as follows: 

    onMapResize: function() {
      if (this.singleTile) {
        this.clearGrid();
        this.setTileSize();
        if (this.visibility) {
            this.initSingleTile(this.map.getExtent());
        }
      }

I'll update the patch once there is consensus over the approach. We could also do the visibility check in the initSingleTile function itself.

Thoughts?

in reply to: ↑ 5 ; follow-up: ↓ 10   Changed 5 years ago by elemoine

Replying to bartvde:

Using this morning's trunk (which has the fix for ticket:937) this is still happening. On updateSize, all WMS layers (even the invisible ones) load (whether they are a baselayer or not).

Does the problem only occur for singleTile Grid layers, as you mentioned it in the ticket summary?

follow-up: ↓ 9   Changed 5 years ago by elemoine

Bart,

Sorry for being a bit slow-minded (hmm... is this english?) but I don't understand why your first proposition wouldn't work. To me if you change a layer visibility to false and then do the resize, the visibility check will fail and onMapResize() won't be called for that layer. What am I missing?

Map.setCenter() itself checks for the layer visibility, so I'd rather keep this check in the Map class if we can.

in reply to: ↑ 8   Changed 5 years ago by bartvde

Replying to elemoine:

Bart, Sorry for being a bit slow-minded (hmm... is this english?) but I don't understand why your first proposition wouldn't work. To me if you change a layer visibility to false and then do the resize, the visibility check will fail and onMapResize() won't be called for that layer. What am I missing? Map.setCenter() itself checks for the layer visibility, so I'd rather keep this check in the Map class if we can.

Hi Eric, the problem with that solution is that the grid/tile will have the wrong dimension, i.e. the layer is drawn at its old size.

in reply to: ↑ 7   Changed 5 years ago by bartvde

Replying to elemoine:

Replying to bartvde:

Using this morning's trunk (which has the fix for ticket:937) this is still happening. On updateSize, all WMS layers (even the invisible ones) load (whether they are a baselayer or not).

Does the problem only occur for singleTile Grid layers, as you mentioned it in the ticket summary?

Hi Eric, indeed it only occurs with singleTile Grid layers.

Changed 5 years ago by bartvde

example to reproduce

  Changed 5 years ago by crschmidt

The problem with onMapResize not being called (or not doing anything) when the layer is not visible, but then needing to do something when the layer is visible again, is a problem we also have to solve for the Google/EventPane layers. It's possible that the right solution is a slightly smarter version of the first fix here, which sets a 'resized_but_nothing_happened' flag which is then taken care of the next time moveTo *is* called (which means the layer is visible again).

Changed 5 years ago by elemoine

follow-up: ↓ 14   Changed 5 years ago by elemoine

Bart, Chris,

How about not drawing anything in onMapResize() as the drawing part will be done as part of the setCenter() call in updateSize() if the map has a base layer?

See the attached (untested) patch.

  Changed 5 years ago by elemoine

The patch seems to work with Bart's example. Thanks a lot for making this example available Bart.

in reply to: ↑ 12 ; follow-up: ↓ 15   Changed 5 years ago by bartvde

Replying to elemoine:

Bart, Chris, How about not drawing anything in onMapResize() as the drawing part will be done as part of the setCenter() call in updateSize() if the map has a base layer? See the attached (untested) patch.

This approach makes sense to me.

in reply to: ↑ 14   Changed 5 years ago by elemoine

Replying to bartvde:

Replying to elemoine:

Bart, Chris, How about not drawing anything in onMapResize() as the drawing part will be done as part of the setCenter() call in updateSize() if the map has a base layer? See the attached (untested) patch.

This approach makes sense to me.

That approach works well with Grid layers but is probably not applicable to the commercial layers. This makes me think that the right approach is Chris'.

  Changed 5 years ago by crschmidt

  • state changed from Needs More Work to Commit

I think my solution was wrong. Unfortunately, Google's issue is entirely different and unrelated. (It happens even when the map doesn't actually change sizes, which means that onMapResize fixes don't help.)

I'm going to go with this solution. Thanks for the patch, and report, Bart!

  Changed 5 years ago by crschmidt

  • status changed from new to closed
  • state changed from Commit to Complete
  • resolution set to fixed

Committed with r6168, thx to all!

Note: See TracTickets for help on using tickets.