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

Fix Uncaught RangeError in canvas tile renderer #2602

Merged
merged 2 commits into from Aug 25, 2014
Merged

Fix Uncaught RangeError in canvas tile renderer #2602

merged 2 commits into from Aug 25, 2014

Conversation

elemoine
Copy link
Member

This PR aims to fix #2542.

The bug triggers when a tile layer has an extent and when the layer extent and the view extent do not intersect. In that case the canvas tile layer renderer's prepareFrame function calculates tile ranges and canvas width and height values that cause js errors further down in the function.

This PR fixes the problems by making the canvas tile renderer's prepareFrame clear its canvas and immediately return when the layer extent and view extent don't intersect. We could probably do better and avoid clearing the canvas when that condition happens. I added a FIXME for this.

Please review.

Closes #2542.

@elemoine elemoine added the bug label Aug 22, 2014
@elemoine elemoine added this to the v3.0.0 milestone Aug 22, 2014
@tschaub
Copy link
Member

tschaub commented Aug 23, 2014

Looks good to me.

I liked the idea of returning false from prepareFrame when there was nothing to render for a layer. I imagine that would save more than avoiding the extra clearRect on the already clear context.

@elemoine
Copy link
Member Author

Ok, you've motivated me to give the "prepareFrame returns false" approach another try. So let's not merge this for now. Thanks!

ol.renderer.canvas.TileLayer#prepareFrame immediately returns false if the layer extent and the view extent do not intersect.
@elemoine
Copy link
Member Author

New patch attached.

@tschaub
Copy link
Member

tschaub commented Aug 25, 2014

I didn't run the examples, but the code looks good to me. Assuming all works as expected, please merge. Thanks for the extra work on this one. Not sure how many people will work with layers with limited extents, but I like the idea that we don't compose extra blank pixels in that case.

@elemoine
Copy link
Member Author

Not sure how many people will work with layers with limited extents

It visually looks a bit strange when you zoom in and see that the new tiles covers less ground. I wonder if clipping would be more appropriate for the "limited layer extent" use-case.

elemoine pushed a commit that referenced this pull request Aug 25, 2014
Fix Uncaught RangeError in canvas tile renderer
@elemoine elemoine merged commit cd4063b into openlayers:master Aug 25, 2014
@elemoine elemoine deleted the layer-extent branch August 25, 2014 07:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Bug in ol.renderer.canvas.TileLayer.prototype.prepareFrame
2 participants