Scaled window dimensions off by 1

[wcout]

When setting a scale factor of 1.28 and opening a window with dimensions 800x600 xwininfo reports a size of 1023x767 pixels for the scaled window. This should be 1024x768. Same might be true with positions i.e. open at 800/600 will open at 1023/767.

[fire-eggs]

You really need to tell us what compiler, platform, and compiler options you are using for your app and to build FLTK. The value 1.28 can be exactly represented using IEEE floating point representation (float or double); any other choice of floating point representation will likely result in floating point accuracy issues.

[Albrecht-S]

@fire-eggs Are you sure that "1.28 can be exactly represented using IEEE floating point representation (float or double)"? I don't think so: 0.28 = 28/100 = 7/25 which can't be represented in a binary floating point system. What am I missing?

Or did you mean "can't be exactly represented"? Which would likely explain the potential rounding error.

Puzzled...

[fire-eggs]

Ha ha! I sit corrected!

Microsoft VS2019, using "precise" floating point, 1.28f is 1.2799999714 or thereabouts. [Using 'fast' or 'strict' seems not to make a difference]. So now I'm confused!

The output:

Hello World!
val1: 1.2799999714 val2: 1.2800000000 val3: 1.2799999714
Val1: 1.2799999714
Val2: 1.2800000000
Val3: 1.2799999714

The program:

#include <iostream>
#include <iomanip>
using namespace std;

int main()
{
    std::cout << "Hello World!\n";

    float val1 = 1.28f;
    double val2 = 1.28;
    double val3 = val1;

    printf("val1: %.10f val2: %.10f val3: %.10f\n", val1, val2, val3);

    cout << "Val1: ";
    cout << fixed << setprecision(10) << val1 << endl;
    cout << "Val2: ";
    cout << fixed << setprecision(10) << val2 << endl;
    cout << "Val3: ";
    cout << fixed << setprecision(10) << val3 << endl;
}

[fire-eggs]

In the Google Group, @wcout out states he's on Ubuntu 16.

I get the same results on Ubuntu 20 (g++ 9.3.0). Even with "Full" compliance
(g++ -frounding-math -fsignaling-nans fptest.cxx).

Playing around further, the problem may be with mixing doubles and floats. To wit:
float val = 800.0 * 1.28f; results in 1024.00000000
double val = 800.0 * 1.28f; results in 1023.9999771118
float val = 800.0f * 1.28f; results in 1024.0000000
double val = 800.0f * 1.28f; results in 1024.00000000
double val = 800 * 1.28f; results in 1024.00000000

[wcout]

My opinion is, that the rounding error manifests, when multiplying the scale with positions or dimensions. For me it was unexpected, that a multiplication that leads to an integral value (800*1.28 = 1024) in decimal system is not computed correctly.

My suggestion would be to use a rounding function for these multiplications.
As a test I changedsrc/Fl_x.cxxto use use rint(scale * value) and my 800x600 window now has the correct dimensions 1024x768.
BTW: Running on Linux X11/Xft (Ubuntu 16.04),

This was just a test, a real change would probably be much more work as these scaling multiplications are spread over the code. Also rint() may be not the best function to use, there are others like nearybyint,...
Anyway this were my changes (NOTE: this is with an older FLTK version, line numbers may be wrong):

*** fltk-1.4.x-
--- fltk/src/Fl_x.cxx
***************
*** 2274,2285 ****
      if (is_a_resize) {
        if (!pWindow->resizable()) pWindow->size_range(w(), h(), w(), h());
        if (is_a_move) {
!         XMoveResizeWindow(fl_display, fl_xid(pWindow), X*s, Y*s, W>0 ? W*s : 1, H>0 ? H*s : 1);
        } else {
!         XResizeWindow(fl_display, fl_xid(pWindow), W>0 ? W*s : 1, H>0 ? H*s : 1);
        }
      } else
!       XMoveWindow(fl_display, fl_xid(pWindow), X*s, Y*s);
    }
  }
  
--- 2274,2285 ----
      if (is_a_resize) {
        if (!pWindow->resizable()) pWindow->size_range(w(), h(), w(), h());
        if (is_a_move) {
!         XMoveResizeWindow(fl_display, fl_xid(pWindow), rint(X*s), rint(Y*s), W>0 ? rint(W*s) : 1, H>0 ? rint(H*s) : 1);
        } else {
!         XResizeWindow(fl_display, fl_xid(pWindow), W>0 ? rint(W*s) : 1, H>0 ? rint(H*s) : 1);
        }
      } else
!       XMoveWindow(fl_display, fl_xid(pWindow), rint(X*s), rint(Y*s));
    }
  }
  
***************
*** 2749,2758 ****
    // memset(&hints, 0, sizeof(hints)); jreiser suggestion to fix purify?
    float s = Fl::screen_driver()->scale(screen_num());
  
!   hints->min_width = s*minw();
!   hints->min_height = s*minh();
!   hints->max_width = s*maxw();
!   hints->max_height = s*maxh();
    if (int(s) == s) { // use win size increment value only if scale is an integer. Is it possible to do better?
      hints->width_inc = s*dw();
      hints->height_inc = s*dh();
--- 2749,2758 ----
    // memset(&hints, 0, sizeof(hints)); jreiser suggestion to fix purify?
    float s = Fl::screen_driver()->scale(screen_num());
  
!   hints->min_width = rint(s*minw());
!   hints->min_height = rint(s*minh());
!   hints->max_width = rint(s*maxw());
!   hints->max_height = rint(s*maxh());
    if (int(s) == s) { // use win size increment value only if scale is an integer. Is it possible to do better?
      hints->width_inc = s*dw();
      hints->height_inc = s*dh();
***************
*** 2903,2908 ****
--- 2903,2968 ----
    }
  }

[Albrecht-S]

Yes, technically it would maybe be appropriate to use rounding, but this would not only introduce many, many code changes but also a speed penalty because it's used for all graphics coordinates.

The "problem" with rounding is that it sometimes rounds up and sometimes down. This can be a problem with parallel lines in filled boxes with shades as in the gleam scheme. Integer truncation however always "rounds" down to the next smaller integer which might be an advantage in that case. Note that this is only a theory, I don't know of the exact effects a change would have.

Another option to gain a little more precision would be to use double instead of float everywhere. I'm not sure why float was used at all. Size, speed, whatever?

This is something only @ManoloFLTK can answer.

@fire-eggs Note that in your examples even 1024.00000000 may not be exact that value. It may be slightly smaller or larger. The problem with floating point precision is that the real value may be either larger or smaller by a fraction of the least significant bit. After multiplication and truncation to an int the value above may be either 1023 or 1024 (theoretically, of course, w/o testing).

[Albrecht-S]

@wcout wrote:

For me it was unexpected, that a multiplication that leads to an integral value (800*1.28 = 1024) in decimal system is not computed correctly.

Yes, from the POV of decimal arithmetic it looks unexpected, but as you see the problem lies in the limited floating point precision of the scaling factor 1.28.

[wcout]

Yes I agree it would be a huge change to adapt the drawing code as well, with uncertain outcome.
But perhaps rounding of the window dimensions and positions is less invasive and also a more important use case. For X11 I think all changes would be in Fl_x.cxx (::resize, ::make_xid, ::sendxjunk). I cannot judge though if those changes might conflict with the other scaling operations and various tweaks in the drawing/screen routines.

[ManoloFLTK]

@wcout: could you, please, explain what is the practical consequence, if any, of having a window
sometimes 1 pixel smaller than expected for a given scale factor?
I stress that all quantities manipulated by the program are independent from the scale factor.
As hinted by Albrecht, a window size rounded up could be undrawn in its last pixel. Thus,
opting for the rint() function to compute scaled window sizes would require to reconsider
potentially several other things.

About the use of type float rather than double for the scale factor: I assumed, without checking,
a float was enough to get a 1-pixel accuracy for actual window sizes.

[wcout]

@ManoloFLTK: I have an application with two monitors (each having 1024x768), where the second monitor is configured to extend the screen on the right side. Now two borderless windows of 800x600 are opened at 0/0 for the first monitor and 800/0 for the second (and scaled by FLTK). Due to the rounding error the second window is opened at 1023/0 instead of 1024/0, which disturbs the output of the first monitor on the last pixel column. This seem negligible, but in case of an animation displayed on the second monitor leads to annoying moving pixels on the first monitor.

But I agree, a change to use rounding might open a lot of new potential bugs or drawing problems, as the current code is very well crafted now.
So I'am thinking meanwhile of different ways how to solve my problem...

[wcout]

To put it different: Is it considered inevitable, when a (borderless i.e. without window manager interaction) window is opened at say 20/60 and when that window later queries its coordinates gets reported to be at 19/59? That is what happens currently when there is scaling involved. This can at least cause failure to some unit tests.

[Albrecht-S]

Is it considered inevitable, when a (borderless i.e. without window manager interaction) window is opened at say 20/60 and when that window later queries its coordinates gets reported to be at 19/59?

FYI: Even a borderless window is positioned by the window manager (WM) and, as your question implied, you're aware of the fact that x/y window coordinates are only a request to the WM for these coordinates and subject to be changed by the WM. To verify this for borderless windows I attach a modified test/hello.cxx where the borderless window "requests" position (-10, -10). The output on my Linux Mint system (WM: cinnamon) w/o scaling is:

Window coordinates and sizes are: (0, 0, 340, 180)

Source: hello.cxx.txt

Well, that's kinda nitpicking and doesn't resolve the issue at hand, but anyway...

@wcout : were you able to resolve your positioning problem and can we close this issue now?

[wcout]

To verify this for borderless windows I attach a modified test/hello.cxx where the borderless window "requests" position (-10, -10). The output on my Linux Mint system (WM: cinnamon) w/o scaling is:

Yes, true, some window managers may intercept negative top/left positions, but normally not any decent positive values.

@wcout : were you able to resolve your positioning problem and can we close this issue now?

Not without patching FLTK minimally and additionally tune the positions/dimensions of some windows of our application.
But you can close the issue anyway..

[Albrecht-S]

OTOH: maybe it would be possible (w/o looking at the code actually) to round window positions only when creating windows. As you wrote:

... perhaps rounding of the window [dimensions and] positions is less invasive and also a more important use case.

Rounding window "dimensions" (i.e. width/height) could lead to all sorts of "pixel not drawn inside a window" issues and should certainly not be attempted. But I can see that rounding the positions would solve your (@wcout) issue and be a very limited change. Maybe we could try that ... (?)

Can you tell us what you patched? Maybe this could be helpful. TIA.

[wcout]

Oh, thanks for tackling this issue, I will certainly try, when time permits...

I'm getting a warning under Ubuntu 20.04 when compiling, that is new I think:

Compiling drivers/X11/Fl_X11_Screen_Driver.cxx...
drivers/X11/Fl_X11_Screen_Driver.cxx: In member function ‘virtual Fl_RGB_Image* Fl_X11_Screen_Driver::read_win_rectangle(int, int, int, int, Fl_Window*, bool, bool*)’:
drivers/X11/Fl_X11_Screen_Driver.cxx:726:9: warning: ‘sy’ may be used uninitialized in this function [-Wmaybe-uninitialized]
  726 |     off = a - b;
      |     ~~~~^~~~~~~
drivers/X11/Fl_X11_Screen_Driver.cxx:789:21: note: ‘sy’ was declared here
  789 |     int dx, dy, sx, sy, sw, sh;
      |                     ^~

Certainly it stems from line 821, where sy is passed as first parameter to fl_subimage_offsets But why does it not warn for sx in the line before? Maybe a compiler glitch?

[ManoloFLTK]

There's nothing to worry from that warning because no uninitialized variable is used.

[erco77]

@ManoloFLTK Perhaps I'm missing something, but If win isn't set, won't it skip calling Fl::screen_xywh() causing sy to be uninit'd when used at line 807?

I mean I guess it might not make it past the || in that if() statement (if the compiler parses it left-to-right -- not sure that's required though). Maybe setting the int's to zero would be best tho.

[ManoloFLTK]

The C language guarantees that if A is null, B is not evaluated in
expression (!A || B). By the way, I get no warning myself on Ubuntu 20.04.
That code structure has been there unchanged since at least 2014.

[Albrecht-S]

I've seen warnings in static code analyzers that really surprised me. They can check many, many cases, some of which are maybe "impossible" to be true (for human thinking) but the code analyzers still find a case ...

Manolo is correct about evaluation of logical && and || in expressions in C and C++. A usual case is if (win && win->x() < 0) { ... }. This construct is only correct if the evaluation is guaranteed to be terminated before dereferencing win if win == NULL (and it is).

However, in that particular case (see @erco77's screenshot) I believe the point is that the compiler doesn't "know" that Fl::screen_xyhw() always returns a value in sx, sy, sw, and sh (if this is not the case for instance if the screen number is out of range then this is a fault). I've seen this in similar constructs with values returned by reference arguments. In this case the first sx *= s; statement would access an uninitialized variable (sx) if win != NULL.

It's probably wise to initialize these variables with 0 (zero) even if it's not really necessary (defensive programming).

[Albrecht-S]

@wcout What compiler and which switches/options are you using to compile?

[wcout]

Everything default (make without arguments), using g++. Ubuntu 20.04 is updated completley.

g++ --version gives:

gcc (Ubuntu 9.3.0-17ubuntu1~20.04) 9.3.0
Copyright (C) 2019 Free Software Foundation, Inc.

Disabling silent mode shows:

Compiling drivers/X11/Fl_X11_Screen_Driver.cxx...
g++ -I..   -I../jpeg  -Os -Wall -Wunused -Wno-format-y2k  -fno-exceptions -fno-strict-aliasing -ffunction-sections -fdata-sections -D_LARGEFILE_SOURCE -D_LARGEFILE64_SOURCE -D_THREAD_SAFE -D_REENTRANT -I/usr/include/uuid -I/usr/include/freetype2 -I/usr/include/libpng16  -I/usr/include/uuid -I/usr/include/freetype2 -I/usr/include/libpng16   -DFL_LIBRARY -c drivers/X11/Fl_X11_Screen_Driver.cxx -o drivers/X11/Fl_X11_Screen_Driver.o
drivers/X11/Fl_X11_Screen_Driver.cxx: In member function ‘virtual Fl_RGB_Image* Fl_X11_Screen_Driver::read_win_rectangle(int, int, int, int, Fl_Window*, bool, bool*)’:
drivers/X11/Fl_X11_Screen_Driver.cxx:726:9: warning: ‘sy’ may be used uninitialized in this function [-Wmaybe-uninitialized]
  726 |     off = a - b;
      |     ~~~~^~~~~~~
drivers/X11/Fl_X11_Screen_Driver.cxx:789:21: note: ‘sy’ was declared here
  789 |     int dx, dy, sx, sy, sw, sh;
      |                     ^~

Maybe @ManoloFLTK does not use optimization? Or using cmake?

[ManoloFLTK]

I used the default optimization setting, that is -Os

The code in Greg's screenshot is as before the commit that triggered the warning.
That commit removed the call to Fl::screen_xyhw() with arguments by reference and
replaced it by plain assignments:
sx = screens[ns].x_org;
It's therefore curious this change triggered a warning.

Anyway, I agree defensive programming is the way to go.

[wcout]

I think the initially reported problems have been addressed and fixed meanwhile by using rounding.
So this issue could be closed.
Thanks.

[Albrecht-S]

Thanks for confirmation. Closing ...