Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve heatmap rendering performance when zsmooth is false #6574

Merged
merged 10 commits into from
May 12, 2023

Conversation

lvlte
Copy link
Contributor

@lvlte lvlte commented Apr 18, 2023

This PR fixes #6573, here are the main changes :

  • add trace._is_linear flag to help determine drawing method
  • use "fast" drawing method whenever possible (ie. flag is true, no gap - Nb. still need to check browser support for the image-rendering directive, I should probably have started with this !)
  • fix heatmap tests, taking account of these changes

Although the vocabulary about the drawing method might not be appropriate or sounds confusing, the proposition here is to render the heatmap according to the determined method instead of the zsmooth parameter, for now these methods appears in the code as :

  • smooth: zsmooth='best' (manual interpolation), uses imageData
  • fast: zsmooth=false|'fast', uses imageData; fastest method but requires all bricks be uniform, with no xgap/ygap, if zsmooth is 'fast' we let the browser do the interpolation, otherwise we prevent it from doing so;
  • default: for all other situations (zsmooth=false), uses fillRect

- add `trace._is_linear` flag to help determine drawing method
- use "fast" drawing method whenever possible (ie. flag is true, no gap)
- fix heatmap tests, taking account of these changes
@alexcjohnson
Copy link
Collaborator

Thanks @lvlte! Re: feature detection - I did a little checking and wasn't able to get an svg <image> tag to tell me whether it successfully pixelated or not, but I could on an html <img> tag. Could translate this into D3 but doesn't much matter, and I was fiddling in vanilla JS. Here's what I came up with:

var _supportsPixelated = null;
function supportsPixelated() {
    if(_supportsPixelated === null) {  // only run the feature detection once
        var img = document.createElement('img');
        document.body.appendChild(img);
        img.style.imageRendering = 'pixelated';
        _supportsPixelated = getComputedStyle(img).imageRendering === 'pixelated';
        document.body.removeChild(img);
    }
    return _supportsPixelated;
}

@lvlte
Copy link
Contributor Author

lvlte commented Apr 21, 2023

Thank you @alexcjohnson, I used your code as a starting point to make the testing work with SVG <image> using d3, also added a chunk that uses CSS.supports() instead (for browsers that supports it lol..). I also added CSS fallbacks for older browsers to give them a chance to have a faster rendering.

Also, it's worth noting the exact same trick is used for image traces, the fallbacks were taken from there actually (see my comment on the issue, you probably missed it because I've made an edit). I also tried -ms-interpolation-mode: nearest-neighbor; for IE but it works only with <img>, that's why I finally force the testing on an actual SVG <image>. So the only difference with image traces is that gd._context._exportedPlot does not matter here (unless I missed something) I tested exports with Plotly.toImage() and it looks ok (the same) with and without the faster/pixelated rendering.

@archmoj archmoj added community community contribution status: reviewable labels Apr 25, 2023
@archmoj
Copy link
Contributor

archmoj commented Apr 25, 2023

@lvlte Thanks for the PR.
Would you please download baselines.tar file from https://app.circleci.com/pipelines/github/plotly/plotly.js/8555/workflows/27e9361b-a71d-499d-8412-e76ebf63ad20/jobs/189349/artifacts and replace the baselines in test/image/baselines, commit & push.

src/traces/heatmap/calc.js Outdated Show resolved Hide resolved
// `-ms-interpolation-mode` works only with <img> not with SVG <image>
_supportsPixelated = false;
} else {
var declarations = Array.from(pixelatedImageCSS).reverse();
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you please elaborate why we need to reverse the list here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The pixelatedImageCSS array is ordered so that the actual declaration image-rendering: pixelated, which targets modern browsers (most users) is the last one, and the oldest fallback comes first. Reversing the array is just for doing the testing on the actual declaration first, and let declarations.some returns early.

src/traces/heatmap/plot.js Outdated Show resolved Hide resolved
Co-authored-by: Mojtaba Samimi <33888540+archmoj@users.noreply.github.com>
draftlogs/6574_change.md Outdated Show resolved Hide resolved
lvlte and others added 4 commits April 27, 2023 18:09
Co-authored-by: Mojtaba Samimi <33888540+archmoj@users.noreply.github.com>
- move style and support function into `src/lib`
- make heatmap and image trace use the same base
- fix circular dependency (introduced in previous commit)
- move css constants into `src/constants/pixelated_image`
- move support function into `src/lib/supports_pixelated_image`
Make the condition test work regardless of the drawing method
@lvlte lvlte requested a review from archmoj May 4, 2023 18:41
@archmoj
Copy link
Contributor

archmoj commented May 12, 2023

💃

@archmoj archmoj merged commit 44c97be into plotly:master May 12, 2023
@alexcjohnson
Copy link
Collaborator

@lvlte great work! Not only better performance, I think the results actually look better than before, ie the brick edges are positioned more accurately. Also you taught me about CSS.supports 🧑‍🏫

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

Successfully merging this pull request may close these issues.

Improve Heatmap rendering performance when zsmooth is false
3 participants