Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

A tile queue that can be aborted. #179

Merged
merged 9 commits into from
Feb 14, 2012
Merged

Conversation

ahocevar
Copy link
Member

This saves us a significant amount of server requests when changing zoom levels often, and because we use OpenLayers.Animation, setting img.src on a tile should not freeze iOS any more during panning, so we can hopefully get rid of scheduleMoveGriddedTiles.

While at it, I also removed the spiralTileLoad function. Although definitely a masterpiece of coding art, I replaced it with the less elegant but more efficient approach of sorting tiles by distance from the viewport center.

There are no tests yet for the tile queue, and I haven't tested on a real IPad, so I don't know if it really improves performance as compared to scheduleMoveGriddedTiles.

@elemoine
Copy link
Member

This look great work Andreas! One quick question for me to understand: OpenLayers.Animation, and therefore requestAnimationFrame, are used for panTo and kinetic, so the I don't see the direct relation to dragging the map. Also, I don't think requestAnimationFrame is available on iOS (am not sure about iOS 5 actually) so the new OpenLayers.Animation code should not change anything on iOS.

@ahocevar
Copy link
Member Author

Also browsers that do not support requestAnimationFrame benefit from this. Not all img.src are set at the same time, so when you change zoom levels quickly, the queue is cleared and we don't send out reqiests for all tiles.

Regarding panning: mouse dragging gets more efficient with this change. On IPad, setting img.src freezes the browser, and now we don't do multiple set img.src operations at the same time.

@fredj
Copy link
Member

fredj commented Jan 26, 2012

Tested with a customer project using Chrome, FF and IE8: works like a charm

ahocevar and others added 5 commits January 28, 2012 15:30
This saves server requests, and because we use OpenLayers.Animation, setting img.src on a tile should not freeze iOS any more, so we can hopefully get rid of scheduleMoveGriddedTiles.
Saves 3 unnecessary instances creation and 6 parseFloat calls.
Since draw is the only tile operation that we defer, the tile queue can be an array of tiles and queue handling can be simplified. We now use the beforedraw event to defer drawing, and remove all occurrences of a tile from the tile queue when we draw it.

Instead of layers that want to defer tile drawing having to override the tile's draw method, layers can now abort drawing by returning false from a beforedraw listener, and later call draw(true) to draw the tile directly, without clearing it first.
@ahocevar
Copy link
Member Author

@elemoine reported decreased performance of the mobile.html example on iOS. With 720c49c, we should get back the previous performance on iOS, with smoother kinetic dragging on devices that support native requestAnimationFrame, and more instant tile loading on all devices.

Only use it if no native requestAnimationFrame function is available. This should improve performance on mobile devices.
This results in a smaller queue that we don't have to unqueue from.
@ahocevar
Copy link
Member Author

ahocevar commented Feb 3, 2012

Live examples updated. Try e.g. http://ahocevar.github.com/openlayers/tile-queue/examples/mobile.html

@ahocevar
Copy link
Member Author

@elemoine's concerns about performance on iOS have been addressed, and browsers that support requestAnimationFrame can take full advantage of this new feature. Anybody willing to give a "go" on this?

@elemoine
Copy link
Member

@ahocevar I just tested the mobile.html example on the iPhone 4S. The experience is good, as good as with http://openlayers.org/dev/examples/mobile.html. I'll try with the iPad we have in the office tomorrow (if I manage to shut Angry Birds down :). Thanks for your effort on this, this a great patch.

@fredj
Copy link
Member

fredj commented Feb 13, 2012

Tested in Android 4.0.2 (default browser and Chrome Beta)

@elemoine
Copy link
Member

@ahocevar, @tonio and I just tested your example in an iPad 1/iOS 4.3. We feel the experience might be a bit better with the example on openlayers.org, but this is hard to tell, and surely not significant.

@ahocevar
Copy link
Member Author

@elemoine, @tonio Thanks for testing. If you could spare a few more minutes, it would be great to get a code review so we can merge this.

@elemoine
Copy link
Member

@ahocevar I'm planning to do that tonight.

@elemoine
Copy link
Member

Excellent patch @ahocevar. Please merge.

@ahocevar
Copy link
Member Author

Thanks a lot @elemoine for taking the time to test, your feedback and the review!

ahocevar added a commit that referenced this pull request Feb 14, 2012
@ahocevar ahocevar merged commit 5e734f2 into openlayers:master Feb 14, 2012
@ahocevar
Copy link
Member Author

Thanks @fredj and @tonio for the testing as well!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants