[OpenLayers-Trac] Re: [OpenLayers] #3419: Tile simplifications and partial rewrite

[OpenLayers]

#3419: Tile simplifications and partial rewrite
----------------------+-----------------------------------------------------
 Reporter:  ahocevar  |       Owner:  ahocevar    
     Type:  feature   |      Status:  new         
 Priority:  minor     |   Milestone:  2.12 Release
Component:  Tile      |     Version:  2.10        
 Keywords:            |       State:  Review      
----------------------+-----------------------------------------------------

Comment(by erilem):

 I love this patch! Thanks Andreas.

 Results of tests with the transition.html example:

  * Open the transition.html example in FF, select one of both tiled layers
 in the layer switcher, you should see image loading icons (on the top-left
 corner) while tiles are loading. IE8 and Chromium aren't affected by this.
 We didn't have this behavior in FF previously, so this is related to new
 code somewhere.

 Code review comments:

  * I'm not a big fan of having the mixin register listeners to events
 triggered by the mixin target class. I think I'd rather define empty
 {{{beforeDraw}}} and {{{afterDraw}}} functions in the {{{Image}}}
 prototype, and have the mixin override these functions. {{{Image}}} and
 {{{ImageMixin}}} are tightly coupled already, so I see no value in
 decoupling them by relying on events. And not relying on events means less
 overhead. If you agree with that, I can create a new patch.
  * We can save bits by changing from {{{return undefined;}}} to
 {{{return;}}}. I think it is safer too, as the application could have set
 {{{undefined}}} to something that isn't undefined. You have two {{{return
 undefined;}}} statements in your patch.
  * In the {{{Image}}} class don't you think we could rename
 {{{this.imgDiv}}} to {{{this.image}}} or {{{this.imgElt}}} as this DOM
 element isn't actually a div.
  * In {{{Image.onImageLoad}}} you define a local variable {{{img}}}, so
 you may want to use in place of {{{this.imgDiv}}} when setting the filter
 for the alpha hack.
  * You call {{{this.layer.getResolution()}}} in three places. To get the
 current resolution I think it's safer to call
 {{{this.layer.map.getResolution()}}}, to get the resolution from the base
 layer.
  * In {{{ImageMixin.initialize}}} you use expressions like {{{singleTile
 && 1}}} for the setting of {{{this.backBufferMode}}}. If {{{singleTile}}}
 and {{{indefOf != -1}}} are both {{{false}}}, then
 {{{this.backBufferMode}}} will be set to {{{false}}}, while it should be
 an integer value. Using {{{(singleTile & 1) | ((indexOf() != -1) & 2))}}}
 would sound better to me. I can add unit tests for this if you agree.

 That's all for now.

-- 
Ticket URL: <http://trac.openlayers.org/ticket/3419#comment:20>
OpenLayers <http://openlayers.org/>
A free AJAX map viewer