Apply pixelRatio to line dash #6091

merged 3 commits into from Dec 6, 2016


None yet

3 participants


Fixes #6090

@@ -525,7 +525,10 @@ ol.render.canvas.Replay.prototype.replay_ = function(
context.lineJoin = /** @type {string} */ (instruction[4]);
context.miterLimit = /** @type {number} */ (instruction[5]);
if (ol.has.CANVAS_LINE_DASH) {
- context.setLineDash(/** @type {Array.<number>} */ (instruction[6]));
+ var lineDash = instruction[6].map(function(dash) {
ahocevar Nov 8, 2016 Member

Since this operation is on the hot path, it would be better to reuse the modified lineDash, instead of creating a new array every time. Also, there is no need to modify the lineDash array when the pixel ratio is 1.


@ahocevar I don't see how I should a reuse the previous line dash array.

ahocevar commented Nov 8, 2016

@tchandelle The easiest would be to add the pixel ratio as another instruction with SET_STROKE_STYLE. It would initially be 1. When replaying at a different pixel ratio, you'd store the modified lineDash in the instruction set, and update the pixel ratio. So when creating the replay:

        strokeStyle, lineWidth, lineCap, lineJoin, miterLimit, lineDash, true, 1

And when replaying:

var lineDash = /** @type {Array.<number>} */ (instruction[6]);
var renderedPixelRatio = instruction[8];
if (usePixelRatio && pixelRatio !== renderedPixelRatio) {
  lineDash = {
    return dash * pixelRatio / renderedPixelRatio;
  instruction[6] = lineDash;
  instruction[8] = pixelRatio;

This looks good already. As always, tests for this would be much appreciated. Do you think you can either add a renderer test (verifying the correct result) or a unit test (verifying the correct line dash and pixel ratio in the instruction)?


@ahocevar : I've added both tests. I'm not sure if I've implemented the unit test the correct way.

tchandelle added some commits Nov 8, 2016
@tchandelle tchandelle Apply pixelRatio to line dash a0e3107
@tchandelle tchandelle Compare current linedash and new linedash as arrays 7ee04ec
@tchandelle tchandelle Add tests for linedash for HiDPI display
fredj approved these changes Dec 6, 2016 View changes
fredj commented Dec 6, 2016

@ahocevar do you want to take a final look?


Looks good to me as well. Thanks @tchandelle

@ahocevar ahocevar merged commit 89ebf0a into openlayers:master Dec 6, 2016

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
coverage/coveralls Coverage increased (+0.0008%) to 86.601%
@tchandelle tchandelle deleted the tchandelle:linedash branch Dec 7, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment