-
Notifications
You must be signed in to change notification settings - Fork 23
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
Use a hidden canvas in memory to paint to avoid jank #72
base: master
Are you sure you want to change the base?
Conversation
This pull request is being automatically deployed with ZEIT Now (learn more). 🔍 Inspect: https://zeit.co/ritz078/synth/fh28wueo6 |
I think it's okay now, let me know when you've sometime to look at this, I'll remove the hardcodings to always force into |
// TODO: undo the ternary; not wrapping via Comlink to test offcanvas perf | ||
// on latest chrome, too lazy to install anything else | ||
const canvasProxyRef = useRef<any>(controlVisualizer); | ||
// offScreenCanvasSupport === OFFSCREEN_2D_CANVAS_SUPPORT.SUPPORTED |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll remove this, once you test the perf :P
offScreenCanvasSupport === OFFSCREEN_2D_CANVAS_SUPPORT.SUPPORTED | ||
? canvasRef.current.transferControlToOffscreen() | ||
: canvasRef.current; | ||
const canvas: any = false //isOffscreenSupported |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this too
components/Visualizer/Visualizer.tsx
Outdated
hiddenCanvasElement.height | ||
); | ||
// I think this is too agressive, for later, what's Clock? | ||
requestAnimationFrame(paintToMainCanvas); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The window.requestAnimationFrame() method tells the browser that you wish to perform an animation and requests that the browser call a specified function to update an animation before the next repaint.
The problem here is that we don't have that much time that we can wait till the next repaint. This will make the animation slow and hence out of sync. canvas doesn't trigger repaint and that's why they are faster so if we use requestAnimationFrame
which actually depends on the paint
timing, we are actually going back to the same constraints that div/svg have.
In this case setTimeout will give us better result but then again all the limitations of main thread will come into play which will eventually cause the delay.
I tested this PR and that confirms what I wrote above. You can open the PR deployment URL
You can check the prod build of this PR at https://synth-git-fork-ankeetmaini-no-offscreen.ritz078.now.sh Let me know if you are not able to open it. In case you had trouble on opening the app on browsers other than chrome, take a master pull. |
components/Visualizer/Visualizer.tsx
Outdated
hiddenCanvasElement.width, | ||
hiddenCanvasElement.height | ||
); | ||
// I think this is too agressive, for later, what's Clock? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Clock
keeps track of time and progress of the song. It's separate because it provides better API that way. For eg. when you need to pause and song on canvas, you just have to stop that clock. So it's king of virtual clock which is in our control.
- If we stop it, the canvas pauses because the progress (it's a number between 0-1 which depicts the song's progress) doesn't increase.
- If we change the speed, the clock's speed should be increased.
- If we skip to a particular position, the clock skips to that note's time.
Eventually based on all these metrics, it provide the song progress which is used by Visualizer
utility to control the visualizations.
So if progress is moving faster, the canvas will be fast. If progress is stopped, the canvas remains at that position etc.
You can assume that the Clock
actually is the time machine for the canvas which we can control.
Our first priority here is making this work on the main thread but in case that doesn't work (highly likely), I think that we can use the same logic in a worker and render on the main canvas after doing the calculations on a offscreen canvas. Might improve the performance even further. For that we can create a OffscreenCanvas in the worker and do all the rendering on it. after everything for a single frame is done, we can draw the image on the main canvas (whose access we already have in the canvas). |
I did some profiling, and I think main thread is busy because of something in Keyboard.ts, the profile showed that the browser is doing a lot in terms of recalc and layout too. Will do a deep dive this weekend. |
I'm trying improve the profile perf on this with some trial & error experiments. I'll let you know when this is ready for review. |
sure, take your time. |
> running for the first time | ||
|
||
``` | ||
yarn build:midi |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is not needed anymore
So I played with this app a bit. To support OffScreenCanvas, I tried using an in-memory canvas (not drawn on the main screen) to draw and copy it to main screen. I think it mostly works, but somehow I see long continuous bars and not exact notes. I'm guessing it's not able to reset the buffer canvas. Attached an image.
Will update this.