Added pixelRatio option to override canvas upscaling. Resolves #876, test included. #877
Conversation
Falls back to old behavior if not specified. Added to options reference, Added to Dygraph.prototype.resizeElements_, Added to rangeSelector.prototype.updateVisibility.
First fails without change, then passes after change.
|
Just checking, any issues with merging this pull request? |
src/dygraph-options-reference.js
Outdated
| "pixelRatio": { | ||
| "default": "(devicePixelRatio / context.backingStoreRatio)", | ||
| "labels": ["Overall display"], | ||
| "type": "integer", |
There was a problem hiding this comment.
Does this have to be an integer? Can it be a float?
There was a problem hiding this comment.
Yep, good idea, since devicePixelRatio can often be a non-whole number for different zoom levels and certain phones.
src/dygraph-options-reference.js
Outdated
| "default": "(devicePixelRatio / context.backingStoreRatio)", | ||
| "labels": ["Overall display"], | ||
| "type": "integer", | ||
| "description": "Overrides the pixel ratio scaling factor for the canvas's 3d context. Ordinarily, this is set to the devicePixelRatio / (context.backingStoreRatio || 1), so on mobile devices, where the devicePixelRatio can be somewhere around 3, performance can be improved by overriding this value to something less precise, like 1." |
src/dygraph-options-reference.js
Outdated
| "default": "(devicePixelRatio / context.backingStoreRatio)", | ||
| "labels": ["Overall display"], | ||
| "type": "integer", | ||
| "description": "Overrides the pixel ratio scaling factor for the canvas's 3d context. Ordinarily, this is set to the devicePixelRatio / (context.backingStoreRatio || 1), so on mobile devices, where the devicePixelRatio can be somewhere around 3, performance can be improved by overriding this value to something less precise, like 1." |
There was a problem hiding this comment.
add a caveat here, something along the lines of "performance can be improved at the expense of resolution"
src/dygraph.js
Outdated
| var canvasScale = utils.getContextPixelRatio(this.canvas_ctx_); | ||
| var pixelRatioOption = this.getNumericOption('pixelRatio') | ||
|
|
||
| var canvasScale = pixelRatioOption > 0 |
There was a problem hiding this comment.
Any reason not to do pixelRatioOption || utils.getContextPixelRatio(this.canvas_ctx_)?
There was a problem hiding this comment.
Probably trying to make my linter happier with 80 char lines, I switched it back here and in range-selector.js, it does read more naturally.
Fix typo in docs, documented a float type, Simplified method of reading option.
|
Hi, just wondering if you can merge in this pull request? I couldn't find any other changes asked for. |
|
Thanks! |
Let me know about any desired changes or adjustments. I hand tested the range-selector gallery example with
pixelRatio: 3, and it seems to behave the same and properly force scaling; the implementation followed a definitive pattern of overriding utils.getContextPixelRatio()