Implement fast zsmooth option for image trace - #5354

Merged
archmoj merged 14 commits into
plotly:masterfrom
almarklein:interp
Jan 7, 2021
Merged

Implement fast zsmooth option for image trace#5354
archmoj merged 14 commits into
plotly:masterfrom
almarklein:interp

Conversation

@almarklein

@almarkleinalmarklein commented Dec 18, 2020•
edited
Loading

Copy link
Copy Markdown
Contributor

Closes#5353

I confirmed the working using a simple html doc.
What's the best way to add a test for this? I'm not sure if an image-based test will be flaky?

The value of image-rendering, according to the MDN docs is auto by default, the behavior of which is UA dependent. This is probably linear interpolation for most/all browsers, but we could consider using auto instead of linear.

<html><scriptsrc='../dist/plotly.js'></script><body><divid='myDiv'></div><script>varsource="data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAADIAAAAlCAYAAAAA7LqSAAADmklEQVR4nO2XUWhTVxjHfyfdmKMN1oelK9QSLUVY2RWHrEKtLw66KQX30Ifpg0qxz31wb8O0UNjL1I3h09iDAy000D0Z0DJhEdslBXN7SwZBJmmbErKwNuU2UXDes4d4r700bib33KflD+Ge8+We78sv3/fdcy401VRTTTXVVFMIvxwnEhG5c97fP+lbLPARZHz8qBwY+AAA0zTp69MA/4B8cTo+flRq2n4MY42dMMFgEICRkZ+Uxw2odgigafudq2majn3nWLV8ATGMNdf44cPfd9lVyxeQwcF3MIw1gsEgY2OLzM9fYmDgPebnLzlQquVLj1y//pFcWan+R1eujLm+KxYTHDr0o/K4b6l2GI2efPnY3eTUqSMUiwmgBQDD+ANNC6sOCSgurUxmVAKUSpfp6tpHLJYiFksBL4AXaFr45Vy9lGUkkxmVhpEll9ukra1q6+raB+D68d3d76oK6ZKSjGQyozIWS5HLbVIujwCQyz0jHL4LVIHsT3f3r0xNfS3/zV8jUlpaNsT6eoS2tq/Q9TnSaZOZmRAzMyFKpcvo+hz5fEE5jGcQOxt2GeXzBQC2t6fY3p5y3bu+HnFsqmE8gdh9AThlZGtr67kzDoU6CIU6nPnGxioXLnzD0NCXLC7+rQTGE4jd3MePJ9H1OQBu3PjW2SO2tp6TTvfQ2dnhWjc9PSvs++2rVyl7aun6XTY2Vl22dLqHw4erp97Ozg6WlnqIRqfFqzVqIMADiL3x7cyGrb1733bGds8ArvK6f/+HRkPXVMNHhWj0pMzlNkkmw46tvT1NofAUTXtfPnnyrKbvgwf3yImJpPIznufSOns2C8Dt22EKhafMzq6KfL7XKpfZ1cStrUEeP/YasbY8Hd6KxS/kvXvfkc3eQdO+Z3j4kTh27BMrELCoVCpYlvUqkBBIKWltrb5cLSz8ojQrnpzZEOHwaYaHHwkAISSVSgWAlpYWDCMZMIxkwLIshBAIoXxTBzyA3Lr1p8xm7zA4+DnnzoUEwNWrnzkpEEKQSi04/peXFwMA5XKZoSGT8+c/tHZ7bVwNgcTjJQfixIl2AXDt2qeWlILe3iKAC8LW0lIicOZMdXzggNrDY90g8XhJPnjwswsCqiVlmn8hhETXf3ut38nJhC9vpXU5fR2ErWy2ws2by//pc2WlXE/YN9Ibg9gQ4fDpmhCgvlx8UTxekvF4qeYjJxLptyYmPq6reS9e7Kt7zf9C/wDLc4opp/2WUgAAAABJRU5ErkJggg==";vardata=[{"type": "image","source": source,"xaxis": "x1",yaxis: "y1","interpolate": "nearest"},{"type": "image","source": source,"xaxis": "x2",yaxis: "y2","interpolate": "linear"},];varlayout={"grid": {"rows": 1,"columns": 2,"pattern": "independent"},"width": 600,"height": 300,"margin": {"t": 35,"l": 35,"b": 35,"r": 35}};Plotly.newPlot('myDiv',data,layout,{scrollZoom: true});</script></body></html>

archmoj
archmoj reviewed Dec 18, 2020
View reviewed changes
Comment threadsrc/traces/image/attributes.js Outdated
archmoj
archmoj reviewed Dec 18, 2020
View reviewed changes
Comment threadsrc/traces/image/defaults.js
archmoj
archmoj reviewed Dec 18, 2020
View reviewed changes
Comment threadsrc/traces/image/plot.js Outdated
@antoinerg

antoinerg commented Dec 18, 2020

Copy link
Copy Markdown
Contributor

Thanks, @almarklein for adding support for interpolation in the image trace.

At this point, you took care of one of the 2 rendering modes of image:

  • the fast one that simply displays an image element
  • legacy one (used for static image export) that draws a bunch of SVG rect in place of pixels

I'm not sure what would be the best way to add support for interpolation in the legacy mode. But when it's done, it should be 🔒 down via an image test.

cc @alexcjohnson@archmoj

antoinerg
antoinerg reviewed Dec 18, 2020
View reviewed changes
Comment threadsrc/traces/image/attributes.js Outdated
@alexcjohnson

alexcjohnson commented Dec 18, 2020

Copy link
Copy Markdown
Collaborator

This seems awfully similar to zsmooth='fast' for heatmaps:

plotly.js/src/traces/heatmap/attributes.js

Lines 80 to 89 in d45eef5

zsmooth: {
valType: 'enumerated',
values: ['fast','best',false],
dflt: false,
role: 'style',
editType: 'calc',
description: [
'Picks a smoothing algorithm use to smooth `z` data.'
].join(' ')
},

would it be too awkward to use the same attribute name & values?

Also, what happens if fastImage is false? In that case it looks like we're filling rectangles that are larger than 1px with constant color. Doing manual bilinear interpolation in that case is what zsmooth='best' does for heatmaps.

@almarklein

almarklein commented Dec 18, 2020

Copy link
Copy Markdown
ContributorAuthor

would it be too awkward to use the same attribute name & values?

No, +1 for consistency. This also solves the issue that we don't know the interpolation method that a browser will actually use.

So I think either:

  • make this property specific to image with source set (not good for consistency).
  • implement smoothing in the legacy rendering approach.

@alexcjohnson

alexcjohnson commented Dec 18, 2020

Copy link
Copy Markdown
Collaborator

It's fine to implement this only in a subset of situations for now, as long as we won't have to change the existing behavior if and when we extend it to more situations, and as long as we document the limitations.

@archmojarchmoj added the community community contribution label Dec 18, 2020
archmoj
archmoj reviewed Dec 21, 2020
View reviewed changes
Comment threadsrc/traces/image/defaults.js Outdated
archmoj
archmoj reviewed Dec 21, 2020
View reviewed changes
Comment threadsrc/traces/image/plot.js Outdated
@archmojarchmoj changed the title Add Image.interpolate propertyImplement fast zsmooth option for image traceDec 21, 2020
@archmoj

archmoj commented Jan 4, 2021

Copy link
Copy Markdown
Contributor

@almarklein Happy New Year!
Are you still interested to finish this feature?

@almarklein@archmoj
Update src/traces/image/defaults.js
1b53120
Co-authored-by: Mojtaba Samimi <33888540+archmoj@users.noreply.github.com>
archmoj
archmoj reviewed Jan 4, 2021
View reviewed changes
Comment threadsrc/traces/image/attributes.js Outdated
archmoj
archmoj reviewed Jan 4, 2021
View reviewed changes
Comment threadsrc/traces/image/attributes.js Outdated
archmoj
archmoj reviewed Jan 4, 2021
View reviewed changes
Comment threadsrc/traces/image/attributes.js Outdated
archmoj
archmoj reviewed Jan 4, 2021
View reviewed changes
Comment threadsrc/traces/image/attributes.js
@archmoj

archmoj commented Jan 4, 2021

Copy link
Copy Markdown
Contributor

The syntax test is failing due to new year headers.
When possible, please sync your fork then merge upstream/master into your branch.
Here you could find useful info https://digitaldrummerj.me/git-sync-fork-to-master/

@almarklein

almarklein commented Jan 4, 2021

Copy link
Copy Markdown
ContributorAuthor

@almarklein Happy New Year!
Are you still interested to finish this feature?

Happy 2021! Yeah I'll have at least another stab at it this week...

almarkleinand others added 4 commits January 5, 2021 09:46
@almarklein@archmoj
Update src/traces/image/attributes.js
8f0b0f1
Co-authored-by: Mojtaba Samimi <33888540+archmoj@users.noreply.github.com>
@almarklein@archmoj
Update src/traces/image/attributes.js
33ab959
Co-authored-by: Mojtaba Samimi <33888540+archmoj@users.noreply.github.com>
@almarklein@archmoj
Update src/traces/image/attributes.js
a3eff71
Co-authored-by: Mojtaba Samimi <33888540+archmoj@users.noreply.github.com>
@almarklein@archmoj
Update src/traces/image/attributes.js
bd28585
Co-authored-by: Mojtaba Samimi <33888540+archmoj@users.noreply.github.com>
almarklein
almarklein commented Jan 5, 2021
View reviewed changes
Comment threadsrc/traces/image/attributes.js Outdated
Comment on lines +54 to +63
zsmooth: {
valType: 'enumerated',
values: ['fast', false],
dflt: false,
role: 'info',
editType: 'plot',
description: [
'Picks a smoothing algorithm used to smooth `z` data.',
'This only applies for image traces that use the `source` attribute.'
].join(' ')

@almarkleinalmarkleinJan 5, 2021•
edited
Loading

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The heatmap trace has ['fast', 'best', false]. I'm not sure what false means there, but would it not make more sense to have 'fast' and 'best' here, which would resolve to pixelated and "auto", respectively?

We should also add a note that the result may be browser dependent.

@alexcjohnsonalexcjohnsonJan 5, 2021

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ha, we really need better documentation of those options for heatmaps, but here's what they mean:

  • false means no smoothing, each data point is a rectangle (or "brick") of constant color.
  • 'fast' means we let the browser interpolate smoothly between one data value and the next (so yes, the result may be browser-dependent). This only works when all pixels are the same size, and it can cause problems if two neighboring points have very different value, as the browser will probably interpolate linearly in RGB space and intermediate values may not be present in the colorscale at all.
  • 'best' means we smooth by calculating the color at each screen pixel independently, first with bilinear interpolation of data values, then we map the result onto the colorscale. This can be slow, but it handles nonuniform x or y spacing and ensures every interpolated value is in the colorscale.

So what you've implemented here - assuming the browser behaves as expected - corresponds to false (pixelated) and 'fast' (smoothed). 'best' probably doesn't make sense for images unless there are important browsers that we can't get to behave correctly with 'fast'. But we don't support nonuniform pixels in image traces, and there's no colorscale so the interpolation we would do manually is likely the same as the browser does, unless perhaps we wanted to support interpolating in HSL space.

@archmoj

archmoj commented Jan 5, 2021

Copy link
Copy Markdown
Contributor

Thanks @almarklein for completing the PR.
Here are some minor syntax issues reported by the test-syntax.
Screenshot from 2021-01-05 09-34-38

@archmojarchmoj added this to the 1.59.0 milestone Jan 5, 2021
@almarklein

almarklein commented Jan 6, 2021

Copy link
Copy Markdown
ContributorAuthor

Here are some minor syntax issues reported by the test-syntax.

Sorry about that. I'm on another machine and did not have Eslint et al. configured yet. I thought I could get away with it - apparently not :P

@archmoj

archmoj commented Jan 7, 2021

Copy link
Copy Markdown
Contributor

image_source_axis_reverse is broken.
Could you please investigate why?

@almarklein

almarklein commented Jan 7, 2021

Copy link
Copy Markdown
ContributorAuthor

It should be fixed now.

@archmoj

archmoj commented Jan 7, 2021

Copy link
Copy Markdown
Contributor

Nicely done.
💃
I'll add an image test in a follow up PR.

@archmoj
archmoj merged commit 3a501ef into plotly:masterJan 7, 2021
@almarklein
almarklein deleted the interp branch January 7, 2021 16:16
@archmojarchmoj mentioned this pull request Jan 7, 2021
Merged
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@antoinergantoinergantoinerg left review comments
@alexcjohnsonalexcjohnsonalexcjohnson left review comments
+1 more reviewer
@archmojarchmojarchmoj left review comments
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

communitycommunity contributionfeaturesomething new

Projects

None yet

Milestone

v2.0.0

Development

Successfully merging this pull request may close these issues.

Rendering Image traces with bilinear interpolation

4 participants

@almarklein@antoinerg@alexcjohnson@archmoj
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Add copy buttons to all 
 blocks\n(function() {\n function addCopyButtons() {\n document.querySelectorAll('pre code').forEach(function(codeBlock) {\n if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;\n codeBlock.parentElement.setAttribute('data-copy-added', 'true');\n \n var btn = document.createElement('button');\n btn.textContent = 'Copy';\n btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';\n btn.onmouseover = function() { this.style.opacity = '1'; };\n btn.onmouseout = function() { this.style.opacity = '0.7'; };\n btn.onclick = function() {\n navigator.clipboard.writeText(codeBlock.textContent).then(function() {\n btn.textContent = 'Copied!';\n setTimeout(function() { btn.textContent = 'Copy'; }, 1500);\n });\n };\n codeBlock.parentElement.style.position = 'relative';\n codeBlock.parentElement.appendChild(btn);\n });\n }\n \n addCopyButtons();\n \n // Re-run on dynamic content\n var observer = new MutationObserver(addCopyButtons);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Add Copy Buttons to Code Blocks");
}
} catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); }
})();
(function(){
try {
var __m = "github.com";
var __re = new RegExp('^' + "github\\.com" + '

Skip to content
Implement fast zsmooth option for image trace - #5354
Merged
archmoj merged 14 commits into
plotly:masterfrom 
almarklein:interp
Jan 7, 2021
Merged
Implement fast zsmooth option for image trace#5354
archmoj merged 14 commits into
plotly:masterfrom 
almarklein:interp
Conversation
@almarklein
@almarkleinalmarklein
commented
Dec 18, 2020•

edited

Loading

Copy link


Copy Markdown

Contributor
Closes#5353
I confirmed the working using a simple html doc.

What's the best way to add a test for this? I'm not sure if an image-based test will be flaky?
The value of image-rendering, according to the MDN docs is auto by default, the behavior of which is UA dependent. This is probably linear interpolation for most/all browsers, but we could consider using auto instead of linear.
<html><scriptsrc='../dist/plotly.js'></script><body><divid='myDiv'></div><script>varsource="data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAADIAAAAlCAYAAAAA7LqSAAADmklEQVR4nO2XUWhTVxjHfyfdmKMN1oelK9QSLUVY2RWHrEKtLw66KQX30Ifpg0qxz31wb8O0UNjL1I3h09iDAy000D0Z0DJhEdslBXN7SwZBJmmbErKwNuU2UXDes4d4r700bib33KflD+Ge8+We78sv3/fdcy401VRTTTXVVFMIvxwnEhG5c97fP+lbLPARZHz8qBwY+AAA0zTp69MA/4B8cTo+flRq2n4MY42dMMFgEICRkZ+Uxw2odgigafudq2majn3nWLV8ATGMNdf44cPfd9lVyxeQwcF3MIw1gsEgY2OLzM9fYmDgPebnLzlQquVLj1y//pFcWan+R1eujLm+KxYTHDr0o/K4b6l2GI2efPnY3eTUqSMUiwmgBQDD+ANNC6sOCSgurUxmVAKUSpfp6tpHLJYiFksBL4AXaFr45Vy9lGUkkxmVhpEll9ukra1q6+raB+D68d3d76oK6ZKSjGQyozIWS5HLbVIujwCQyz0jHL4LVIHsT3f3r0xNfS3/zV8jUlpaNsT6eoS2tq/Q9TnSaZOZmRAzMyFKpcvo+hz5fEE5jGcQOxt2GeXzBQC2t6fY3p5y3bu+HnFsqmE8gdh9AThlZGtr67kzDoU6CIU6nPnGxioXLnzD0NCXLC7+rQTGE4jd3MePJ9H1OQBu3PjW2SO2tp6TTvfQ2dnhWjc9PSvs++2rVyl7aun6XTY2Vl22dLqHw4erp97Ozg6WlnqIRqfFqzVqIMADiL3x7cyGrb1733bGds8ArvK6f/+HRkPXVMNHhWj0pMzlNkkmw46tvT1NofAUTXtfPnnyrKbvgwf3yImJpPIznufSOns2C8Dt22EKhafMzq6KfL7XKpfZ1cStrUEeP/YasbY8Hd6KxS/kvXvfkc3eQdO+Z3j4kTh27BMrELCoVCpYlvUqkBBIKWltrb5cLSz8ojQrnpzZEOHwaYaHHwkAISSVSgWAlpYWDCMZMIxkwLIshBAIoXxTBzyA3Lr1p8xm7zA4+DnnzoUEwNWrnzkpEEKQSi04/peXFwMA5XKZoSGT8+c/tHZ7bVwNgcTjJQfixIl2AXDt2qeWlILe3iKAC8LW0lIicOZMdXzggNrDY90g8XhJPnjwswsCqiVlmn8hhETXf3ut38nJhC9vpXU5fR2ErWy2ws2by//pc2WlXE/YN9Ibg9gQ4fDpmhCgvlx8UTxekvF4qeYjJxLptyYmPq6reS9e7Kt7zf9C/wDLc4opp/2WUgAAAABJRU5ErkJggg==";vardata=[{"type": "image","source": source,"xaxis": "x1",yaxis: "y1","interpolate": "nearest"},{"type": "image","source": source,"xaxis": "x2",yaxis: "y2","interpolate": "linear"},];varlayout={"grid": {"rows": 1,"columns": 2,"pattern": "independent"},"width": 600,"height": 300,"margin": {"t": 35,"l": 35,"b": 35,"r": 35}};Plotly.newPlot('myDiv',data,layout,{scrollZoom: true});</script></body></html>
archmoj
archmoj
reviewed
Dec 18, 2020
View reviewed changes
Comment threadsrc/traces/image/attributes.js
Outdated

archmoj
archmoj
reviewed
Dec 18, 2020
View reviewed changes
Comment threadsrc/traces/image/defaults.js
archmoj
archmoj
reviewed
Dec 18, 2020
View reviewed changes
Comment threadsrc/traces/image/plot.js
Outdated

@antoinerg
antoinerg
commented
Dec 18, 2020

Copy link


Copy Markdown

Contributor
Thanks, @almarklein for adding support for interpolation in the image trace.
At this point, you took care of one of the 2 rendering modes of image:
  •  the fast one that simply displays an image element
  •  legacy one (used for static image export) that draws a bunch of SVG rect in place of pixels
I'm not sure what would be the best way to add support for interpolation in the legacy mode. But when it's done, it should be 🔒 down via an image test.
cc @alexcjohnson@archmoj
antoinerg
antoinerg
reviewed
Dec 18, 2020
View reviewed changes
Comment threadsrc/traces/image/attributes.js
Outdated

@alexcjohnson
alexcjohnson
commented
Dec 18, 2020

Copy link


Copy Markdown

Collaborator
This seems awfully similar to zsmooth='fast' for heatmaps:
plotly.js/src/traces/heatmap/attributes.js

Lines 80 to 89
in
d45eef5
zsmooth: {
valType: 'enumerated',
values: ['fast','best',false],
dflt: false,
role: 'style',
editType: 'calc',
description: [
'Picks a smoothing algorithm use to smooth `z` data.'
].join(' ')
},
would it be too awkward to use the same attribute name & values?
Also, what happens if fastImage is false? In that case it looks like we're filling rectangles that are larger than 1px with constant color. Doing manual bilinear interpolation in that case is what zsmooth='best' does for heatmaps.
@almarklein
almarklein
commented
Dec 18, 2020

Copy link


Copy Markdown

ContributorAuthor
would it be too awkward to use the same attribute name & values?
No, +1 for consistency. This also solves the issue that we don't know the interpolation method that a browser will actually use.
So I think either:
  • make this property specific to image with source set (not good for consistency).
  • implement smoothing in the legacy rendering approach.
@alexcjohnson
alexcjohnson
commented
Dec 18, 2020

Copy link


Copy Markdown

Collaborator
It's fine to implement this only in a subset of situations for now, as long as we won't have to change the existing behavior if and when we extend it to more situations, and as long as we document the limitations.
@archmojarchmoj
added
the 
community
community contribution
label
Dec 18, 2020
archmoj
archmoj
reviewed
Dec 21, 2020
View reviewed changes
Comment threadsrc/traces/image/defaults.js
Outdated

archmoj
archmoj
reviewed
Dec 21, 2020
View reviewed changes
Comment threadsrc/traces/image/plot.js
Outdated

@archmojarchmoj
changed the title
Add Image.interpolate propertyImplement fast zsmooth option for image traceDec 21, 2020
@archmoj
archmoj
commented
Jan 4, 2021

Copy link


Copy Markdown

Contributor
@almarklein Happy New Year!

Are you still interested to finish this feature?
@almarklein@archmoj
Update src/traces/image/defaults.js
1b53120
Co-authored-by: Mojtaba Samimi <33888540+archmoj@users.noreply.github.com>
archmoj
archmoj
reviewed
Jan 4, 2021
View reviewed changes
Comment threadsrc/traces/image/attributes.js
Outdated

archmoj
archmoj
reviewed
Jan 4, 2021
View reviewed changes
Comment threadsrc/traces/image/attributes.js
Outdated

archmoj
archmoj
reviewed
Jan 4, 2021
View reviewed changes
Comment threadsrc/traces/image/attributes.js
Outdated

archmoj
archmoj
reviewed
Jan 4, 2021
View reviewed changes
Comment threadsrc/traces/image/attributes.js
@archmoj
archmoj
commented
Jan 4, 2021

Copy link


Copy Markdown

Contributor
The syntax test is failing due to new year headers.

When possible, please sync your fork then merge upstream/master into your branch.

Here you could find useful info https://digitaldrummerj.me/git-sync-fork-to-master/
@almarklein
almarklein
commented
Jan 4, 2021

Copy link


Copy Markdown

ContributorAuthor
@almarklein Happy New Year!

Are you still interested to finish this feature?
Happy 2021! Yeah I'll have at least another stab at it this week...
almarkleinand others
added 4 commits
January 5, 2021 09:46
@almarklein@archmoj
Update src/traces/image/attributes.js
8f0b0f1
Co-authored-by: Mojtaba Samimi <33888540+archmoj@users.noreply.github.com>
@almarklein@archmoj
Update src/traces/image/attributes.js
33ab959
Co-authored-by: Mojtaba Samimi <33888540+archmoj@users.noreply.github.com>
@almarklein@archmoj
Update src/traces/image/attributes.js
a3eff71
Co-authored-by: Mojtaba Samimi <33888540+archmoj@users.noreply.github.com>
@almarklein@archmoj
Update src/traces/image/attributes.js
bd28585
Co-authored-by: Mojtaba Samimi <33888540+archmoj@users.noreply.github.com>
almarklein
almarklein
commented
Jan 5, 2021
View reviewed changes
Comment threadsrc/traces/image/attributes.js
Outdated


Comment on lines

+54
 to 
+63

 zsmooth: {
 valType: 'enumerated',
 values: ['fast', false],
 dflt: false,
 role: 'info',
 editType: 'plot',
 description: [ 
 'Picks a smoothing algorithm used to smooth `z` data.',
 'This only applies for image traces that use the `source` attribute.'
 ].join(' ') 
@almarkleinalmarkleinJan 5, 2021•

edited

Loading

Copy link


Copy Markdown

ContributorAuthor
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The heatmap trace has ['fast', 'best', false]. I'm not sure what false means there, but would it not make more sense to have 'fast' and 'best' here, which would resolve to pixelated and "auto", respectively?
We should also add a note that the result may be browser dependent.
@alexcjohnsonalexcjohnsonJan 5, 2021

Copy link


Copy Markdown

Collaborator
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ha, we really need better documentation of those options for heatmaps, but here's what they mean:
  • false means no smoothing, each data point is a rectangle (or "brick") of constant color.
  • 'fast' means we let the browser interpolate smoothly between one data value and the next (so yes, the result may be browser-dependent). This only works when all pixels are the same size, and it can cause problems if two neighboring points have very different value, as the browser will probably interpolate linearly in RGB space and intermediate values may not be present in the colorscale at all.
  • 'best' means we smooth by calculating the color at each screen pixel independently, first with bilinear interpolation of data values, then we map the result onto the colorscale. This can be slow, but it handles nonuniform x or y spacing and ensures every interpolated value is in the colorscale.
So what you've implemented here - assuming the browser behaves as expected - corresponds to false (pixelated) and 'fast' (smoothed). 'best' probably doesn't make sense for images unless there are important browsers that we can't get to behave correctly with 'fast'. But we don't support nonuniform pixels in image traces, and there's no colorscale so the interpolation we would do manually is likely the same as the browser does, unless perhaps we wanted to support interpolating in HSL space.
@archmoj
archmoj
commented
Jan 5, 2021

Copy link


Copy Markdown

Contributor
Thanks @almarklein for completing the PR.

Here are some minor syntax issues reported by the test-syntax.
Screenshot from 2021-01-05 09-34-38
@archmojarchmoj
added this to the 1.59.0 milestone
Jan 5, 2021
@almarklein
almarklein
commented
Jan 6, 2021

Copy link


Copy Markdown

ContributorAuthor
Here are some minor syntax issues reported by the test-syntax.
Sorry about that. I'm on another machine and did not have Eslint et al. configured yet. I thought I could get away with it - apparently not :P
@archmoj
archmoj
commented
Jan 7, 2021

Copy link


Copy Markdown

Contributor
image_source_axis_reverse is broken.

Could you please investigate why?
@almarklein
almarklein
commented
Jan 7, 2021

Copy link


Copy Markdown

ContributorAuthor
It should be fixed now.
@archmoj
archmoj
commented
Jan 7, 2021

Copy link


Copy Markdown

Contributor
Nicely done.

💃

I'll add an image test in a follow up PR.
@archmoj
archmoj
merged commit 3a501ef
into
plotly:masterJan 7, 2021
@almarklein
almarklein
deleted the

interp

branch
January 7, 2021 16:16
@archmojarchmoj
mentioned this pull request
Jan 7, 2021
 Merged

Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment

Reviewers

@antoinergantoinergantoinerg left review comments
@alexcjohnsonalexcjohnsonalexcjohnson left review comments

+1 more reviewer

@archmojarchmojarchmoj left review comments
Reviewers whose approvals may not affect merge requirements

Assignees


No one assigned


Labels

communitycommunity contributionfeaturesomething new

Projects


None yet


Milestone

v2.0.0

Development

Successfully merging this pull request may close these issues.

Rendering Image traces with bilinear interpolation


4 participants

@almarklein@antoinerg@alexcjohnson@archmoj

, 'i');
if (__m === '*' || __re.test(location.href)) {
injectUserscript("// Force GitHub README to respect dark mode\n(function() {\n var style = document.createElement('style');\n style.textContent = '\n .markdown-body {\n color-scheme: dark light;\n }\n .markdown-body pre { background: #161b22 !important; }\n .markdown-body code { background: rgba(110, 118, 129, 0.4) !important; }\n .markdown-body table th, .markdown-body table td { border-color: #30363d !important; }\n .markdown-body img { background: #0d1117; }\n .markdown-body blockquote { border-left-color: #8b949e; }\n .markdown-body hr { border-color: #30363d; }\n ';\n document.head.appendChild(style);\n})();", "GitHub Dark Mode README Fix");
}
} catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); }
})();
(function(){
try {
var __m = "*";
var __re = new RegExp('^' + ".*" + '

Skip to content
Implement fast zsmooth option for image trace - #5354
Merged
archmoj merged 14 commits into
plotly:masterfrom 
almarklein:interp
Jan 7, 2021
Merged
Implement fast zsmooth option for image trace#5354
archmoj merged 14 commits into
plotly:masterfrom 
almarklein:interp
Conversation
@almarklein
@almarkleinalmarklein
commented
Dec 18, 2020•

edited

Loading

Copy link


Copy Markdown

Contributor
Closes#5353
I confirmed the working using a simple html doc.

What's the best way to add a test for this? I'm not sure if an image-based test will be flaky?
The value of image-rendering, according to the MDN docs is auto by default, the behavior of which is UA dependent. This is probably linear interpolation for most/all browsers, but we could consider using auto instead of linear.
<html><scriptsrc='../dist/plotly.js'></script><body><divid='myDiv'></div><script>varsource="data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAADIAAAAlCAYAAAAA7LqSAAADmklEQVR4nO2XUWhTVxjHfyfdmKMN1oelK9QSLUVY2RWHrEKtLw66KQX30Ifpg0qxz31wb8O0UNjL1I3h09iDAy000D0Z0DJhEdslBXN7SwZBJmmbErKwNuU2UXDes4d4r700bib33KflD+Ge8+We78sv3/fdcy401VRTTTXVVFMIvxwnEhG5c97fP+lbLPARZHz8qBwY+AAA0zTp69MA/4B8cTo+flRq2n4MY42dMMFgEICRkZ+Uxw2odgigafudq2majn3nWLV8ATGMNdf44cPfd9lVyxeQwcF3MIw1gsEgY2OLzM9fYmDgPebnLzlQquVLj1y//pFcWan+R1eujLm+KxYTHDr0o/K4b6l2GI2efPnY3eTUqSMUiwmgBQDD+ANNC6sOCSgurUxmVAKUSpfp6tpHLJYiFksBL4AXaFr45Vy9lGUkkxmVhpEll9ukra1q6+raB+D68d3d76oK6ZKSjGQyozIWS5HLbVIujwCQyz0jHL4LVIHsT3f3r0xNfS3/zV8jUlpaNsT6eoS2tq/Q9TnSaZOZmRAzMyFKpcvo+hz5fEE5jGcQOxt2GeXzBQC2t6fY3p5y3bu+HnFsqmE8gdh9AThlZGtr67kzDoU6CIU6nPnGxioXLnzD0NCXLC7+rQTGE4jd3MePJ9H1OQBu3PjW2SO2tp6TTvfQ2dnhWjc9PSvs++2rVyl7aun6XTY2Vl22dLqHw4erp97Ozg6WlnqIRqfFqzVqIMADiL3x7cyGrb1733bGds8ArvK6f/+HRkPXVMNHhWj0pMzlNkkmw46tvT1NofAUTXtfPnnyrKbvgwf3yImJpPIznufSOns2C8Dt22EKhafMzq6KfL7XKpfZ1cStrUEeP/YasbY8Hd6KxS/kvXvfkc3eQdO+Z3j4kTh27BMrELCoVCpYlvUqkBBIKWltrb5cLSz8ojQrnpzZEOHwaYaHHwkAISSVSgWAlpYWDCMZMIxkwLIshBAIoXxTBzyA3Lr1p8xm7zA4+DnnzoUEwNWrnzkpEEKQSi04/peXFwMA5XKZoSGT8+c/tHZ7bVwNgcTjJQfixIl2AXDt2qeWlILe3iKAC8LW0lIicOZMdXzggNrDY90g8XhJPnjwswsCqiVlmn8hhETXf3ut38nJhC9vpXU5fR2ErWy2ws2by//pc2WlXE/YN9Ibg9gQ4fDpmhCgvlx8UTxekvF4qeYjJxLptyYmPq6reS9e7Kt7zf9C/wDLc4opp/2WUgAAAABJRU5ErkJggg==";vardata=[{"type": "image","source": source,"xaxis": "x1",yaxis: "y1","interpolate": "nearest"},{"type": "image","source": source,"xaxis": "x2",yaxis: "y2","interpolate": "linear"},];varlayout={"grid": {"rows": 1,"columns": 2,"pattern": "independent"},"width": 600,"height": 300,"margin": {"t": 35,"l": 35,"b": 35,"r": 35}};Plotly.newPlot('myDiv',data,layout,{scrollZoom: true});</script></body></html>
archmoj
archmoj
reviewed
Dec 18, 2020
View reviewed changes
Comment threadsrc/traces/image/attributes.js
Outdated

archmoj
archmoj
reviewed
Dec 18, 2020
View reviewed changes
Comment threadsrc/traces/image/defaults.js
archmoj
archmoj
reviewed
Dec 18, 2020
View reviewed changes
Comment threadsrc/traces/image/plot.js
Outdated

@antoinerg
antoinerg
commented
Dec 18, 2020

Copy link


Copy Markdown

Contributor
Thanks, @almarklein for adding support for interpolation in the image trace.
At this point, you took care of one of the 2 rendering modes of image:
  •  the fast one that simply displays an image element
  •  legacy one (used for static image export) that draws a bunch of SVG rect in place of pixels
I'm not sure what would be the best way to add support for interpolation in the legacy mode. But when it's done, it should be 🔒 down via an image test.
cc @alexcjohnson@archmoj
antoinerg
antoinerg
reviewed
Dec 18, 2020
View reviewed changes
Comment threadsrc/traces/image/attributes.js
Outdated

@alexcjohnson
alexcjohnson
commented
Dec 18, 2020

Copy link


Copy Markdown

Collaborator
This seems awfully similar to zsmooth='fast' for heatmaps:
plotly.js/src/traces/heatmap/attributes.js

Lines 80 to 89
in
d45eef5
zsmooth: {
valType: 'enumerated',
values: ['fast','best',false],
dflt: false,
role: 'style',
editType: 'calc',
description: [
'Picks a smoothing algorithm use to smooth `z` data.'
].join(' ')
},
would it be too awkward to use the same attribute name & values?
Also, what happens if fastImage is false? In that case it looks like we're filling rectangles that are larger than 1px with constant color. Doing manual bilinear interpolation in that case is what zsmooth='best' does for heatmaps.
@almarklein
almarklein
commented
Dec 18, 2020

Copy link


Copy Markdown

ContributorAuthor
would it be too awkward to use the same attribute name & values?
No, +1 for consistency. This also solves the issue that we don't know the interpolation method that a browser will actually use.
So I think either:
  • make this property specific to image with source set (not good for consistency).
  • implement smoothing in the legacy rendering approach.
@alexcjohnson
alexcjohnson
commented
Dec 18, 2020

Copy link


Copy Markdown

Collaborator
It's fine to implement this only in a subset of situations for now, as long as we won't have to change the existing behavior if and when we extend it to more situations, and as long as we document the limitations.
@archmojarchmoj
added
the 
community
community contribution
label
Dec 18, 2020
archmoj
archmoj
reviewed
Dec 21, 2020
View reviewed changes
Comment threadsrc/traces/image/defaults.js
Outdated

archmoj
archmoj
reviewed
Dec 21, 2020
View reviewed changes
Comment threadsrc/traces/image/plot.js
Outdated

@archmojarchmoj
changed the title
Add Image.interpolate propertyImplement fast zsmooth option for image traceDec 21, 2020
@archmoj
archmoj
commented
Jan 4, 2021

Copy link


Copy Markdown

Contributor
@almarklein Happy New Year!

Are you still interested to finish this feature?
@almarklein@archmoj
Update src/traces/image/defaults.js
1b53120
Co-authored-by: Mojtaba Samimi <33888540+archmoj@users.noreply.github.com>
archmoj
archmoj
reviewed
Jan 4, 2021
View reviewed changes
Comment threadsrc/traces/image/attributes.js
Outdated

archmoj
archmoj
reviewed
Jan 4, 2021
View reviewed changes
Comment threadsrc/traces/image/attributes.js
Outdated

archmoj
archmoj
reviewed
Jan 4, 2021
View reviewed changes
Comment threadsrc/traces/image/attributes.js
Outdated

archmoj
archmoj
reviewed
Jan 4, 2021
View reviewed changes
Comment threadsrc/traces/image/attributes.js
@archmoj
archmoj
commented
Jan 4, 2021

Copy link


Copy Markdown

Contributor
The syntax test is failing due to new year headers.

When possible, please sync your fork then merge upstream/master into your branch.

Here you could find useful info https://digitaldrummerj.me/git-sync-fork-to-master/
@almarklein
almarklein
commented
Jan 4, 2021

Copy link


Copy Markdown

ContributorAuthor
@almarklein Happy New Year!

Are you still interested to finish this feature?
Happy 2021! Yeah I'll have at least another stab at it this week...
almarkleinand others
added 4 commits
January 5, 2021 09:46
@almarklein@archmoj
Update src/traces/image/attributes.js
8f0b0f1
Co-authored-by: Mojtaba Samimi <33888540+archmoj@users.noreply.github.com>
@almarklein@archmoj
Update src/traces/image/attributes.js
33ab959
Co-authored-by: Mojtaba Samimi <33888540+archmoj@users.noreply.github.com>
@almarklein@archmoj
Update src/traces/image/attributes.js
a3eff71
Co-authored-by: Mojtaba Samimi <33888540+archmoj@users.noreply.github.com>
@almarklein@archmoj
Update src/traces/image/attributes.js
bd28585
Co-authored-by: Mojtaba Samimi <33888540+archmoj@users.noreply.github.com>
almarklein
almarklein
commented
Jan 5, 2021
View reviewed changes
Comment threadsrc/traces/image/attributes.js
Outdated


Comment on lines

+54
 to 
+63

 zsmooth: {
 valType: 'enumerated',
 values: ['fast', false],
 dflt: false,
 role: 'info',
 editType: 'plot',
 description: [ 
 'Picks a smoothing algorithm used to smooth `z` data.',
 'This only applies for image traces that use the `source` attribute.'
 ].join(' ') 
@almarkleinalmarkleinJan 5, 2021•

edited

Loading

Copy link


Copy Markdown

ContributorAuthor
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The heatmap trace has ['fast', 'best', false]. I'm not sure what false means there, but would it not make more sense to have 'fast' and 'best' here, which would resolve to pixelated and "auto", respectively?
We should also add a note that the result may be browser dependent.
@alexcjohnsonalexcjohnsonJan 5, 2021

Copy link


Copy Markdown

Collaborator
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ha, we really need better documentation of those options for heatmaps, but here's what they mean:
  • false means no smoothing, each data point is a rectangle (or "brick") of constant color.
  • 'fast' means we let the browser interpolate smoothly between one data value and the next (so yes, the result may be browser-dependent). This only works when all pixels are the same size, and it can cause problems if two neighboring points have very different value, as the browser will probably interpolate linearly in RGB space and intermediate values may not be present in the colorscale at all.
  • 'best' means we smooth by calculating the color at each screen pixel independently, first with bilinear interpolation of data values, then we map the result onto the colorscale. This can be slow, but it handles nonuniform x or y spacing and ensures every interpolated value is in the colorscale.
So what you've implemented here - assuming the browser behaves as expected - corresponds to false (pixelated) and 'fast' (smoothed). 'best' probably doesn't make sense for images unless there are important browsers that we can't get to behave correctly with 'fast'. But we don't support nonuniform pixels in image traces, and there's no colorscale so the interpolation we would do manually is likely the same as the browser does, unless perhaps we wanted to support interpolating in HSL space.
@archmoj
archmoj
commented
Jan 5, 2021

Copy link


Copy Markdown

Contributor
Thanks @almarklein for completing the PR.

Here are some minor syntax issues reported by the test-syntax.
Screenshot from 2021-01-05 09-34-38
@archmojarchmoj
added this to the 1.59.0 milestone
Jan 5, 2021
@almarklein
almarklein
commented
Jan 6, 2021

Copy link


Copy Markdown

ContributorAuthor
Here are some minor syntax issues reported by the test-syntax.
Sorry about that. I'm on another machine and did not have Eslint et al. configured yet. I thought I could get away with it - apparently not :P
@archmoj
archmoj
commented
Jan 7, 2021

Copy link


Copy Markdown

Contributor
image_source_axis_reverse is broken.

Could you please investigate why?
@almarklein
almarklein
commented
Jan 7, 2021

Copy link


Copy Markdown

ContributorAuthor
It should be fixed now.
@archmoj
archmoj
commented
Jan 7, 2021

Copy link


Copy Markdown

Contributor
Nicely done.

💃

I'll add an image test in a follow up PR.
@archmoj
archmoj
merged commit 3a501ef
into
plotly:masterJan 7, 2021
@almarklein
almarklein
deleted the

interp

branch
January 7, 2021 16:16
@archmojarchmoj
mentioned this pull request
Jan 7, 2021
 Merged

Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment

Reviewers

@antoinergantoinergantoinerg left review comments
@alexcjohnsonalexcjohnsonalexcjohnson left review comments

+1 more reviewer

@archmojarchmojarchmoj left review comments
Reviewers whose approvals may not affect merge requirements

Assignees


No one assigned


Labels

communitycommunity contributionfeaturesomething new

Projects


None yet


Milestone

v2.0.0

Development

Successfully merging this pull request may close these issues.

Rendering Image traces with bilinear interpolation


4 participants

@almarklein@antoinerg@alexcjohnson@archmoj

, 'i');
if (__m === '*' || __re.test(location.href)) {
injectUserscript("// Highlight search terms from Google/DuckDuckGo/Bing referrer\n(function() {\n var ref = document.referrer;\n var terms = [];\n \n if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) {\n var url = new URL(ref);\n var q = url.searchParams.get('q') || url.searchParams.get('p');\n if (q) {\n terms = q.split(/\\s+/).filter(function(t) { return t.length > 2; });\n }\n }\n \n if (terms.length === 0) return;\n \n var style = document.createElement('style');\n style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }';\n document.head.appendChild(style);\n \n function highlight(node) {\n if (node.nodeType === 3) { // text node\n var text = node.textContent;\n var found = false;\n terms.forEach(function(term) {\n var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\\]\\\\]/g, '\\\\') + ')', 'gi');\n if (regex.test(text)) {\n found = true;\n var frag = document.createDocumentFragment();\n var parts = text.split(regex);\n parts.forEach(function(part, i) {\n if (i % 2 === 0) {\n frag.appendChild(document.createTextNode(part));\n } else {\n var span = document.createElement('span');\n span.className = 'userscript-highlight';\n span.textContent = part;\n frag.appendChild(span);\n }\n });\n node.parentNode.replaceChild(frag, node);\n }\n });\n } else if (node.nodeType === 1 && node.childNodes) { // element\n var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT'];\n if (!skipTags.includes(node.tagName)) {\n Array.from(node.childNodes).forEach(highlight);\n }\n }\n }\n \n highlight(document.body);\n \n // Re-highlight on dynamic content\n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1 || node.nodeType === 3) highlight(node);\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Highlight Search Terms");
}
} catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); }
})();
(function(){
try {
var __m = "*";
var __re = new RegExp('^' + ".*" + '

Skip to content
Implement fast zsmooth option for image trace - #5354
Merged
archmoj merged 14 commits into
plotly:masterfrom 
almarklein:interp
Jan 7, 2021
Merged
Implement fast zsmooth option for image trace#5354
archmoj merged 14 commits into
plotly:masterfrom 
almarklein:interp
Conversation
@almarklein
@almarkleinalmarklein
commented
Dec 18, 2020•

edited

Loading

Copy link


Copy Markdown

Contributor
Closes#5353
I confirmed the working using a simple html doc.

What's the best way to add a test for this? I'm not sure if an image-based test will be flaky?
The value of image-rendering, according to the MDN docs is auto by default, the behavior of which is UA dependent. This is probably linear interpolation for most/all browsers, but we could consider using auto instead of linear.
<html><scriptsrc='../dist/plotly.js'></script><body><divid='myDiv'></div><script>varsource="data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAADIAAAAlCAYAAAAA7LqSAAADmklEQVR4nO2XUWhTVxjHfyfdmKMN1oelK9QSLUVY2RWHrEKtLw66KQX30Ifpg0qxz31wb8O0UNjL1I3h09iDAy000D0Z0DJhEdslBXN7SwZBJmmbErKwNuU2UXDes4d4r700bib33KflD+Ge8+We78sv3/fdcy401VRTTTXVVFMIvxwnEhG5c97fP+lbLPARZHz8qBwY+AAA0zTp69MA/4B8cTo+flRq2n4MY42dMMFgEICRkZ+Uxw2odgigafudq2majn3nWLV8ATGMNdf44cPfd9lVyxeQwcF3MIw1gsEgY2OLzM9fYmDgPebnLzlQquVLj1y//pFcWan+R1eujLm+KxYTHDr0o/K4b6l2GI2efPnY3eTUqSMUiwmgBQDD+ANNC6sOCSgurUxmVAKUSpfp6tpHLJYiFksBL4AXaFr45Vy9lGUkkxmVhpEll9ukra1q6+raB+D68d3d76oK6ZKSjGQyozIWS5HLbVIujwCQyz0jHL4LVIHsT3f3r0xNfS3/zV8jUlpaNsT6eoS2tq/Q9TnSaZOZmRAzMyFKpcvo+hz5fEE5jGcQOxt2GeXzBQC2t6fY3p5y3bu+HnFsqmE8gdh9AThlZGtr67kzDoU6CIU6nPnGxioXLnzD0NCXLC7+rQTGE4jd3MePJ9H1OQBu3PjW2SO2tp6TTvfQ2dnhWjc9PSvs++2rVyl7aun6XTY2Vl22dLqHw4erp97Ozg6WlnqIRqfFqzVqIMADiL3x7cyGrb1733bGds8ArvK6f/+HRkPXVMNHhWj0pMzlNkkmw46tvT1NofAUTXtfPnnyrKbvgwf3yImJpPIznufSOns2C8Dt22EKhafMzq6KfL7XKpfZ1cStrUEeP/YasbY8Hd6KxS/kvXvfkc3eQdO+Z3j4kTh27BMrELCoVCpYlvUqkBBIKWltrb5cLSz8ojQrnpzZEOHwaYaHHwkAISSVSgWAlpYWDCMZMIxkwLIshBAIoXxTBzyA3Lr1p8xm7zA4+DnnzoUEwNWrnzkpEEKQSi04/peXFwMA5XKZoSGT8+c/tHZ7bVwNgcTjJQfixIl2AXDt2qeWlILe3iKAC8LW0lIicOZMdXzggNrDY90g8XhJPnjwswsCqiVlmn8hhETXf3ut38nJhC9vpXU5fR2ErWy2ws2by//pc2WlXE/YN9Ibg9gQ4fDpmhCgvlx8UTxekvF4qeYjJxLptyYmPq6reS9e7Kt7zf9C/wDLc4opp/2WUgAAAABJRU5ErkJggg==";vardata=[{"type": "image","source": source,"xaxis": "x1",yaxis: "y1","interpolate": "nearest"},{"type": "image","source": source,"xaxis": "x2",yaxis: "y2","interpolate": "linear"},];varlayout={"grid": {"rows": 1,"columns": 2,"pattern": "independent"},"width": 600,"height": 300,"margin": {"t": 35,"l": 35,"b": 35,"r": 35}};Plotly.newPlot('myDiv',data,layout,{scrollZoom: true});</script></body></html>
archmoj
archmoj
reviewed
Dec 18, 2020
View reviewed changes
Comment threadsrc/traces/image/attributes.js
Outdated

archmoj
archmoj
reviewed
Dec 18, 2020
View reviewed changes
Comment threadsrc/traces/image/defaults.js
archmoj
archmoj
reviewed
Dec 18, 2020
View reviewed changes
Comment threadsrc/traces/image/plot.js
Outdated

@antoinerg
antoinerg
commented
Dec 18, 2020

Copy link


Copy Markdown

Contributor
Thanks, @almarklein for adding support for interpolation in the image trace.
At this point, you took care of one of the 2 rendering modes of image:
  •  the fast one that simply displays an image element
  •  legacy one (used for static image export) that draws a bunch of SVG rect in place of pixels
I'm not sure what would be the best way to add support for interpolation in the legacy mode. But when it's done, it should be 🔒 down via an image test.
cc @alexcjohnson@archmoj
antoinerg
antoinerg
reviewed
Dec 18, 2020
View reviewed changes
Comment threadsrc/traces/image/attributes.js
Outdated

@alexcjohnson
alexcjohnson
commented
Dec 18, 2020

Copy link


Copy Markdown

Collaborator
This seems awfully similar to zsmooth='fast' for heatmaps:
plotly.js/src/traces/heatmap/attributes.js

Lines 80 to 89
in
d45eef5
zsmooth: {
valType: 'enumerated',
values: ['fast','best',false],
dflt: false,
role: 'style',
editType: 'calc',
description: [
'Picks a smoothing algorithm use to smooth `z` data.'
].join(' ')
},
would it be too awkward to use the same attribute name & values?
Also, what happens if fastImage is false? In that case it looks like we're filling rectangles that are larger than 1px with constant color. Doing manual bilinear interpolation in that case is what zsmooth='best' does for heatmaps.
@almarklein
almarklein
commented
Dec 18, 2020

Copy link


Copy Markdown

ContributorAuthor
would it be too awkward to use the same attribute name & values?
No, +1 for consistency. This also solves the issue that we don't know the interpolation method that a browser will actually use.
So I think either:
  • make this property specific to image with source set (not good for consistency).
  • implement smoothing in the legacy rendering approach.
@alexcjohnson
alexcjohnson
commented
Dec 18, 2020

Copy link


Copy Markdown

Collaborator
It's fine to implement this only in a subset of situations for now, as long as we won't have to change the existing behavior if and when we extend it to more situations, and as long as we document the limitations.
@archmojarchmoj
added
the 
community
community contribution
label
Dec 18, 2020
archmoj
archmoj
reviewed
Dec 21, 2020
View reviewed changes
Comment threadsrc/traces/image/defaults.js
Outdated

archmoj
archmoj
reviewed
Dec 21, 2020
View reviewed changes
Comment threadsrc/traces/image/plot.js
Outdated

@archmojarchmoj
changed the title
Add Image.interpolate propertyImplement fast zsmooth option for image traceDec 21, 2020
@archmoj
archmoj
commented
Jan 4, 2021

Copy link


Copy Markdown

Contributor
@almarklein Happy New Year!

Are you still interested to finish this feature?
@almarklein@archmoj
Update src/traces/image/defaults.js
1b53120
Co-authored-by: Mojtaba Samimi <33888540+archmoj@users.noreply.github.com>
archmoj
archmoj
reviewed
Jan 4, 2021
View reviewed changes
Comment threadsrc/traces/image/attributes.js
Outdated

archmoj
archmoj
reviewed
Jan 4, 2021
View reviewed changes
Comment threadsrc/traces/image/attributes.js
Outdated

archmoj
archmoj
reviewed
Jan 4, 2021
View reviewed changes
Comment threadsrc/traces/image/attributes.js
Outdated

archmoj
archmoj
reviewed
Jan 4, 2021
View reviewed changes
Comment threadsrc/traces/image/attributes.js
@archmoj
archmoj
commented
Jan 4, 2021

Copy link


Copy Markdown

Contributor
The syntax test is failing due to new year headers.

When possible, please sync your fork then merge upstream/master into your branch.

Here you could find useful info https://digitaldrummerj.me/git-sync-fork-to-master/
@almarklein
almarklein
commented
Jan 4, 2021

Copy link


Copy Markdown

ContributorAuthor
@almarklein Happy New Year!

Are you still interested to finish this feature?
Happy 2021! Yeah I'll have at least another stab at it this week...
almarkleinand others
added 4 commits
January 5, 2021 09:46
@almarklein@archmoj
Update src/traces/image/attributes.js
8f0b0f1
Co-authored-by: Mojtaba Samimi <33888540+archmoj@users.noreply.github.com>
@almarklein@archmoj
Update src/traces/image/attributes.js
33ab959
Co-authored-by: Mojtaba Samimi <33888540+archmoj@users.noreply.github.com>
@almarklein@archmoj
Update src/traces/image/attributes.js
a3eff71
Co-authored-by: Mojtaba Samimi <33888540+archmoj@users.noreply.github.com>
@almarklein@archmoj
Update src/traces/image/attributes.js
bd28585
Co-authored-by: Mojtaba Samimi <33888540+archmoj@users.noreply.github.com>
almarklein
almarklein
commented
Jan 5, 2021
View reviewed changes
Comment threadsrc/traces/image/attributes.js
Outdated


Comment on lines

+54
 to 
+63

 zsmooth: {
 valType: 'enumerated',
 values: ['fast', false],
 dflt: false,
 role: 'info',
 editType: 'plot',
 description: [ 
 'Picks a smoothing algorithm used to smooth `z` data.',
 'This only applies for image traces that use the `source` attribute.'
 ].join(' ') 
@almarkleinalmarkleinJan 5, 2021•

edited

Loading

Copy link


Copy Markdown

ContributorAuthor
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The heatmap trace has ['fast', 'best', false]. I'm not sure what false means there, but would it not make more sense to have 'fast' and 'best' here, which would resolve to pixelated and "auto", respectively?
We should also add a note that the result may be browser dependent.
@alexcjohnsonalexcjohnsonJan 5, 2021

Copy link


Copy Markdown

Collaborator
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ha, we really need better documentation of those options for heatmaps, but here's what they mean:
  • false means no smoothing, each data point is a rectangle (or "brick") of constant color.
  • 'fast' means we let the browser interpolate smoothly between one data value and the next (so yes, the result may be browser-dependent). This only works when all pixels are the same size, and it can cause problems if two neighboring points have very different value, as the browser will probably interpolate linearly in RGB space and intermediate values may not be present in the colorscale at all.
  • 'best' means we smooth by calculating the color at each screen pixel independently, first with bilinear interpolation of data values, then we map the result onto the colorscale. This can be slow, but it handles nonuniform x or y spacing and ensures every interpolated value is in the colorscale.
So what you've implemented here - assuming the browser behaves as expected - corresponds to false (pixelated) and 'fast' (smoothed). 'best' probably doesn't make sense for images unless there are important browsers that we can't get to behave correctly with 'fast'. But we don't support nonuniform pixels in image traces, and there's no colorscale so the interpolation we would do manually is likely the same as the browser does, unless perhaps we wanted to support interpolating in HSL space.
@archmoj
archmoj
commented
Jan 5, 2021

Copy link


Copy Markdown

Contributor
Thanks @almarklein for completing the PR.

Here are some minor syntax issues reported by the test-syntax.
Screenshot from 2021-01-05 09-34-38
@archmojarchmoj
added this to the 1.59.0 milestone
Jan 5, 2021
@almarklein
almarklein
commented
Jan 6, 2021

Copy link


Copy Markdown

ContributorAuthor
Here are some minor syntax issues reported by the test-syntax.
Sorry about that. I'm on another machine and did not have Eslint et al. configured yet. I thought I could get away with it - apparently not :P
@archmoj
archmoj
commented
Jan 7, 2021

Copy link


Copy Markdown

Contributor
image_source_axis_reverse is broken.

Could you please investigate why?
@almarklein
almarklein
commented
Jan 7, 2021

Copy link


Copy Markdown

ContributorAuthor
It should be fixed now.
@archmoj
archmoj
commented
Jan 7, 2021

Copy link


Copy Markdown

Contributor
Nicely done.

💃

I'll add an image test in a follow up PR.
@archmoj
archmoj
merged commit 3a501ef
into
plotly:masterJan 7, 2021
@almarklein
almarklein
deleted the

interp

branch
January 7, 2021 16:16
@archmojarchmoj
mentioned this pull request
Jan 7, 2021
 Merged

Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment

Reviewers

@antoinergantoinergantoinerg left review comments
@alexcjohnsonalexcjohnsonalexcjohnson left review comments

+1 more reviewer

@archmojarchmojarchmoj left review comments
Reviewers whose approvals may not affect merge requirements

Assignees


No one assigned


Labels

communitycommunity contributionfeaturesomething new

Projects


None yet


Milestone

v2.0.0

Development

Successfully merging this pull request may close these issues.

Rendering Image traces with bilinear interpolation


4 participants

@almarklein@antoinerg@alexcjohnson@archmoj

, 'i');
if (__m === '*' || __re.test(location.href)) {
injectUserscript("// Strip utm_, fbclid, gclid, etc. from all links on page\n(function() {\n var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content',\n 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid',\n 'ref', 'ref_src', 'source', 'medium', 'campaign'];\n \n function cleanUrl(url) {\n try {\n var u = new URL(url, window.location.origin);\n var changed = false;\n trackingParams.forEach(function(p) {\n if (u.searchParams.has(p)) {\n u.searchParams.delete(p);\n changed = true;\n }\n });\n return changed ? u.toString() : url;\n } catch (e) {\n return url;\n }\n }\n \n function cleanLinks() {\n document.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n \n cleanLinks();\n \n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1) {\n if (node.tagName === 'A') cleanLinks();\n node.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Remove Tracking Parameters from Links");
}
} catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); }
})();
(function(){
try {
var __m = "youtube.com";
var __re = new RegExp('^' + "youtube\\.com" + '

Skip to content
Implement fast zsmooth option for image trace - #5354
Merged
archmoj merged 14 commits into
plotly:masterfrom 
almarklein:interp
Jan 7, 2021
Merged
Implement fast zsmooth option for image trace#5354
archmoj merged 14 commits into
plotly:masterfrom 
almarklein:interp
Conversation
@almarklein
@almarkleinalmarklein
commented
Dec 18, 2020•

edited

Loading

Copy link


Copy Markdown

Contributor
Closes#5353
I confirmed the working using a simple html doc.

What's the best way to add a test for this? I'm not sure if an image-based test will be flaky?
The value of image-rendering, according to the MDN docs is auto by default, the behavior of which is UA dependent. This is probably linear interpolation for most/all browsers, but we could consider using auto instead of linear.
<html><scriptsrc='../dist/plotly.js'></script><body><divid='myDiv'></div><script>varsource="data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAADIAAAAlCAYAAAAA7LqSAAADmklEQVR4nO2XUWhTVxjHfyfdmKMN1oelK9QSLUVY2RWHrEKtLw66KQX30Ifpg0qxz31wb8O0UNjL1I3h09iDAy000D0Z0DJhEdslBXN7SwZBJmmbErKwNuU2UXDes4d4r700bib33KflD+Ge8+We78sv3/fdcy401VRTTTXVVFMIvxwnEhG5c97fP+lbLPARZHz8qBwY+AAA0zTp69MA/4B8cTo+flRq2n4MY42dMMFgEICRkZ+Uxw2odgigafudq2majn3nWLV8ATGMNdf44cPfd9lVyxeQwcF3MIw1gsEgY2OLzM9fYmDgPebnLzlQquVLj1y//pFcWan+R1eujLm+KxYTHDr0o/K4b6l2GI2efPnY3eTUqSMUiwmgBQDD+ANNC6sOCSgurUxmVAKUSpfp6tpHLJYiFksBL4AXaFr45Vy9lGUkkxmVhpEll9ukra1q6+raB+D68d3d76oK6ZKSjGQyozIWS5HLbVIujwCQyz0jHL4LVIHsT3f3r0xNfS3/zV8jUlpaNsT6eoS2tq/Q9TnSaZOZmRAzMyFKpcvo+hz5fEE5jGcQOxt2GeXzBQC2t6fY3p5y3bu+HnFsqmE8gdh9AThlZGtr67kzDoU6CIU6nPnGxioXLnzD0NCXLC7+rQTGE4jd3MePJ9H1OQBu3PjW2SO2tp6TTvfQ2dnhWjc9PSvs++2rVyl7aun6XTY2Vl22dLqHw4erp97Ozg6WlnqIRqfFqzVqIMADiL3x7cyGrb1733bGds8ArvK6f/+HRkPXVMNHhWj0pMzlNkkmw46tvT1NofAUTXtfPnnyrKbvgwf3yImJpPIznufSOns2C8Dt22EKhafMzq6KfL7XKpfZ1cStrUEeP/YasbY8Hd6KxS/kvXvfkc3eQdO+Z3j4kTh27BMrELCoVCpYlvUqkBBIKWltrb5cLSz8ojQrnpzZEOHwaYaHHwkAISSVSgWAlpYWDCMZMIxkwLIshBAIoXxTBzyA3Lr1p8xm7zA4+DnnzoUEwNWrnzkpEEKQSi04/peXFwMA5XKZoSGT8+c/tHZ7bVwNgcTjJQfixIl2AXDt2qeWlILe3iKAC8LW0lIicOZMdXzggNrDY90g8XhJPnjwswsCqiVlmn8hhETXf3ut38nJhC9vpXU5fR2ErWy2ws2by//pc2WlXE/YN9Ibg9gQ4fDpmhCgvlx8UTxekvF4qeYjJxLptyYmPq6reS9e7Kt7zf9C/wDLc4opp/2WUgAAAABJRU5ErkJggg==";vardata=[{"type": "image","source": source,"xaxis": "x1",yaxis: "y1","interpolate": "nearest"},{"type": "image","source": source,"xaxis": "x2",yaxis: "y2","interpolate": "linear"},];varlayout={"grid": {"rows": 1,"columns": 2,"pattern": "independent"},"width": 600,"height": 300,"margin": {"t": 35,"l": 35,"b": 35,"r": 35}};Plotly.newPlot('myDiv',data,layout,{scrollZoom: true});</script></body></html>
archmoj
archmoj
reviewed
Dec 18, 2020
View reviewed changes
Comment threadsrc/traces/image/attributes.js
Outdated

archmoj
archmoj
reviewed
Dec 18, 2020
View reviewed changes
Comment threadsrc/traces/image/defaults.js
archmoj
archmoj
reviewed
Dec 18, 2020
View reviewed changes
Comment threadsrc/traces/image/plot.js
Outdated

@antoinerg
antoinerg
commented
Dec 18, 2020

Copy link


Copy Markdown

Contributor
Thanks, @almarklein for adding support for interpolation in the image trace.
At this point, you took care of one of the 2 rendering modes of image:
  •  the fast one that simply displays an image element
  •  legacy one (used for static image export) that draws a bunch of SVG rect in place of pixels
I'm not sure what would be the best way to add support for interpolation in the legacy mode. But when it's done, it should be 🔒 down via an image test.
cc @alexcjohnson@archmoj
antoinerg
antoinerg
reviewed
Dec 18, 2020
View reviewed changes
Comment threadsrc/traces/image/attributes.js
Outdated

@alexcjohnson
alexcjohnson
commented
Dec 18, 2020

Copy link


Copy Markdown

Collaborator
This seems awfully similar to zsmooth='fast' for heatmaps:
plotly.js/src/traces/heatmap/attributes.js

Lines 80 to 89
in
d45eef5
zsmooth: {
valType: 'enumerated',
values: ['fast','best',false],
dflt: false,
role: 'style',
editType: 'calc',
description: [
'Picks a smoothing algorithm use to smooth `z` data.'
].join(' ')
},
would it be too awkward to use the same attribute name & values?
Also, what happens if fastImage is false? In that case it looks like we're filling rectangles that are larger than 1px with constant color. Doing manual bilinear interpolation in that case is what zsmooth='best' does for heatmaps.
@almarklein
almarklein
commented
Dec 18, 2020

Copy link


Copy Markdown

ContributorAuthor
would it be too awkward to use the same attribute name & values?
No, +1 for consistency. This also solves the issue that we don't know the interpolation method that a browser will actually use.
So I think either:
  • make this property specific to image with source set (not good for consistency).
  • implement smoothing in the legacy rendering approach.
@alexcjohnson
alexcjohnson
commented
Dec 18, 2020

Copy link


Copy Markdown

Collaborator
It's fine to implement this only in a subset of situations for now, as long as we won't have to change the existing behavior if and when we extend it to more situations, and as long as we document the limitations.
@archmojarchmoj
added
the 
community
community contribution
label
Dec 18, 2020
archmoj
archmoj
reviewed
Dec 21, 2020
View reviewed changes
Comment threadsrc/traces/image/defaults.js
Outdated

archmoj
archmoj
reviewed
Dec 21, 2020
View reviewed changes
Comment threadsrc/traces/image/plot.js
Outdated

@archmojarchmoj
changed the title
Add Image.interpolate propertyImplement fast zsmooth option for image traceDec 21, 2020
@archmoj
archmoj
commented
Jan 4, 2021

Copy link


Copy Markdown

Contributor
@almarklein Happy New Year!

Are you still interested to finish this feature?
@almarklein@archmoj
Update src/traces/image/defaults.js
1b53120
Co-authored-by: Mojtaba Samimi <33888540+archmoj@users.noreply.github.com>
archmoj
archmoj
reviewed
Jan 4, 2021
View reviewed changes
Comment threadsrc/traces/image/attributes.js
Outdated

archmoj
archmoj
reviewed
Jan 4, 2021
View reviewed changes
Comment threadsrc/traces/image/attributes.js
Outdated

archmoj
archmoj
reviewed
Jan 4, 2021
View reviewed changes
Comment threadsrc/traces/image/attributes.js
Outdated

archmoj
archmoj
reviewed
Jan 4, 2021
View reviewed changes
Comment threadsrc/traces/image/attributes.js
@archmoj
archmoj
commented
Jan 4, 2021

Copy link


Copy Markdown

Contributor
The syntax test is failing due to new year headers.

When possible, please sync your fork then merge upstream/master into your branch.

Here you could find useful info https://digitaldrummerj.me/git-sync-fork-to-master/
@almarklein
almarklein
commented
Jan 4, 2021

Copy link


Copy Markdown

ContributorAuthor
@almarklein Happy New Year!

Are you still interested to finish this feature?
Happy 2021! Yeah I'll have at least another stab at it this week...
almarkleinand others
added 4 commits
January 5, 2021 09:46
@almarklein@archmoj
Update src/traces/image/attributes.js
8f0b0f1
Co-authored-by: Mojtaba Samimi <33888540+archmoj@users.noreply.github.com>
@almarklein@archmoj
Update src/traces/image/attributes.js
33ab959
Co-authored-by: Mojtaba Samimi <33888540+archmoj@users.noreply.github.com>
@almarklein@archmoj
Update src/traces/image/attributes.js
a3eff71
Co-authored-by: Mojtaba Samimi <33888540+archmoj@users.noreply.github.com>
@almarklein@archmoj
Update src/traces/image/attributes.js
bd28585
Co-authored-by: Mojtaba Samimi <33888540+archmoj@users.noreply.github.com>
almarklein
almarklein
commented
Jan 5, 2021
View reviewed changes
Comment threadsrc/traces/image/attributes.js
Outdated


Comment on lines

+54
 to 
+63

 zsmooth: {
 valType: 'enumerated',
 values: ['fast', false],
 dflt: false,
 role: 'info',
 editType: 'plot',
 description: [ 
 'Picks a smoothing algorithm used to smooth `z` data.',
 'This only applies for image traces that use the `source` attribute.'
 ].join(' ') 
@almarkleinalmarkleinJan 5, 2021•

edited

Loading

Copy link


Copy Markdown

ContributorAuthor
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The heatmap trace has ['fast', 'best', false]. I'm not sure what false means there, but would it not make more sense to have 'fast' and 'best' here, which would resolve to pixelated and "auto", respectively?
We should also add a note that the result may be browser dependent.
@alexcjohnsonalexcjohnsonJan 5, 2021

Copy link


Copy Markdown

Collaborator
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ha, we really need better documentation of those options for heatmaps, but here's what they mean:
  • false means no smoothing, each data point is a rectangle (or "brick") of constant color.
  • 'fast' means we let the browser interpolate smoothly between one data value and the next (so yes, the result may be browser-dependent). This only works when all pixels are the same size, and it can cause problems if two neighboring points have very different value, as the browser will probably interpolate linearly in RGB space and intermediate values may not be present in the colorscale at all.
  • 'best' means we smooth by calculating the color at each screen pixel independently, first with bilinear interpolation of data values, then we map the result onto the colorscale. This can be slow, but it handles nonuniform x or y spacing and ensures every interpolated value is in the colorscale.
So what you've implemented here - assuming the browser behaves as expected - corresponds to false (pixelated) and 'fast' (smoothed). 'best' probably doesn't make sense for images unless there are important browsers that we can't get to behave correctly with 'fast'. But we don't support nonuniform pixels in image traces, and there's no colorscale so the interpolation we would do manually is likely the same as the browser does, unless perhaps we wanted to support interpolating in HSL space.
@archmoj
archmoj
commented
Jan 5, 2021

Copy link


Copy Markdown

Contributor
Thanks @almarklein for completing the PR.

Here are some minor syntax issues reported by the test-syntax.
Screenshot from 2021-01-05 09-34-38
@archmojarchmoj
added this to the 1.59.0 milestone
Jan 5, 2021
@almarklein
almarklein
commented
Jan 6, 2021

Copy link


Copy Markdown

ContributorAuthor
Here are some minor syntax issues reported by the test-syntax.
Sorry about that. I'm on another machine and did not have Eslint et al. configured yet. I thought I could get away with it - apparently not :P
@archmoj
archmoj
commented
Jan 7, 2021

Copy link


Copy Markdown

Contributor
image_source_axis_reverse is broken.

Could you please investigate why?
@almarklein
almarklein
commented
Jan 7, 2021

Copy link


Copy Markdown

ContributorAuthor
It should be fixed now.
@archmoj
archmoj
commented
Jan 7, 2021

Copy link


Copy Markdown

Contributor
Nicely done.

💃

I'll add an image test in a follow up PR.
@archmoj
archmoj
merged commit 3a501ef
into
plotly:masterJan 7, 2021
@almarklein
almarklein
deleted the

interp

branch
January 7, 2021 16:16
@archmojarchmoj
mentioned this pull request
Jan 7, 2021
 Merged

Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment

Reviewers

@antoinergantoinergantoinerg left review comments
@alexcjohnsonalexcjohnsonalexcjohnson left review comments

+1 more reviewer

@archmojarchmojarchmoj left review comments
Reviewers whose approvals may not affect merge requirements

Assignees


No one assigned


Labels

communitycommunity contributionfeaturesomething new

Projects


None yet


Milestone

v2.0.0

Development

Successfully merging this pull request may close these issues.

Rendering Image traces with bilinear interpolation


4 participants

@almarklein@antoinerg@alexcjohnson@archmoj

, 'i');
if (__m === '*' || __re.test(location.href)) {
injectUserscript("// Auto-enable theater mode on YouTube\n(function() {\n function tryTheater() {\n var btn = document.querySelector('button[aria-label=\"Theater mode\"], ytd-player #player button[title=\"Theater mode\"]');\n if (btn && !btn.classList.contains('activated')) {\n btn.click();\n }\n }\n \n // Try immediately\n tryTheater();\n \n // Try after navigation (SPA)\n var lastUrl = location.href;\n setInterval(function() {\n if (location.href !== lastUrl) {\n lastUrl = location.href;\n setTimeout(tryTheater, 500);\n }\n }, 1000);\n \n // Also try on player load\n var observer = new MutationObserver(tryTheater);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "YouTube Theater Mode Default");
}
} catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); }
})();
(function(){
try {
var __m = "*";
var __re = new RegExp('^' + ".*" + '

Skip to content
Implement fast zsmooth option for image trace - #5354
Merged
archmoj merged 14 commits into
plotly:masterfrom 
almarklein:interp
Jan 7, 2021
Merged
Implement fast zsmooth option for image trace#5354
archmoj merged 14 commits into
plotly:masterfrom 
almarklein:interp
Conversation
@almarklein
@almarkleinalmarklein
commented
Dec 18, 2020•

edited

Loading

Copy link


Copy Markdown

Contributor
Closes#5353
I confirmed the working using a simple html doc.

What's the best way to add a test for this? I'm not sure if an image-based test will be flaky?
The value of image-rendering, according to the MDN docs is auto by default, the behavior of which is UA dependent. This is probably linear interpolation for most/all browsers, but we could consider using auto instead of linear.
<html><scriptsrc='../dist/plotly.js'></script><body><divid='myDiv'></div><script>varsource="data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAADIAAAAlCAYAAAAA7LqSAAADmklEQVR4nO2XUWhTVxjHfyfdmKMN1oelK9QSLUVY2RWHrEKtLw66KQX30Ifpg0qxz31wb8O0UNjL1I3h09iDAy000D0Z0DJhEdslBXN7SwZBJmmbErKwNuU2UXDes4d4r700bib33KflD+Ge8+We78sv3/fdcy401VRTTTXVVFMIvxwnEhG5c97fP+lbLPARZHz8qBwY+AAA0zTp69MA/4B8cTo+flRq2n4MY42dMMFgEICRkZ+Uxw2odgigafudq2majn3nWLV8ATGMNdf44cPfd9lVyxeQwcF3MIw1gsEgY2OLzM9fYmDgPebnLzlQquVLj1y//pFcWan+R1eujLm+KxYTHDr0o/K4b6l2GI2efPnY3eTUqSMUiwmgBQDD+ANNC6sOCSgurUxmVAKUSpfp6tpHLJYiFksBL4AXaFr45Vy9lGUkkxmVhpEll9ukra1q6+raB+D68d3d76oK6ZKSjGQyozIWS5HLbVIujwCQyz0jHL4LVIHsT3f3r0xNfS3/zV8jUlpaNsT6eoS2tq/Q9TnSaZOZmRAzMyFKpcvo+hz5fEE5jGcQOxt2GeXzBQC2t6fY3p5y3bu+HnFsqmE8gdh9AThlZGtr67kzDoU6CIU6nPnGxioXLnzD0NCXLC7+rQTGE4jd3MePJ9H1OQBu3PjW2SO2tp6TTvfQ2dnhWjc9PSvs++2rVyl7aun6XTY2Vl22dLqHw4erp97Ozg6WlnqIRqfFqzVqIMADiL3x7cyGrb1733bGds8ArvK6f/+HRkPXVMNHhWj0pMzlNkkmw46tvT1NofAUTXtfPnnyrKbvgwf3yImJpPIznufSOns2C8Dt22EKhafMzq6KfL7XKpfZ1cStrUEeP/YasbY8Hd6KxS/kvXvfkc3eQdO+Z3j4kTh27BMrELCoVCpYlvUqkBBIKWltrb5cLSz8ojQrnpzZEOHwaYaHHwkAISSVSgWAlpYWDCMZMIxkwLIshBAIoXxTBzyA3Lr1p8xm7zA4+DnnzoUEwNWrnzkpEEKQSi04/peXFwMA5XKZoSGT8+c/tHZ7bVwNgcTjJQfixIl2AXDt2qeWlILe3iKAC8LW0lIicOZMdXzggNrDY90g8XhJPnjwswsCqiVlmn8hhETXf3ut38nJhC9vpXU5fR2ErWy2ws2by//pc2WlXE/YN9Ibg9gQ4fDpmhCgvlx8UTxekvF4qeYjJxLptyYmPq6reS9e7Kt7zf9C/wDLc4opp/2WUgAAAABJRU5ErkJggg==";vardata=[{"type": "image","source": source,"xaxis": "x1",yaxis: "y1","interpolate": "nearest"},{"type": "image","source": source,"xaxis": "x2",yaxis: "y2","interpolate": "linear"},];varlayout={"grid": {"rows": 1,"columns": 2,"pattern": "independent"},"width": 600,"height": 300,"margin": {"t": 35,"l": 35,"b": 35,"r": 35}};Plotly.newPlot('myDiv',data,layout,{scrollZoom: true});</script></body></html>
archmoj
archmoj
reviewed
Dec 18, 2020
View reviewed changes
Comment threadsrc/traces/image/attributes.js
Outdated

archmoj
archmoj
reviewed
Dec 18, 2020
View reviewed changes
Comment threadsrc/traces/image/defaults.js
archmoj
archmoj
reviewed
Dec 18, 2020
View reviewed changes
Comment threadsrc/traces/image/plot.js
Outdated

@antoinerg
antoinerg
commented
Dec 18, 2020

Copy link


Copy Markdown

Contributor
Thanks, @almarklein for adding support for interpolation in the image trace.
At this point, you took care of one of the 2 rendering modes of image:
  •  the fast one that simply displays an image element
  •  legacy one (used for static image export) that draws a bunch of SVG rect in place of pixels
I'm not sure what would be the best way to add support for interpolation in the legacy mode. But when it's done, it should be 🔒 down via an image test.
cc @alexcjohnson@archmoj
antoinerg
antoinerg
reviewed
Dec 18, 2020
View reviewed changes
Comment threadsrc/traces/image/attributes.js
Outdated

@alexcjohnson
alexcjohnson
commented
Dec 18, 2020

Copy link


Copy Markdown

Collaborator
This seems awfully similar to zsmooth='fast' for heatmaps:
plotly.js/src/traces/heatmap/attributes.js

Lines 80 to 89
in
d45eef5
zsmooth: {
valType: 'enumerated',
values: ['fast','best',false],
dflt: false,
role: 'style',
editType: 'calc',
description: [
'Picks a smoothing algorithm use to smooth `z` data.'
].join(' ')
},
would it be too awkward to use the same attribute name & values?
Also, what happens if fastImage is false? In that case it looks like we're filling rectangles that are larger than 1px with constant color. Doing manual bilinear interpolation in that case is what zsmooth='best' does for heatmaps.
@almarklein
almarklein
commented
Dec 18, 2020

Copy link


Copy Markdown

ContributorAuthor
would it be too awkward to use the same attribute name & values?
No, +1 for consistency. This also solves the issue that we don't know the interpolation method that a browser will actually use.
So I think either:
  • make this property specific to image with source set (not good for consistency).
  • implement smoothing in the legacy rendering approach.
@alexcjohnson
alexcjohnson
commented
Dec 18, 2020

Copy link


Copy Markdown

Collaborator
It's fine to implement this only in a subset of situations for now, as long as we won't have to change the existing behavior if and when we extend it to more situations, and as long as we document the limitations.
@archmojarchmoj
added
the 
community
community contribution
label
Dec 18, 2020
archmoj
archmoj
reviewed
Dec 21, 2020
View reviewed changes
Comment threadsrc/traces/image/defaults.js
Outdated

archmoj
archmoj
reviewed
Dec 21, 2020
View reviewed changes
Comment threadsrc/traces/image/plot.js
Outdated

@archmojarchmoj
changed the title
Add Image.interpolate propertyImplement fast zsmooth option for image traceDec 21, 2020
@archmoj
archmoj
commented
Jan 4, 2021

Copy link


Copy Markdown

Contributor
@almarklein Happy New Year!

Are you still interested to finish this feature?
@almarklein@archmoj
Update src/traces/image/defaults.js
1b53120
Co-authored-by: Mojtaba Samimi <33888540+archmoj@users.noreply.github.com>
archmoj
archmoj
reviewed
Jan 4, 2021
View reviewed changes
Comment threadsrc/traces/image/attributes.js
Outdated

archmoj
archmoj
reviewed
Jan 4, 2021
View reviewed changes
Comment threadsrc/traces/image/attributes.js
Outdated

archmoj
archmoj
reviewed
Jan 4, 2021
View reviewed changes
Comment threadsrc/traces/image/attributes.js
Outdated

archmoj
archmoj
reviewed
Jan 4, 2021
View reviewed changes
Comment threadsrc/traces/image/attributes.js
@archmoj
archmoj
commented
Jan 4, 2021

Copy link


Copy Markdown

Contributor
The syntax test is failing due to new year headers.

When possible, please sync your fork then merge upstream/master into your branch.

Here you could find useful info https://digitaldrummerj.me/git-sync-fork-to-master/
@almarklein
almarklein
commented
Jan 4, 2021

Copy link


Copy Markdown

ContributorAuthor
@almarklein Happy New Year!

Are you still interested to finish this feature?
Happy 2021! Yeah I'll have at least another stab at it this week...
almarkleinand others
added 4 commits
January 5, 2021 09:46
@almarklein@archmoj
Update src/traces/image/attributes.js
8f0b0f1
Co-authored-by: Mojtaba Samimi <33888540+archmoj@users.noreply.github.com>
@almarklein@archmoj
Update src/traces/image/attributes.js
33ab959
Co-authored-by: Mojtaba Samimi <33888540+archmoj@users.noreply.github.com>
@almarklein@archmoj
Update src/traces/image/attributes.js
a3eff71
Co-authored-by: Mojtaba Samimi <33888540+archmoj@users.noreply.github.com>
@almarklein@archmoj
Update src/traces/image/attributes.js
bd28585
Co-authored-by: Mojtaba Samimi <33888540+archmoj@users.noreply.github.com>
almarklein
almarklein
commented
Jan 5, 2021
View reviewed changes
Comment threadsrc/traces/image/attributes.js
Outdated


Comment on lines

+54
 to 
+63

 zsmooth: {
 valType: 'enumerated',
 values: ['fast', false],
 dflt: false,
 role: 'info',
 editType: 'plot',
 description: [ 
 'Picks a smoothing algorithm used to smooth `z` data.',
 'This only applies for image traces that use the `source` attribute.'
 ].join(' ') 
@almarkleinalmarkleinJan 5, 2021•

edited

Loading

Copy link


Copy Markdown

ContributorAuthor
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The heatmap trace has ['fast', 'best', false]. I'm not sure what false means there, but would it not make more sense to have 'fast' and 'best' here, which would resolve to pixelated and "auto", respectively?
We should also add a note that the result may be browser dependent.
@alexcjohnsonalexcjohnsonJan 5, 2021

Copy link


Copy Markdown

Collaborator
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ha, we really need better documentation of those options for heatmaps, but here's what they mean:
  • false means no smoothing, each data point is a rectangle (or "brick") of constant color.
  • 'fast' means we let the browser interpolate smoothly between one data value and the next (so yes, the result may be browser-dependent). This only works when all pixels are the same size, and it can cause problems if two neighboring points have very different value, as the browser will probably interpolate linearly in RGB space and intermediate values may not be present in the colorscale at all.
  • 'best' means we smooth by calculating the color at each screen pixel independently, first with bilinear interpolation of data values, then we map the result onto the colorscale. This can be slow, but it handles nonuniform x or y spacing and ensures every interpolated value is in the colorscale.
So what you've implemented here - assuming the browser behaves as expected - corresponds to false (pixelated) and 'fast' (smoothed). 'best' probably doesn't make sense for images unless there are important browsers that we can't get to behave correctly with 'fast'. But we don't support nonuniform pixels in image traces, and there's no colorscale so the interpolation we would do manually is likely the same as the browser does, unless perhaps we wanted to support interpolating in HSL space.
@archmoj
archmoj
commented
Jan 5, 2021

Copy link


Copy Markdown

Contributor
Thanks @almarklein for completing the PR.

Here are some minor syntax issues reported by the test-syntax.
Screenshot from 2021-01-05 09-34-38
@archmojarchmoj
added this to the 1.59.0 milestone
Jan 5, 2021
@almarklein
almarklein
commented
Jan 6, 2021

Copy link


Copy Markdown

ContributorAuthor
Here are some minor syntax issues reported by the test-syntax.
Sorry about that. I'm on another machine and did not have Eslint et al. configured yet. I thought I could get away with it - apparently not :P
@archmoj
archmoj
commented
Jan 7, 2021

Copy link


Copy Markdown

Contributor
image_source_axis_reverse is broken.

Could you please investigate why?
@almarklein
almarklein
commented
Jan 7, 2021

Copy link


Copy Markdown

ContributorAuthor
It should be fixed now.
@archmoj
archmoj
commented
Jan 7, 2021

Copy link


Copy Markdown

Contributor
Nicely done.

💃

I'll add an image test in a follow up PR.
@archmoj
archmoj
merged commit 3a501ef
into
plotly:masterJan 7, 2021
@almarklein
almarklein
deleted the

interp

branch
January 7, 2021 16:16
@archmojarchmoj
mentioned this pull request
Jan 7, 2021
 Merged

Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment

Reviewers

@antoinergantoinergantoinerg left review comments
@alexcjohnsonalexcjohnsonalexcjohnson left review comments

+1 more reviewer

@archmojarchmojarchmoj left review comments
Reviewers whose approvals may not affect merge requirements

Assignees


No one assigned


Labels

communitycommunity contributionfeaturesomething new

Projects


None yet


Milestone

v2.0.0

Development

Successfully merging this pull request may close these issues.

Rendering Image traces with bilinear interpolation


4 participants

@almarklein@antoinerg@alexcjohnson@archmoj

, 'i');
if (__m === '*' || __re.test(location.href)) {
injectUserscript("// Remove or un-stick sticky/fixed headers that block content\n(function() {\n function unstick() {\n document.querySelectorAll('header, nav, [role=\"banner\"], .header, .navbar, .sticky, .fixed-top, [style*=\"position: fixed\"], [style*=\"position:sticky\"]').forEach(function(el) {\n if (el.style.position === 'fixed' || el.style.position === 'sticky' || \n getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') {\n el.style.position = 'static';\n el.style.top = 'auto';\n el.style.zIndex = 'auto';\n }\n });\n }\n \n unstick();\n \n var observer = new MutationObserver(unstick);\n observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] });\n})();", "Kill Sticky Headers");
}
} catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); }
})();
(function(){
try {
var __m = "*";
var __re = new RegExp('^' + ".*" + '

Skip to content
Implement fast zsmooth option for image trace - #5354
Merged
archmoj merged 14 commits into
plotly:masterfrom 
almarklein:interp
Jan 7, 2021
Merged
Implement fast zsmooth option for image trace#5354
archmoj merged 14 commits into
plotly:masterfrom 
almarklein:interp
Conversation
@almarklein
@almarkleinalmarklein
commented
Dec 18, 2020•

edited

Loading

Copy link


Copy Markdown

Contributor
Closes#5353
I confirmed the working using a simple html doc.

What's the best way to add a test for this? I'm not sure if an image-based test will be flaky?
The value of image-rendering, according to the MDN docs is auto by default, the behavior of which is UA dependent. This is probably linear interpolation for most/all browsers, but we could consider using auto instead of linear.
<html><scriptsrc='../dist/plotly.js'></script><body><divid='myDiv'></div><script>varsource="data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAADIAAAAlCAYAAAAA7LqSAAADmklEQVR4nO2XUWhTVxjHfyfdmKMN1oelK9QSLUVY2RWHrEKtLw66KQX30Ifpg0qxz31wb8O0UNjL1I3h09iDAy000D0Z0DJhEdslBXN7SwZBJmmbErKwNuU2UXDes4d4r700bib33KflD+Ge8+We78sv3/fdcy401VRTTTXVVFMIvxwnEhG5c97fP+lbLPARZHz8qBwY+AAA0zTp69MA/4B8cTo+flRq2n4MY42dMMFgEICRkZ+Uxw2odgigafudq2majn3nWLV8ATGMNdf44cPfd9lVyxeQwcF3MIw1gsEgY2OLzM9fYmDgPebnLzlQquVLj1y//pFcWan+R1eujLm+KxYTHDr0o/K4b6l2GI2efPnY3eTUqSMUiwmgBQDD+ANNC6sOCSgurUxmVAKUSpfp6tpHLJYiFksBL4AXaFr45Vy9lGUkkxmVhpEll9ukra1q6+raB+D68d3d76oK6ZKSjGQyozIWS5HLbVIujwCQyz0jHL4LVIHsT3f3r0xNfS3/zV8jUlpaNsT6eoS2tq/Q9TnSaZOZmRAzMyFKpcvo+hz5fEE5jGcQOxt2GeXzBQC2t6fY3p5y3bu+HnFsqmE8gdh9AThlZGtr67kzDoU6CIU6nPnGxioXLnzD0NCXLC7+rQTGE4jd3MePJ9H1OQBu3PjW2SO2tp6TTvfQ2dnhWjc9PSvs++2rVyl7aun6XTY2Vl22dLqHw4erp97Ozg6WlnqIRqfFqzVqIMADiL3x7cyGrb1733bGds8ArvK6f/+HRkPXVMNHhWj0pMzlNkkmw46tvT1NofAUTXtfPnnyrKbvgwf3yImJpPIznufSOns2C8Dt22EKhafMzq6KfL7XKpfZ1cStrUEeP/YasbY8Hd6KxS/kvXvfkc3eQdO+Z3j4kTh27BMrELCoVCpYlvUqkBBIKWltrb5cLSz8ojQrnpzZEOHwaYaHHwkAISSVSgWAlpYWDCMZMIxkwLIshBAIoXxTBzyA3Lr1p8xm7zA4+DnnzoUEwNWrnzkpEEKQSi04/peXFwMA5XKZoSGT8+c/tHZ7bVwNgcTjJQfixIl2AXDt2qeWlILe3iKAC8LW0lIicOZMdXzggNrDY90g8XhJPnjwswsCqiVlmn8hhETXf3ut38nJhC9vpXU5fR2ErWy2ws2by//pc2WlXE/YN9Ibg9gQ4fDpmhCgvlx8UTxekvF4qeYjJxLptyYmPq6reS9e7Kt7zf9C/wDLc4opp/2WUgAAAABJRU5ErkJggg==";vardata=[{"type": "image","source": source,"xaxis": "x1",yaxis: "y1","interpolate": "nearest"},{"type": "image","source": source,"xaxis": "x2",yaxis: "y2","interpolate": "linear"},];varlayout={"grid": {"rows": 1,"columns": 2,"pattern": "independent"},"width": 600,"height": 300,"margin": {"t": 35,"l": 35,"b": 35,"r": 35}};Plotly.newPlot('myDiv',data,layout,{scrollZoom: true});</script></body></html>
archmoj
archmoj
reviewed
Dec 18, 2020
View reviewed changes
Comment threadsrc/traces/image/attributes.js
Outdated

archmoj
archmoj
reviewed
Dec 18, 2020
View reviewed changes
Comment threadsrc/traces/image/defaults.js
archmoj
archmoj
reviewed
Dec 18, 2020
View reviewed changes
Comment threadsrc/traces/image/plot.js
Outdated

@antoinerg
antoinerg
commented
Dec 18, 2020

Copy link


Copy Markdown

Contributor
Thanks, @almarklein for adding support for interpolation in the image trace.
At this point, you took care of one of the 2 rendering modes of image:
  •  the fast one that simply displays an image element
  •  legacy one (used for static image export) that draws a bunch of SVG rect in place of pixels
I'm not sure what would be the best way to add support for interpolation in the legacy mode. But when it's done, it should be 🔒 down via an image test.
cc @alexcjohnson@archmoj
antoinerg
antoinerg
reviewed
Dec 18, 2020
View reviewed changes
Comment threadsrc/traces/image/attributes.js
Outdated

@alexcjohnson
alexcjohnson
commented
Dec 18, 2020

Copy link


Copy Markdown

Collaborator
This seems awfully similar to zsmooth='fast' for heatmaps:
plotly.js/src/traces/heatmap/attributes.js

Lines 80 to 89
in
d45eef5
zsmooth: {
valType: 'enumerated',
values: ['fast','best',false],
dflt: false,
role: 'style',
editType: 'calc',
description: [
'Picks a smoothing algorithm use to smooth `z` data.'
].join(' ')
},
would it be too awkward to use the same attribute name & values?
Also, what happens if fastImage is false? In that case it looks like we're filling rectangles that are larger than 1px with constant color. Doing manual bilinear interpolation in that case is what zsmooth='best' does for heatmaps.
@almarklein
almarklein
commented
Dec 18, 2020

Copy link


Copy Markdown

ContributorAuthor
would it be too awkward to use the same attribute name & values?
No, +1 for consistency. This also solves the issue that we don't know the interpolation method that a browser will actually use.
So I think either:
  • make this property specific to image with source set (not good for consistency).
  • implement smoothing in the legacy rendering approach.
@alexcjohnson
alexcjohnson
commented
Dec 18, 2020

Copy link


Copy Markdown

Collaborator
It's fine to implement this only in a subset of situations for now, as long as we won't have to change the existing behavior if and when we extend it to more situations, and as long as we document the limitations.
@archmojarchmoj
added
the 
community
community contribution
label
Dec 18, 2020
archmoj
archmoj
reviewed
Dec 21, 2020
View reviewed changes
Comment threadsrc/traces/image/defaults.js
Outdated

archmoj
archmoj
reviewed
Dec 21, 2020
View reviewed changes
Comment threadsrc/traces/image/plot.js
Outdated

@archmojarchmoj
changed the title
Add Image.interpolate propertyImplement fast zsmooth option for image traceDec 21, 2020
@archmoj
archmoj
commented
Jan 4, 2021

Copy link


Copy Markdown

Contributor
@almarklein Happy New Year!

Are you still interested to finish this feature?
@almarklein@archmoj
Update src/traces/image/defaults.js
1b53120
Co-authored-by: Mojtaba Samimi <33888540+archmoj@users.noreply.github.com>
archmoj
archmoj
reviewed
Jan 4, 2021
View reviewed changes
Comment threadsrc/traces/image/attributes.js
Outdated

archmoj
archmoj
reviewed
Jan 4, 2021
View reviewed changes
Comment threadsrc/traces/image/attributes.js
Outdated

archmoj
archmoj
reviewed
Jan 4, 2021
View reviewed changes
Comment threadsrc/traces/image/attributes.js
Outdated

archmoj
archmoj
reviewed
Jan 4, 2021
View reviewed changes
Comment threadsrc/traces/image/attributes.js
@archmoj
archmoj
commented
Jan 4, 2021

Copy link


Copy Markdown

Contributor
The syntax test is failing due to new year headers.

When possible, please sync your fork then merge upstream/master into your branch.

Here you could find useful info https://digitaldrummerj.me/git-sync-fork-to-master/
@almarklein
almarklein
commented
Jan 4, 2021

Copy link


Copy Markdown

ContributorAuthor
@almarklein Happy New Year!

Are you still interested to finish this feature?
Happy 2021! Yeah I'll have at least another stab at it this week...
almarkleinand others
added 4 commits
January 5, 2021 09:46
@almarklein@archmoj
Update src/traces/image/attributes.js
8f0b0f1
Co-authored-by: Mojtaba Samimi <33888540+archmoj@users.noreply.github.com>
@almarklein@archmoj
Update src/traces/image/attributes.js
33ab959
Co-authored-by: Mojtaba Samimi <33888540+archmoj@users.noreply.github.com>
@almarklein@archmoj
Update src/traces/image/attributes.js
a3eff71
Co-authored-by: Mojtaba Samimi <33888540+archmoj@users.noreply.github.com>
@almarklein@archmoj
Update src/traces/image/attributes.js
bd28585
Co-authored-by: Mojtaba Samimi <33888540+archmoj@users.noreply.github.com>
almarklein
almarklein
commented
Jan 5, 2021
View reviewed changes
Comment threadsrc/traces/image/attributes.js
Outdated


Comment on lines

+54
 to 
+63

 zsmooth: {
 valType: 'enumerated',
 values: ['fast', false],
 dflt: false,
 role: 'info',
 editType: 'plot',
 description: [ 
 'Picks a smoothing algorithm used to smooth `z` data.',
 'This only applies for image traces that use the `source` attribute.'
 ].join(' ') 
@almarkleinalmarkleinJan 5, 2021•

edited

Loading

Copy link


Copy Markdown

ContributorAuthor
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The heatmap trace has ['fast', 'best', false]. I'm not sure what false means there, but would it not make more sense to have 'fast' and 'best' here, which would resolve to pixelated and "auto", respectively?
We should also add a note that the result may be browser dependent.
@alexcjohnsonalexcjohnsonJan 5, 2021

Copy link


Copy Markdown

Collaborator
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ha, we really need better documentation of those options for heatmaps, but here's what they mean:
  • false means no smoothing, each data point is a rectangle (or "brick") of constant color.
  • 'fast' means we let the browser interpolate smoothly between one data value and the next (so yes, the result may be browser-dependent). This only works when all pixels are the same size, and it can cause problems if two neighboring points have very different value, as the browser will probably interpolate linearly in RGB space and intermediate values may not be present in the colorscale at all.
  • 'best' means we smooth by calculating the color at each screen pixel independently, first with bilinear interpolation of data values, then we map the result onto the colorscale. This can be slow, but it handles nonuniform x or y spacing and ensures every interpolated value is in the colorscale.
So what you've implemented here - assuming the browser behaves as expected - corresponds to false (pixelated) and 'fast' (smoothed). 'best' probably doesn't make sense for images unless there are important browsers that we can't get to behave correctly with 'fast'. But we don't support nonuniform pixels in image traces, and there's no colorscale so the interpolation we would do manually is likely the same as the browser does, unless perhaps we wanted to support interpolating in HSL space.
@archmoj
archmoj
commented
Jan 5, 2021

Copy link


Copy Markdown

Contributor
Thanks @almarklein for completing the PR.

Here are some minor syntax issues reported by the test-syntax.
Screenshot from 2021-01-05 09-34-38
@archmojarchmoj
added this to the 1.59.0 milestone
Jan 5, 2021
@almarklein
almarklein
commented
Jan 6, 2021

Copy link


Copy Markdown

ContributorAuthor
Here are some minor syntax issues reported by the test-syntax.
Sorry about that. I'm on another machine and did not have Eslint et al. configured yet. I thought I could get away with it - apparently not :P
@archmoj
archmoj
commented
Jan 7, 2021

Copy link


Copy Markdown

Contributor
image_source_axis_reverse is broken.

Could you please investigate why?
@almarklein
almarklein
commented
Jan 7, 2021

Copy link


Copy Markdown

ContributorAuthor
It should be fixed now.
@archmoj
archmoj
commented
Jan 7, 2021

Copy link


Copy Markdown

Contributor
Nicely done.

💃

I'll add an image test in a follow up PR.
@archmoj
archmoj
merged commit 3a501ef
into
plotly:masterJan 7, 2021
@almarklein
almarklein
deleted the

interp

branch
January 7, 2021 16:16
@archmojarchmoj
mentioned this pull request
Jan 7, 2021
 Merged

Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment

Reviewers

@antoinergantoinergantoinerg left review comments
@alexcjohnsonalexcjohnsonalexcjohnson left review comments

+1 more reviewer

@archmojarchmojarchmoj left review comments
Reviewers whose approvals may not affect merge requirements

Assignees


No one assigned


Labels

communitycommunity contributionfeaturesomething new

Projects


None yet


Milestone

v2.0.0

Development

Successfully merging this pull request may close these issues.

Rendering Image traces with bilinear interpolation


4 participants

@almarklein@antoinerg@alexcjohnson@archmoj

, 'i');
if (__m === '*' || __re.test(location.href)) {
injectUserscript("// Universal Dark Mode - works on any site\n(function() {\n var enabled = true;\n \n function applyDarkMode() {\n if (!enabled) return;\n \n // Create style element if it doesn't exist\n var style = document.getElementById('universal-dark-mode-style');\n if (!style) {\n style = document.createElement('style');\n style.id = 'universal-dark-mode-style';\n document.head.appendChild(style);\n }\n \n // Dark mode CSS - inverts colors but preserves images/video\n style.textContent = '\n /* Invert everything except media */\n html {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #1a1a2e !important;\n }\n \n /* Restore images, videos, iframes, canvas */\n img, video, iframe, canvas, svg, picture, [style*=\"background-image\"] {\n filter: invert(1) hue-rotate(180deg) !important;\n }\n \n /* Preserve specific elements that should not be inverted */\n .no-dark-mode, .no-dark-mode *,\n [data-theme=\"light\"], [data-theme=\"light\"],\n .ace_editor, .ace_editor *,\n .CodeMirror, .CodeMirror *,\n .monaco-editor, .monaco-editor *,\n .markdown-body pre, .markdown-body pre *,\n .highlight, .highlight *,\n pre code, pre code * {\n filter: none !important;\n }\n \n /* Fix common UI elements */\n .modal, .popup, .dropdown-menu, .tooltip, .popover {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #2d2d44 !important;\n border-color: #444 !important;\n }\n \n /* Scrollbars */\n ::-webkit-scrollbar { background: #1a1a2e !important; }\n ::-webkit-scrollbar-thumb { background: #444 !important; }\n ::-webkit-scrollbar-thumb:hover { background: #555 !important; }\n \n /* Selection */\n ::selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ::-moz-selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ';\n }\n \n function removeDarkMode() {\n var style = document.getElementById('universal-dark-mode-style');\n if (style) style.remove();\n }\n \n // Toggle with Alt+Shift+D\n document.addEventListener('keydown', function(e) {\n if (e.altKey && e.shiftKey && e.key === 'D') {\n e.preventDefault();\n enabled = !enabled;\n if (enabled) {\n applyDarkMode();\n console.log('[Universal Dark Mode] Enabled');\n } else {\n removeDarkMode();\n console.log('[Universal Dark Mode] Disabled');\n }\n }\n });\n \n // Apply on load\n applyDarkMode();\n \n // Re-apply on dynamic content\n var observer = new MutationObserver(function(mutations) {\n if (enabled && !document.getElementById('universal-dark-mode-style')) {\n applyDarkMode();\n }\n });\n observer.observe(document.head, { childList: true });\n \n console.log('[Universal Dark Mode] Loaded - Press Alt+Shift+D to toggle');\n})();", "Universal Dark Mode");
}
} catch(__e) { console.warn('[Userscript:Universal Dark Mode]', __e); }
})();
})();

Skip to content
Implement fast zsmooth option for image trace - #5354
Merged
archmoj merged 14 commits into
plotly:masterfrom 
almarklein:interp
Jan 7, 2021
Merged
Implement fast zsmooth option for image trace#5354
archmoj merged 14 commits into
plotly:masterfrom 
almarklein:interp
Conversation
@almarklein
@almarkleinalmarklein
commented
Dec 18, 2020•

edited

Loading

Copy link


Copy Markdown

Contributor
Closes#5353
I confirmed the working using a simple html doc.

What's the best way to add a test for this? I'm not sure if an image-based test will be flaky?
The value of image-rendering, according to the MDN docs is auto by default, the behavior of which is UA dependent. This is probably linear interpolation for most/all browsers, but we could consider using auto instead of linear.
<html><scriptsrc='../dist/plotly.js'></script><body><divid='myDiv'></div><script>varsource="data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAADIAAAAlCAYAAAAA7LqSAAADmklEQVR4nO2XUWhTVxjHfyfdmKMN1oelK9QSLUVY2RWHrEKtLw66KQX30Ifpg0qxz31wb8O0UNjL1I3h09iDAy000D0Z0DJhEdslBXN7SwZBJmmbErKwNuU2UXDes4d4r700bib33KflD+Ge8+We78sv3/fdcy401VRTTTXVVFMIvxwnEhG5c97fP+lbLPARZHz8qBwY+AAA0zTp69MA/4B8cTo+flRq2n4MY42dMMFgEICRkZ+Uxw2odgigafudq2majn3nWLV8ATGMNdf44cPfd9lVyxeQwcF3MIw1gsEgY2OLzM9fYmDgPebnLzlQquVLj1y//pFcWan+R1eujLm+KxYTHDr0o/K4b6l2GI2efPnY3eTUqSMUiwmgBQDD+ANNC6sOCSgurUxmVAKUSpfp6tpHLJYiFksBL4AXaFr45Vy9lGUkkxmVhpEll9ukra1q6+raB+D68d3d76oK6ZKSjGQyozIWS5HLbVIujwCQyz0jHL4LVIHsT3f3r0xNfS3/zV8jUlpaNsT6eoS2tq/Q9TnSaZOZmRAzMyFKpcvo+hz5fEE5jGcQOxt2GeXzBQC2t6fY3p5y3bu+HnFsqmE8gdh9AThlZGtr67kzDoU6CIU6nPnGxioXLnzD0NCXLC7+rQTGE4jd3MePJ9H1OQBu3PjW2SO2tp6TTvfQ2dnhWjc9PSvs++2rVyl7aun6XTY2Vl22dLqHw4erp97Ozg6WlnqIRqfFqzVqIMADiL3x7cyGrb1733bGds8ArvK6f/+HRkPXVMNHhWj0pMzlNkkmw46tvT1NofAUTXtfPnnyrKbvgwf3yImJpPIznufSOns2C8Dt22EKhafMzq6KfL7XKpfZ1cStrUEeP/YasbY8Hd6KxS/kvXvfkc3eQdO+Z3j4kTh27BMrELCoVCpYlvUqkBBIKWltrb5cLSz8ojQrnpzZEOHwaYaHHwkAISSVSgWAlpYWDCMZMIxkwLIshBAIoXxTBzyA3Lr1p8xm7zA4+DnnzoUEwNWrnzkpEEKQSi04/peXFwMA5XKZoSGT8+c/tHZ7bVwNgcTjJQfixIl2AXDt2qeWlILe3iKAC8LW0lIicOZMdXzggNrDY90g8XhJPnjwswsCqiVlmn8hhETXf3ut38nJhC9vpXU5fR2ErWy2ws2by//pc2WlXE/YN9Ibg9gQ4fDpmhCgvlx8UTxekvF4qeYjJxLptyYmPq6reS9e7Kt7zf9C/wDLc4opp/2WUgAAAABJRU5ErkJggg==";vardata=[{"type": "image","source": source,"xaxis": "x1",yaxis: "y1","interpolate": "nearest"},{"type": "image","source": source,"xaxis": "x2",yaxis: "y2","interpolate": "linear"},];varlayout={"grid": {"rows": 1,"columns": 2,"pattern": "independent"},"width": 600,"height": 300,"margin": {"t": 35,"l": 35,"b": 35,"r": 35}};Plotly.newPlot('myDiv',data,layout,{scrollZoom: true});</script></body></html>
archmoj
archmoj
reviewed
Dec 18, 2020
View reviewed changes
Comment threadsrc/traces/image/attributes.js
Outdated

archmoj
archmoj
reviewed
Dec 18, 2020
View reviewed changes
Comment threadsrc/traces/image/defaults.js
archmoj
archmoj
reviewed
Dec 18, 2020
View reviewed changes
Comment threadsrc/traces/image/plot.js
Outdated

@antoinerg
antoinerg
commented
Dec 18, 2020

Copy link


Copy Markdown

Contributor
Thanks, @almarklein for adding support for interpolation in the image trace.
At this point, you took care of one of the 2 rendering modes of image:
  •  the fast one that simply displays an image element
  •  legacy one (used for static image export) that draws a bunch of SVG rect in place of pixels
I'm not sure what would be the best way to add support for interpolation in the legacy mode. But when it's done, it should be 🔒 down via an image test.
cc @alexcjohnson@archmoj
antoinerg
antoinerg
reviewed
Dec 18, 2020
View reviewed changes
Comment threadsrc/traces/image/attributes.js
Outdated

@alexcjohnson
alexcjohnson
commented
Dec 18, 2020

Copy link


Copy Markdown

Collaborator
This seems awfully similar to zsmooth='fast' for heatmaps:
plotly.js/src/traces/heatmap/attributes.js

Lines 80 to 89
in
d45eef5
zsmooth: {
valType: 'enumerated',
values: ['fast','best',false],
dflt: false,
role: 'style',
editType: 'calc',
description: [
'Picks a smoothing algorithm use to smooth `z` data.'
].join(' ')
},
would it be too awkward to use the same attribute name & values?
Also, what happens if fastImage is false? In that case it looks like we're filling rectangles that are larger than 1px with constant color. Doing manual bilinear interpolation in that case is what zsmooth='best' does for heatmaps.
@almarklein
almarklein
commented
Dec 18, 2020

Copy link


Copy Markdown

ContributorAuthor
would it be too awkward to use the same attribute name & values?
No, +1 for consistency. This also solves the issue that we don't know the interpolation method that a browser will actually use.
So I think either:
  • make this property specific to image with source set (not good for consistency).
  • implement smoothing in the legacy rendering approach.
@alexcjohnson
alexcjohnson
commented
Dec 18, 2020

Copy link


Copy Markdown

Collaborator
It's fine to implement this only in a subset of situations for now, as long as we won't have to change the existing behavior if and when we extend it to more situations, and as long as we document the limitations.
@archmojarchmoj
added
the 
community
community contribution
label
Dec 18, 2020
archmoj
archmoj
reviewed
Dec 21, 2020
View reviewed changes
Comment threadsrc/traces/image/defaults.js
Outdated

archmoj
archmoj
reviewed
Dec 21, 2020
View reviewed changes
Comment threadsrc/traces/image/plot.js
Outdated

@archmojarchmoj
changed the title
Add Image.interpolate propertyImplement fast zsmooth option for image traceDec 21, 2020
@archmoj
archmoj
commented
Jan 4, 2021

Copy link


Copy Markdown

Contributor
@almarklein Happy New Year!

Are you still interested to finish this feature?
@almarklein@archmoj
Update src/traces/image/defaults.js
1b53120
Co-authored-by: Mojtaba Samimi <33888540+archmoj@users.noreply.github.com>
archmoj
archmoj
reviewed
Jan 4, 2021
View reviewed changes
Comment threadsrc/traces/image/attributes.js
Outdated

archmoj
archmoj
reviewed
Jan 4, 2021
View reviewed changes
Comment threadsrc/traces/image/attributes.js
Outdated

archmoj
archmoj
reviewed
Jan 4, 2021
View reviewed changes
Comment threadsrc/traces/image/attributes.js
Outdated

archmoj
archmoj
reviewed
Jan 4, 2021
View reviewed changes
Comment threadsrc/traces/image/attributes.js
@archmoj
archmoj
commented
Jan 4, 2021

Copy link


Copy Markdown

Contributor
The syntax test is failing due to new year headers.

When possible, please sync your fork then merge upstream/master into your branch.

Here you could find useful info https://digitaldrummerj.me/git-sync-fork-to-master/
@almarklein
almarklein
commented
Jan 4, 2021

Copy link


Copy Markdown

ContributorAuthor
@almarklein Happy New Year!

Are you still interested to finish this feature?
Happy 2021! Yeah I'll have at least another stab at it this week...
almarkleinand others
added 4 commits
January 5, 2021 09:46
@almarklein@archmoj
Update src/traces/image/attributes.js
8f0b0f1
Co-authored-by: Mojtaba Samimi <33888540+archmoj@users.noreply.github.com>
@almarklein@archmoj
Update src/traces/image/attributes.js
33ab959
Co-authored-by: Mojtaba Samimi <33888540+archmoj@users.noreply.github.com>
@almarklein@archmoj
Update src/traces/image/attributes.js
a3eff71
Co-authored-by: Mojtaba Samimi <33888540+archmoj@users.noreply.github.com>
@almarklein@archmoj
Update src/traces/image/attributes.js
bd28585
Co-authored-by: Mojtaba Samimi <33888540+archmoj@users.noreply.github.com>
almarklein
almarklein
commented
Jan 5, 2021
View reviewed changes
Comment threadsrc/traces/image/attributes.js
Outdated


Comment on lines

+54
 to 
+63

 zsmooth: {
 valType: 'enumerated',
 values: ['fast', false],
 dflt: false,
 role: 'info',
 editType: 'plot',
 description: [ 
 'Picks a smoothing algorithm used to smooth `z` data.',
 'This only applies for image traces that use the `source` attribute.'
 ].join(' ') 
@almarkleinalmarkleinJan 5, 2021•

edited

Loading

Copy link


Copy Markdown

ContributorAuthor
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The heatmap trace has ['fast', 'best', false]. I'm not sure what false means there, but would it not make more sense to have 'fast' and 'best' here, which would resolve to pixelated and "auto", respectively?
We should also add a note that the result may be browser dependent.
@alexcjohnsonalexcjohnsonJan 5, 2021

Copy link


Copy Markdown

Collaborator
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ha, we really need better documentation of those options for heatmaps, but here's what they mean:
  • false means no smoothing, each data point is a rectangle (or "brick") of constant color.
  • 'fast' means we let the browser interpolate smoothly between one data value and the next (so yes, the result may be browser-dependent). This only works when all pixels are the same size, and it can cause problems if two neighboring points have very different value, as the browser will probably interpolate linearly in RGB space and intermediate values may not be present in the colorscale at all.
  • 'best' means we smooth by calculating the color at each screen pixel independently, first with bilinear interpolation of data values, then we map the result onto the colorscale. This can be slow, but it handles nonuniform x or y spacing and ensures every interpolated value is in the colorscale.
So what you've implemented here - assuming the browser behaves as expected - corresponds to false (pixelated) and 'fast' (smoothed). 'best' probably doesn't make sense for images unless there are important browsers that we can't get to behave correctly with 'fast'. But we don't support nonuniform pixels in image traces, and there's no colorscale so the interpolation we would do manually is likely the same as the browser does, unless perhaps we wanted to support interpolating in HSL space.
@archmoj
archmoj
commented
Jan 5, 2021

Copy link


Copy Markdown

Contributor
Thanks @almarklein for completing the PR.

Here are some minor syntax issues reported by the test-syntax.
Screenshot from 2021-01-05 09-34-38
@archmojarchmoj
added this to the 1.59.0 milestone
Jan 5, 2021
@almarklein
almarklein
commented
Jan 6, 2021

Copy link


Copy Markdown

ContributorAuthor
Here are some minor syntax issues reported by the test-syntax.
Sorry about that. I'm on another machine and did not have Eslint et al. configured yet. I thought I could get away with it - apparently not :P
@archmoj
archmoj
commented
Jan 7, 2021

Copy link


Copy Markdown

Contributor
image_source_axis_reverse is broken.

Could you please investigate why?
@almarklein
almarklein
commented
Jan 7, 2021

Copy link


Copy Markdown

ContributorAuthor
It should be fixed now.
@archmoj
archmoj
commented
Jan 7, 2021

Copy link


Copy Markdown

Contributor
Nicely done.

💃

I'll add an image test in a follow up PR.
@archmoj
archmoj
merged commit 3a501ef
into
plotly:masterJan 7, 2021
@almarklein
almarklein
deleted the

interp

branch
January 7, 2021 16:16
@archmojarchmoj
mentioned this pull request
Jan 7, 2021
 Merged

Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment

Reviewers

@antoinergantoinergantoinerg left review comments
@alexcjohnsonalexcjohnsonalexcjohnson left review comments

+1 more reviewer

@archmojarchmojarchmoj left review comments
Reviewers whose approvals may not affect merge requirements

Assignees


No one assigned


Labels

communitycommunity contributionfeaturesomething new

Projects


None yet


Milestone

v2.0.0

Development

Successfully merging this pull request may close these issues.

Rendering Image traces with bilinear interpolation


4 participants

@almarklein@antoinerg@alexcjohnson@archmoj