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 <[EMAIL PROTECTED]> 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 [EMAIL PROTECTED] | >> +-----------------------------------------------------------------+ >> |Chief Technology Officer | >> |DM Solutions Group Inc http://www.dmsolutions.ca/ | >> +-----------------------------------------------------------------+ >> >> >> >> >> >> _______________________________________________ >> Dev mailing list >> [email protected] >> http://openlayers.org/mailman/listinfo/dev >> +-----------------------------------------------------------------+ |Paul Spencer [EMAIL PROTECTED] | +-----------------------------------------------------------------+ |Chief Technology Officer | |DM Solutions Group Inc http://www.dmsolutions.ca/ | +-----------------------------------------------------------------+ _______________________________________________ Dev mailing list [email protected] http://openlayers.org/mailman/listinfo/dev
