Clean up viewport handling

Make sure the viewport is properly updated when necessary, on respects
given restrictions.

app/ui.js

@@ -1187,22 +1187,7 @@ var UI;
             if (new_clip && size) {
                 // When clipping is enabled, the screen is limited to
                 // the size of the browser window.
-
+                display.viewportChangeSize(size.w, size.h);
-                var screen = document.getElementById('noVNC_screen');
-                var canvas = document.getElementById('noVNC_canvas');
-
-                // Hide potential scrollbars that can skew the position
-                screen.style.overflow = "hidden";
-
-                // The x position marks the left margin of the canvas,
-                // remove the margin from both sides to keep it centered.
-                var new_w = size.w - (2 * Util.getPosition(canvas).x);
-
-                screen.style.overflow = "visible";
-
-                display.viewportChangeSize(new_w, size.h);
-            } else {
-                display.viewportChangeSize();
             }
         },
 

core/display.js

@@ -26,9 +26,6 @@
     this._fb_width = 0;
     this._fb_height = 0;
 
-    // the visible "physical canvas" viewport
-    this._viewportLoc = { 'x': 0, 'y': 0, 'w': 0, 'h': 0 };
-
     this._prevDrawStyle = "";
     this._tile = null;
     this._tile16x16 = null;
@@ -61,6 +58,9 @@
 
     this._targetCtx = this._target.getContext('2d');
 
+    // the visible canvas viewport (i.e. what actually gets seen)
+    this._viewportLoc = { 'x': 0, 'y': 0, 'w': this._target.width, 'h': this._target.height };
+
     // The hidden canvas, where we do the actual rendering
     this._backbuffer = document.createElement('canvas');
     this._drawCtx = this._backbuffer.getContext('2d');
@@ -156,31 +156,39 @@
 
         viewportChangeSize: function(width, height) {
 
-            if (typeof(width) === "undefined" || typeof(height) === "undefined") {
+            if (!this._viewport ||
+                typeof(width) === "undefined" ||
+                typeof(height) === "undefined") {
 
                 Util.Debug("Setting viewport to full display region");
                 width = this._fb_width;
                 height = this._fb_height;
             }
 
+            if (width > this._fb_width) {
+                width = this._fb_width;
+            }
+            if (height > this._fb_height) {
+                height = this._fb_height;
+            }
+
             var vp = this._viewportLoc;
             if (vp.w !== width || vp.h !== height) {
                 vp.w = width;
                 vp.h = height;
 
                 var canvas = this._target;
-                if (canvas.width !== width || canvas.height !== height) {
+                canvas.width = width;
-                    if (canvas.width !== width) {
+                canvas.height = height;
-                        canvas.width = width;
+
-                        canvas.style.width = width + 'px';
+                // The position might need to be updated if we've grown
-                    }
+                this.viewportChangePos(0, 0);
-                    if (canvas.height !== height) {
+
-                        canvas.height = height;
+                this._damage(vp.x, vp.y, vp.w, vp.h);
-                        canvas.style.height = height + 'px';
+                this.flip();
-                    }
+
-                    this._damage(vp.x, vp.y, vp.w, vp.h);
+                // Update the visible size of the target canvas
-                    this.flip();
+                this._rescale(this._scale);
-                }
             }
         },
 
@@ -219,9 +227,11 @@
                 }
             }
 
-            this._rescale(this._scale);
+            // Readjust the viewport as it may be incorrectly sized
-
+            // and positioned
-            this.viewportChangeSize();
+            var vp = this._viewportLoc;
+            this.viewportChangeSize(vp.w, vp.h);
+            this.viewportChangePos(0, 0);
         },
 
         // Track what parts of the visible canvas that need updating
@@ -547,6 +557,14 @@
             this._rescale(scale);
         },
 
+        set_viewport: function (viewport) {
+            this._viewport = viewport;
+            // May need to readjust the viewport dimensions
+            var vp = this._viewportLoc;
+            this.viewportChangeSize(vp.w, vp.h);
+            this.viewportChangePos(0, 0);
+        },
+
         get_width: function () {
             return this._fb_width;
         },

tests/test.display.js

@@ -89,6 +89,20 @@ describe('Display/Canvas Helper', function () {
             expect(display._target.height).to.equal(2);
         });
 
+        it('should move the viewport if necessary', function() {
+            display.viewportChangeSize(5, 5);
+            expect(display.absX(0)).to.equal(0);
+            expect(display.absY(0)).to.equal(0);
+            expect(display._target.width).to.equal(5);
+            expect(display._target.height).to.equal(5);
+        });
+
+        it('should limit the viewport to the framebuffer size', function() {
+            display.viewportChangeSize(6, 6);
+            expect(display._target.width).to.equal(5);
+            expect(display._target.height).to.equal(5);
+        });
+
         it('should redraw when moving the viewport', function () {
             display.flip = sinon.spy();
             display.viewportChangePos(-1, 1);
@@ -111,13 +125,40 @@ describe('Display/Canvas Helper', function () {
             var clipping = display.clippingDisplay();
             expect(clipping).to.be.false;
         });
+
+        it('should show the entire framebuffer when disabling the viewport', function() {
+            display.set_viewport(false);
+            expect(display.absX(0)).to.equal(0);
+            expect(display.absY(0)).to.equal(0);
+            expect(display._target.width).to.equal(5);
+            expect(display._target.height).to.equal(5);
+        });
+
+        it('should ignore viewport changes when the viewport is disabled', function() {
+            display.set_viewport(false);
+            display.viewportChangeSize(2, 2);
+            display.viewportChangePos(1, 1);
+            expect(display.absX(0)).to.equal(0);
+            expect(display.absY(0)).to.equal(0);
+            expect(display._target.width).to.equal(5);
+            expect(display._target.height).to.equal(5);
+        });
+
+        it('should show the entire framebuffer just after enabling the viewport', function() {
+            display.set_viewport(false);
+            display.set_viewport(true);
+            expect(display.absX(0)).to.equal(0);
+            expect(display.absY(0)).to.equal(0);
+            expect(display._target.width).to.equal(5);
+            expect(display._target.height).to.equal(5);
+        });
     });
 
     describe('resizing', function () {
         var display;
         beforeEach(function () {
-            display = new Display({ target: document.createElement('canvas'), prefer_js: false, viewport: true });
+            display = new Display({ target: document.createElement('canvas'), prefer_js: false, viewport: false });
-            display.resize(4, 3);
+            display.resize(4, 4);
         });
 
         it('should change the size of the logical canvas', function () {
@@ -126,14 +167,8 @@ describe('Display/Canvas Helper', function () {
             expect(display._fb_height).to.equal(7);
         });
 
-        it('should update the viewport dimensions', function () {
-            sinon.spy(display, 'viewportChangeSize');
-            display.resize(2, 2);
-            expect(display.viewportChangeSize).to.have.been.calledOnce;
-        });
-
         it('should keep the framebuffer data', function () {
-            display.fillRect(0, 0, 4, 3, [0, 0, 0xff]);
+            display.fillRect(0, 0, 4, 4, [0, 0, 0xff]);
             display.resize(2, 2);
             display.flip();
             var expected = [];
@@ -144,6 +179,38 @@ describe('Display/Canvas Helper', function () {
             }
             expect(display).to.have.displayed(new Uint8Array(expected));
         });
+
+        describe('viewport', function () {
+            beforeEach(function () {
+                display.set_viewport(true);
+                display.viewportChangeSize(3, 3);
+                display.viewportChangePos(1, 1);
+            });
+
+            it('should keep the viewport position and size if possible', function () {
+                display.resize(6, 6);
+                expect(display.absX(0)).to.equal(1);
+                expect(display.absY(0)).to.equal(1);
+                expect(display._target.width).to.equal(3);
+                expect(display._target.height).to.equal(3);
+            });
+
+            it('should move the viewport if necessary', function () {
+                display.resize(3, 3);
+                expect(display.absX(0)).to.equal(0);
+                expect(display.absY(0)).to.equal(0);
+                expect(display._target.width).to.equal(3);
+                expect(display._target.height).to.equal(3);
+            });
+
+            it('should shrink the viewport if necessary', function () {
+                display.resize(2, 2);
+                expect(display.absX(0)).to.equal(0);
+                expect(display.absY(0)).to.equal(0);
+                expect(display._target.width).to.equal(2);
+                expect(display._target.height).to.equal(2);
+            });
+        });
     });
 
     describe('rescaling', function () {
@@ -152,7 +219,9 @@ describe('Display/Canvas Helper', function () {
 
         beforeEach(function () {
             display = new Display({ target: document.createElement('canvas'), prefer_js: false, viewport: true });
-            display.resize(4, 3);
+            display.resize(4, 4);
+            display.viewportChangeSize(3, 3);
+            display.viewportChangePos(1, 1);
             canvas = display.get_target();
             document.body.appendChild(canvas);
         });
@@ -162,15 +231,25 @@ describe('Display/Canvas Helper', function () {
         });
 
         it('should not change the bitmap size of the canvas', function () {
-            display.set_scale(0.5);
+            display.set_scale(2.0);
-            expect(canvas.width).to.equal(4);
+            expect(canvas.width).to.equal(3);
             expect(canvas.height).to.equal(3);
         });
 
         it('should change the effective rendered size of the canvas', function () {
-            display.set_scale(0.5);
+            display.set_scale(2.0);
-            expect(canvas.clientWidth).to.equal(2);
+            expect(canvas.clientWidth).to.equal(6);
-            expect(canvas.clientHeight).to.equal(2);
+            expect(canvas.clientHeight).to.equal(6);
+        });
+
+        it('should not change when resizing', function () {
+            display.set_scale(2.0);
+            display.resize(5, 5);
+            expect(display.get_scale()).to.equal(2.0);
+            expect(canvas.width).to.equal(3);
+            expect(canvas.height).to.equal(3);
+            expect(canvas.clientWidth).to.equal(6);
+            expect(canvas.clientHeight).to.equal(6);
         });
     });