Refactor client canvas

[gzdunek]

The main issue with the current architecture was that TdpClientCanvas had too many responsibilities. It subscribed to all event listeners, including those unrelated to rendering. Additionally, because event subscription happened within TdpClientCanvas, any work needed at the parent level required passing even more callbacks down to the component.

To address this, I extracted a new component dedicated solely to rendering frames. This component no longer subscribes to event listeners from the parent. Instead, it exposes an imperative API that the parent can interact with. In React, this is achieved using useImperativeHandle combined with forwardRef. Thanks to it, places that use the canvas have much more flexibility.

The exposed API looks like below:

export interface TdpClientCanvasRef {
  setPointer(pointer: Pointer): void;
  renderPngFrame(frame: PngFrame): void;
  renderBitmapFrame(frame: BitmapFrame): void;
  setResolution(resolution: { width: number; height: number }): void;
  clear(): void;
  focus(): void;
}

While refactoring the component, I shipped some improvements:

• Fixed focusing the canvas. Before it happened too early and the MFA dialog was stealing it.
• Scaling the canvas now happens entirely in CSS (mainly thanks to object-fix: 'scale-down). This frees us from calculating the canvas size in the player and fixes a bug where the recording could have lost its proportions.
• We now listen for changes in the canvas size instead of the window size. It eliminates the need to subtract the top bar size from it (which would be problematic in Connect).

Please review the PR commit by commit.

changelog: Resolved an issue where desktop session recordings could have incorrect proportions

[gzdunek]

This is no longer needed thanks to the CSS property object-fit: 'scale-down'. Please note that the scaling behavior has changed a bit. Previously, if the browser window was larger than the recording, we scaled the recording up. I'm not sure if it's the right thing to do, it could result in a low quality, pixelated image in some scenarios.
Instead, now the physical size of a recording won't exceed its resolution. I don't know if the previous behavior was a bug or a feature, but I can bring it back with a different value of object-fit :)

Also, a quick preview of a bug that we had before, caused by manual scaling:

before.mov

[avatus]

I'll have to double check, but I think there is also a usecase where we don't want the canvas to change resolution at all in order to preserve some special aspect ratio on the remote device. however, since that wasn't a thing on the client side before (i think) then its still probably handled on the server side.

i could also be misremembering anyway, so I'll look for it

[gzdunek]

I believe the new implementation should be no worse than the previous one at handling it, but maybe I'm missing something. If you will have more details about it please let me know!

[gzdunek]

All these effects are really verbose. In a separate PR I will refactor them to something like:

  useListener(
    client.onWsClose,
    useCallback(message => {
      setTdpConnection({ status: 'closed', statusText: message });
    }, [])
  );

[gzdunek]

Since now it's a canvas that can render anything, I will rename it to CanvasRenderer in the future.

[avatus]

it works really well for my locally. most the code refactor is good so far (and similar to the direction I was going) but I didn't finish the review. will continue tomorrow morning.

[avatus]

in avatus/desktop-simplify, i moved way from TdpClient from emitting events that then needed to be listened to, to a optional callback in the constructor instead (like this). I don't have the strongest opinion one way or another (especially after your changes here), but what are you opinions on that pattern over the one you have here?