[OpenLayers-Dev] unnecessary method invocations ...

[Paul Spencer]

Okay, I finally realized where I went wrong about this.  I misread a  
line of code ...  I now grok what you were intending and withdraw my  
objection to using removeExcessTiles :)

However, your suggestion to revisit whether or not we should support  
switching from tiled to singleTile is still a valid point that we  
should discuss.  Chris indicated this afternoon that he's not sure we  
should support that as a use case.  Now that I understand the code a  
bit better, I am ambivalent about this particular use case.

Looking forward to hacking with you tomorrow

Cheers

Paul

On 27-Sep-07, at 7:36 AM, Paul Spencer wrote:

> 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/ |
> +-----------------------------------------------------------------+
>
>
>
>
>

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