From 3622895f09d7c6f6a3d608031c790750f995327b Mon Sep 17 00:00:00 2001 From: Ben Whittaker Date: Tue, 28 Sep 2021 22:16:34 -0400 Subject: [PATCH] Layout: avoid floating point numbers The previous approach involved calculating the width of each element independently, and deriving the `x` coordinate of each element by summing the previous widths. However, if the widths are rounded, summing them leads to accumulated rounding errors. This change effectively takes the alternate approach of calculating the `x` coordinate of each element independently, and then deriving the widths from the differences between adjacent values of `x`. As a bonus, this change corrects a couple issues in the `padding_with_fill` test. --- modules/Layout.js | 44 +++++++++++++---------- tests/Layout/tests/filly_issue820.bmp | Bin 15562 -> 15562 bytes tests/Layout/tests/padding_with_fill.bmp | Bin 15562 -> 15562 bytes 3 files changed, 26 insertions(+), 18 deletions(-) diff --git a/modules/Layout.js b/modules/Layout.js index 9f3a805be..53447c503 100644 --- a/modules/Layout.js +++ b/modules/Layout.js @@ -239,29 +239,37 @@ Layout.prototype.layout = function (l) { // exw,exh = extra width/height available switch (l.type) { case "h": { - var x = l.x + (0|l.pad); + var acc_w = l.x + (0|l.pad); + var accfillx = 0; var fillx = l.c && l.c.reduce((a,l)=>a+(0|l.fillx),0); - if (!fillx) { x += (l.w-l._w)/2; } + if (!fillx) { acc_w += (l.w-l._w)>>1; } + var x = acc_w; l.c.forEach(c => { - c.w = c._w + ((0|c.fillx)*(l.w-l._w)/(fillx||1)); - c.h = c.filly ? l.h - (l.pad<<1) : c._h; - c.x = x; - c.y = l.y + (0|l.pad) + (1+(0|c.valign))*(l.h-(l.pad<<1)-c.h)/2; - x += c.w; + c.x = 0|x; + acc_w += c._w; + accfillx += 0|c.fillx; + x = acc_w + Math.floor(accfillx*(l.w-l._w)/(fillx||1)); + c.w = 0|(x - c.x); + c.h = 0|(c.filly ? l.h - (l.pad<<1) : c._h); + c.y = 0|(l.y + (0|l.pad) + ((1+(0|c.valign))*(l.h-(l.pad<<1)-c.h)>>1)); if (c.c) this.layout(c); }); break; } case "v": { - var y = l.y + (0|l.pad);; + var acc_h = l.y + (0|l.pad); + var accfilly = 0; var filly = l.c && l.c.reduce((a,l)=>a+(0|l.filly),0); - if (!filly) { y += (l.h-l._h)/2 } + if (!filly) { acc_h += (l.h-l._h)>>1; } + var y = acc_h; l.c.forEach(c => { - c.w = c.fillx ? l.w - (l.pad<<1) : c._w; - c.h = c._h + ((0|c.filly)*(l.h-l._h)/(filly||1)); - c.y = y; - c.x = l.x + (0|l.pad) + (1+(0|c.halign))*(l.w-(l.pad<<1)-c.w)/2; - y += c.h; + c.y = 0|y; + acc_h += c._h; + accfilly += 0|c.filly; + y = acc_h + Math.floor(accfilly*(l.h-l._h)/(filly||1)); + c.h = 0|(y - c.y); + c.w = 0|(c.fillx ? l.w - (l.pad<<1) : c._w); + c.x = 0|(l.x + (0|l.pad) + ((1+(0|c.halign))*(l.w-(l.pad<<1)-c.w)>>1)); if (c.c) this.layout(c); }); break; @@ -288,8 +296,8 @@ Layout.prototype.update = function() { if (l.r&1) { // rotation var t = l._w;l._w=l._h;l._h=t; } - l._w = Math.max(l._w + (l.pad<<1), 0|l.width); - l._h = Math.max(l._h + (l.pad<<1), 0|l.height); + l._w = 0|Math.max(l._w + (l.pad<<1), 0|l.width); + l._h = 0|Math.max(l._h + (l.pad<<1), 0|l.height); } var cb = { "txt" : function(l) { @@ -349,8 +357,8 @@ Layout.prototype.update = function() { } else { l.w = l._w; l.h = l._h; - l.x = (w-l.w)/2; - l.y = y+(h-l.h)/2; + l.x = (w-l.w)>>1; + l.y = y+((h-l.h)>>1); } // layout children this.layout(l); diff --git a/tests/Layout/tests/filly_issue820.bmp b/tests/Layout/tests/filly_issue820.bmp index 886ba97d8967b1211ae7eb43c8f5bc5cf7cc55a7..adfe9164ef3a4f2d98a312d7f88857b26662606e 100644 GIT binary patch delta 179 zcmX?Ad8%^5ZjH$o#T+IVD6vgmsUbAEK$K;2si-ZGW}O_ViNR<5KRH1Ns2U;*;{o;V z(h_9(&%nSxIZy_u86p5R%o!vJW?vAo1JYtZZ3l%NK`e+CMp@U%2L$+0I6yUtLg7$d PA(I0&f;I=}y7B`6mx()H delta 197 zcmX?Ad8%^5ZjH$WN-UF2By4~*+vJrRLO_~zvZ@%6FUmSOQWJyE%Kw3ZfqyciEL;V$ z2+)jOT7pdcA0{6Zwg(#t c$p-}Z!Frac`vcjmjQo=yN;q#0&<*AX0H-B9ng9R* diff --git a/tests/Layout/tests/padding_with_fill.bmp b/tests/Layout/tests/padding_with_fill.bmp index 15e05b0d980d41eb76f4bc70456126994e45bf63..2c785bbc03cb5da5dad6ba34c80debce9ae9b1a3 100644 GIT binary patch delta 55 zcmV-70LcH!dCGaPjRdph1Xl!;Tmo3LJPE85vp^lQ1GAeTYXXzNAy~5pBjN(HKqjOE Nvy3Tj1G7>scn^Lw6+Qp} delta 67 zcmV-J0KEUodCGaPjRcb*0yC4q1SqqR1XKjGJPE85vydDM1hexX69ThdBjN&+&?7IC ZZzh7XnI=mEvpgym1CziZShG?scn@!C7`y-g