Scale graph canvas based on DPI factor

[EHfive]

Similar to fixes in litegraph.js editor demo: https://github.com/ernestp/litegraph.js/blob/3ef215cf11b5d38cc4f7062d6f78b478e2f02b39/editor/js/code.js#L19-L28

Fixes #161

[EHfive]

Noticed you have reverted a similar DPI fix previously https://github.com/comfyanonymous/ComfyUI/commit/4796e615dd7faad38429fc8e716e3a817a28c526.

Revert DPI fix since it caused more issues than it solved.

I assume "more issues" means the canvas would still be blurry with that fix. I guess the issue is the previous patch uses el.offsetWidth and el.offsetHeight, which are rounded integers, causing scaled width & height to be inaccurate.

Hence in this patch, width and height are get from more accurate getBoundingClientRect() in float number form. And in the last Math.round() are applied to scaled width & height otherwise the decimal part would be implicitly dropped after setting to canvas element.

If this still causes issues in your end, than at least a toggle should be giving to opt-in/out DPI scaling as it's critical to HIDPI screen user.

[comfyanonymous]

The issue with this is that it breaks zooming and unzooming in the browser. When unzooming the canvas only takes part of the window and when zooming you no longer see the fps counter on the bottom left.

[EHfive]

OK, I see the problem here.

litegraph.js internally uses canvas.width & canvas.height as its absolute right & bottom viewport bounds, and the fps counter is positioned to its left-bottom bound. So lets say if we scale the canvas by 2x, the internal viewport would actually be 2x2 times larger than browser viewport, thus the fps counter became invisible.

The "unzooming" problem can be work around by limit the minimal scale to 1.

- const scale = window.devicePixelRatio;
+ const scale = Math.max(window.devicePixelRatio, 1);

And there is a ad-hoc fix for "fps counter" positioning.

diff --git a/web/lib/litegraph.core.js b/web/lib/litegraph.core.js
index a60848d..05f52ce 100644
--- a/web/lib/litegraph.core.js
+++ b/web/lib/litegraph.core.js
@@ -8148,7 +8148,7 @@ LGraphNode.prototype.executeAction = function(action)
      **/
     LGraphCanvas.prototype.renderInfo = function(ctx, x, y) {
         x = x || 10;
-        y = y || this.canvas.height - 80;
+        y = y || this.canvas.offsetHeight - 80;
 
         ctx.save();
         ctx.translate(x, y);

I guess the issue should be addressed in upstream first to make litegraph.js window.devicePixelRatio aware: https://github.com/jagenjo/litegraph.js/issues/181

But I can push a v3 patch if it's OK for you to include the workaround above, though the viewport problem in litegraph.js would still remains.

edit1: Note the renderInfo fix can also be implemented by overriding and wrapping the original LGraphCanvas.prototype.renderInfo in app.js to ease upgrading of litegraph.core.js.

edit2: Pushed the updated patch anyway, it can be useful for try-out and/or reviewing.

[comfyanonymous]

Well I merged it: https://github.com/comfyanonymous/ComfyUI/commit/cef30cc6b66fc6b081d3976a547f8abc56a4c7ae

[EHfive]

Fine, I just fixed a typo in the last version: "viewpoint" -> "viewport".

Though I personally would prefer to use overriding method, but it's your choice.

Anyway, thanks for making progress.