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