[OpenLayers-Dev] unnecessary method invocations ...

[Paul Spencer]

Thanks for the feedback Erik :)  I'm not worried about getting this  
stuff into 2.5, since it is already awesome.  These are just minor  
tweaks.

I would like to spend a couple of minutes talking to you about first  
question if you have some time today.  I am not sure I quite  
understand how calling removeExcessTiles will do anything after you  
have changed the grid array to have only one tile in it.  It seems  
that you should call it *before* resetting the array to facilitate  
switching from tiled to single, otherwise you will end up with a  
bunch of tiles that have nothing pointing to them.

Cheers

Paul

On 26-Sep-07, at 11:03 PM, Erik Uzureau wrote:

> Hi Paul what a great email from you -- it's cool to know that  
> someone else
> is looking through the guts of this stuff. Your comments are very  
> welcome.
> i am responding inline below....
>
> On 9/26/07, Paul Spencer <pspencer at dmsolutions.ca> wrote:
>> I'm starting to look at the impact of modifying rendering in
>> singleTile layers to not clear the existing tile until the new tile
>> arrives.  As I try to grok the code related to this, I'm running into
>> a couple of things I think are unnecessary.  My understanding of the
>> code is still perhaps too limited to understand if the following are
>> extraneous or not, but I thought I would throw it out there and see
>> if others can explain why they are necessary:
>>
>> At the end of initSingleTile in Grid.js, there is a call to:
>>
>> this.removeExcessTiles(1,1);
>>
>> This seems unnecessary to me as several lines before, this.grid is
>> set to an empty array and exactly one new tile is added to it.  It
>> does not seem to me that this.grid can contain any more than one tile
>> at the point removeExcessTiles is called and therefore this is pure
>> overhead (not much, I grant).
>
> So I believe the reason I put that in there was for the case where you
> have a gridded layer and you switch it mid-operation to a single- 
> tile layer.
> I vaguely remember thinking at some point that we should support that
> case. Of course, realistically, that's pretty absurd. As you say,  
> though,
> at least at this point, the overhead is pretty minimal.
>
> If we can all agree that switching the 'singleTile' variable mid- 
> run is NOT
> something that we want to support, then I think we could safely remove
> that. I think the best route for that sort of change is a 2.6  
> ticket with the
> simple patch (note that it will probably break tests, so we'll have  
> to update
> that too).
>
>>
>> In Tile.js moveTo, this.clear() is called prior to calling this.draw
>> (), which also calls this.clear().  It seems calling this.clear in
>> moveTo is unnecessary.  This is not quite true because there is a
>> code path involving the redraw variable where this.draw() is not
>> called and so the clear would need to be done in the alternate path.
>> Of course, if you are moving a layer and not redrawing it, should you
>> clear it?
>
> This is a great find and something that I actually squeezed my head
> over quite a bit just last week with a MetaCarta single-tiled  
> application
> that I'm building. Ran into the same exact problem as I'm sure you  
> are, that
> the tile gets cleared, then the request goes out and the map is  
> blank while
> you wait for the new data to load. clearly not ideal.
>
> Chris helped me think this one through a few times and yes I think  
> you're
> totally right. The clear definitely does *not* need to be in there.  
> The clear()
> belongs *immediately* before the actual drawing happens and nowhere
> else.
>
> The case you bring up about the 'redraw' option is a good one too  
> and but
> it turns out that's only used to facilitate the spiralTileLoad()
> function.... which
> is to say that the tiles which are moved but not drawn are almost  
> all drawn
> after all tiles have been moved.
>
> I meant to make a ticket for this but then ended up finding a
> different workaround
> in my application and forgot to get to it.
>
> I've opened
> http://trac.openlayers.org/ticket/1017
>
> and put a patch in there. Unfortunately, the 2.5 release is pretty
> much on its way out the door, so this will also have to wait for 2.6.
> :-(
>
> Erik
>> Discussion invited :)
>>
>> Cheers
>>
>> Paul
>>
>> +-----------------------------------------------------------------+
>> |Paul Spencer                          pspencer at dmsolutions.ca    |
>> +-----------------------------------------------------------------+
>> |Chief Technology Officer                                         |
>> |DM Solutions Group Inc                http://www.dmsolutions.ca/ |
>> +-----------------------------------------------------------------+
>>
>>
>>
>>
>>
>> _______________________________________________
>> Dev mailing list
>> Dev at openlayers.org
>> http://openlayers.org/mailman/listinfo/dev
>>

+-----------------------------------------------------------------+
|Paul Spencer                          pspencer at dmsolutions.ca    |
+-----------------------------------------------------------------+
|Chief Technology Officer                                         |
|DM Solutions Group Inc                http://www.dmsolutions.ca/ |
+-----------------------------------------------------------------+