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

Jittery text and icons when panning around #7500

Closed
4 tasks done
orestis opened this issue Nov 23, 2017 · 5 comments · Fixed by #7524
Closed
4 tasks done

Jittery text and icons when panning around #7500

orestis opened this issue Nov 23, 2017 · 5 comments · Fixed by #7524

Comments

@orestis
Copy link

orestis commented Nov 23, 2017

I am drawing text from GeoJSON, going from ol.source.Vector to ol.layer.Vector via a styling function that sets ol.style.Text.

When dragging around slowly with the mouse, but also visible during Kinetic animations, the text is jittery, as it moves around the various pixel boundaries. This is visible also in the example, if you pan around slowly. I've tested this with Safari, Chrome and Firefox on macOS.

A similar thing happens with icons that are not using snapToPixel: false.

A few possible fixes that would fix this jittery movement while keeping everything crisp would be:

  1. Ensure the Kinetic Animations only do whole pixel movements.
  2. During Kinetic Animation, disable pixel snapping globally. Re-enable it when the animation finishes and re-render to the pixel grid.

Thanks!

  • I am submitting a bug or feature request, not a usage question. Go to https://stackoverflow.com/questions/tagged/openlayers for questions.
  • I have searched GitHub to see if a similar bug or feature request has already been reported.
  • I have verified that the issue is present in the latest version of OpenLayers (see 'LATEST' on https://openlayers.org/).
  • If reporting a bug, I have created a CodePen or prepared a stack trace (using the latest version and unminified code, so e.g. ol-debug.js, not ol.js) that shows the issue.
@ahocevar
Copy link
Member

Your suggested option 1 (ensure that kinetic animations only do whole pixel movements) makes sense to me. Would you be able and willing to create a pull request?

@orestis
Copy link
Author

orestis commented Nov 24, 2017

Probably able, definitely willing. I will look into this next week.

@orestis
Copy link
Author

orestis commented Nov 27, 2017

I've done some investigation on this. It turns out that the Kinetic calculations do not work the way I expected; I had no idea of the inner workings of OpenLayers.

To my understanding, the call chain goes somewhat like:

  • DragPan animation finishes
  • Kinetic is used to calculate target center, in world coordinates (NOT in pixels).
  • A view animation is created and applied
  • Rendering happens at every frame as the animation progresses

So it seems my original suggestion of having Kinetic supply whole pixel values is impossible with the current architecture.

Seeing that the various renderers have no direct connection to the View, does it make sense for my user code to somehow walk through all my text instances at the start of a Kinetic animation, set snapToPixel to false, then re-enable (and re-render) at the end of the animation? There is still work to be done there to figure out what hooks can be used for this, and if new hooks/events should be added, but it should be a simpler change (for me) than trying to tweak the rendering pipeline.

BTW I have traced the lack of snapToPixel support for text styles in ol.render.canvas.TextReplay.prototype.drawTextImage_; a hard coded true at the point where the instructions are pushed, 14th element. Changing this false indeed makes the text non-pixel snappy and blurry. I will make an issue to discuss adding this.

@ahocevar
Copy link
Member

@orestis Wouldn't it be enough to use Math.round() for the center coordinates in

center: center.slice(),
?

@ahocevar
Copy link
Member

@orestis, can you please check if #7524 fixes the issue for you?

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

Successfully merging a pull request may close this issue.

2 participants