reentrant calls with Fl_Window::resize

[Evan Laforge]

I was tracking down a bug and it turned out to be caused when I call
resize() on a window "from the outside" (so it's my program doing it,
rather than the OS doing it via a window drag). On OS X (with fltk
head), it turns out that each call to resize() causes it to be called
twice, reentrantly. I tried it on X11 (with fltk at 3bb34), and the
reentrant call doesn't happen, but resize gets called 4 times.

I'm not sure if this is a bug or not, but it is a bit odd, and it did
cause a bug in my program, which wasn't expecting reentrant calls.
I'm guessing it has to do with how the window system might feel the
need to tweak a resize, for instance on OS X it won't let you overlap
the menu bar. I'm not sure what a more straightforward approach here
would be, perhaps it would be not to call Fl_Window::resize directly,
but to ask the OS to move the window (how though?), and then the
relevant OS-specific driver will then call the resize with the numbers
it approves of.

Here's a short program to demonstrate the behaviour:

#include <iostream>
#include <FL/Fl_Window.H>
#include <FL/Fl_Box.H>
#include <FL/fl_draw.H>
#include <FL/Fl.H>

class Bug : public Fl_Window {
public:
Bug(int x, int y, int w, int h) : Fl_Window(x, y, w, h), box(0, 0, w, h)
{
box.box(FL_FLAT_BOX);
box.color(FL_WHITE);
}

void resize(int x, int y, int w, int h) override {
static int nest;
nest++;
std::cout << "resize " << nest << "\n";
Fl_Window::resize(x, y, w, h);
nest--;
}

Fl_Box box;
};


Fl_Window *window;

static void
timeout_func(void *unused)
{
window->resize(0, 0, 200, 200);
}

int main()
{
Bug win(200, 200, 200, 200);
window = &win;
win.show();
Fl::add_timeout(1, timeout_func, nullptr);
Fl::run();
return 0;
}

[Albrecht Schlosser]

On 11/22/21 7:46 AM Evan Laforge wrote:
> I was tracking down a bug and it turned out to be caused when I call
> resize() on a window "from the outside" (so it's my program doing it,
> rather than the OS doing it via a window drag). On OS X (with fltk
> head), it turns out that each call to resize() causes it to be called
> twice, reentrantly. I tried it on X11 (with fltk at 3bb34), and the
> reentrant call doesn't happen, but resize gets called 4 times.

I can confirm this behavior.

> I'm not sure if this is a bug or not, but it is a bit odd, and it did
> cause a bug in my program, which wasn't expecting reentrant calls.

I believe it is a bug. If your derived class calls Fl_Window::resize()
in its own resize() method it should not be called back by this. In the
theory you are supposed to *react* on the resize request sent by the
system. If you call Fl_Window::resize() (the base class) this *should*
confirm the resize request and not send another one.

I could track it down to a point (actually two) in src/Fl_cocoa.mm but
I'm not sure it this is the right fix.

@Manolo: I'm attaching my modified test program and a potential
workaround in the diff file. This should help to track down the problem.
TIA.

Note: I'm aware that this is very likely not the correct solution, I
assume you may find a better one.

[Albrecht Schlosser]

On 11/22/21 4:17 PM Albrecht Schlosser wrote:
> On 11/22/21 7:46 AM Evan Laforge wrote:
>> I was tracking down a bug and it turned out to be caused when I call
>> resize() on a window "from the outside" (so it's my program doing it,
>> rather than the OS doing it via a window drag).  On OS X (with fltk
>> head), it turns out that each call to resize() causes it to be called
>> twice, reentrantly.  I tried it on X11 (with fltk at 3bb34), and the
>> reentrant call doesn't happen, but resize gets called 4 times.
>
> I can confirm this behavior.
>
>> I'm not sure if this is a bug or not, but it is a bit odd, and it did
>> cause a bug in my program, which wasn't expecting reentrant calls.
>
> I believe it is a bug. If your derived class calls Fl_Window::resize()
> in its own resize() method it should not be called back by this. In
> the theory you are supposed to *react* on the resize request sent by
> the system. If you call Fl_Window::resize() (the base class) this
> *should* confirm the resize request and not send another one.
>
> I could track it down to a point (actually two) in src/Fl_cocoa.mm but
> I'm not sure it this is the right fix.

Addition: maybe the following call to resize()

window->resize(X, Y, lround(r.size.width/s), lround(r.size.height/s));

in line ~1246 of Fl_cocoa.mm needs to be qualified with Fl_Window:: ?

window->Fl_Window::resize(X, Y, lround(r.size.width/s),
lround(r.size.height/s));

This would avoid calling back into derived classes but again, I'm not sure.

[Manolo]

Many paths through the code are necessary to support

-resize by the user with the mouse

-program-asked resize

-GUI rescaling

-sub-windows

-OS control of admissible window sizes and positions

-fullscreen button of the title bar

I attach what I believe is a fix:

[Bill Spitzak]

I seem to remember adding quite a few checks to see if it was resizing to the size it already was in order to reduce this.

--
You received this message because you are subscribed to the Google Groups "fltk.coredev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to fltkcoredev...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/fltkcoredev/07338395-0dc9-481d-b092-5fbb85442d2bn%40googlegroups.com.

[Albrecht Schlosser]

Great!

This fixes the issue in my tests.

[imacarthur]

On Monday, 22 November 2021 at 18:11:45 UTC Bill wrote:

I seem to remember adding quite a few checks to see if it was resizing to the size it already was in order to reduce this.

FWIW, when I saw this post originally, I had a vague sense that, at some point in the distant past (which I can now find no evidence for...) there was an investigation into recursive calls to resize() and so forth. My recollection, such as it is, was that there was also a platform related aspect to it (i.e. X11 behaved differently to Win32, etc.)

I have no idea what the outcome was, and Bill thinks checks were added to address this.

I also have no idea if this "new" report is actually the same thing or some new/changed behaviour anyway...

[Greg Ercolano]

On 11/23/21 12:29 AM, imacarthur wrote:

On Monday, 22 November 2021 at 18:11:45 UTC Bill wrote:

I seem to remember adding quite a few checks to see if it was resizing to the size it already was in order to reduce this.

FWIW, when I saw this post originally, I had a vague sense that, at some point in the distant past (which I can now find no evidence for...) there was an investigation into recursive calls to resize() and so forth. My recollection, such as it is, was that there was also a platform related aspect to it (i.e. X11 behaved differently to Win32, etc.)

    Sounds familiar -- may have been an STR, as sometimes those threads could
    get really long (esp. if I was involved, lol).

    There are a few deep rabbit holes; (a) following events up and down the widget
    hierarchy was one, and (b) tracking down resize() calls in and out of the window
    manager was another.

    Some of that behavior was even window manager specific, due to how it could
    redefine/restrict/retrigger resize behavior. Also, the app itself in trying to reopen
    itself at the same position/size it had been in when it last closed, might go through
    a few resizes, one opening at the default size, then resizing after loading its previous
    config.

[Bill Spitzak]

The code must be checked to make sure the size fields are updated before any calls that might cause a recursive resize are done. And then checks must be added before sending resizes to ignore them if the size is already correct. I suspect mostly that there has been some rearrangement so that the fields are not updated early enough.

--

You received this message because you are subscribed to the Google Groups "fltk.coredev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to fltkcoredev...@googlegroups.com.

To view this discussion on the web visit https://groups.google.com/d/msgid/fltkcoredev/1c4c3c57-a6c7-953c-cb33-0f41be74e805%40seriss.com.

[Manolo]

This should now be fixed in the git repository (7e484c6 ).

[Evan Laforge]

I just tested it on OS X and my workaround is no longer necessary
after that commit. Thanks for the quick turnaround!

On Wed, Nov 24, 2021 at 1:37 AM Manolo wrote:
>
> This should now be fixed in the git repository (7e484c6 ).
>

> --
> You received this message because you are subscribed to the Google Groups "fltk.coredev" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to fltkcoredev...@googlegroups.com.

> To view this discussion on the web visit https://groups.google.com/d/msgid/fltkcoredev/da2d9427-29ea-413a-afbe-e62f5121d28an%40googlegroups.com.

[Gonzalo Garramuno]

It seems the fix added by Manolo on 7e484c614c16f0a6580ba2c2efaa654ab4e08082 breaks my windows scaling in OpenGL when there’s a fullscreen toggle. The difference from the normal demos is that my GL Window is under two Packs — a slightly modified version of them by Greg -- (one vertical and one horizontal).

—
Gonzalo Garramuno
ggar...@gmail.com

[Manolo]

Le lundi 6 décembre 2021 à 01:35:38 UTC+1, ggar...@gmail.com a écrit :

… breaks my windows scaling in OpenGL when there’s a fullscreen toggle.

@Gonzalo: could you, please, explain in detail what this means?

[Gonzalo Garramuno]

Sure. My image viewer uses two Fl_Packs on top of each other (one vertical and one horizontal). The view window is an OpenGL window which is a subgroup of those Fl_Packs. The main window contains those Fl_Packs together with some other widgets. In my viewer, on user feedback, I send a fullscreen call ( fltk_main->fullscreen() ) to the main window, which resizes the Fl_Packs and thus also the OpenGL window (all of which have the resizable setting on). Previous to the patch all worked properly. With the patch the OpenGL window resizes incorrectly (it is zoomed. and the shown image is offset off screen). And, removing the fullscreen with fullscreen_off, does not return the windows to their original position, as they did before the patch. I also tried fullscreen without any of the other widgets and the OpenGL window is also off.

—
Gonzalo Garramuno
ggar...@gmail.com

[Gonzalo Garramuno]

It is as the gluOrtho call gets corrupted or the texture square that holds the image gets scaled incorrectly. However, I checked both things and both are correct.

—
Gonzalo Garramuno
ggar...@gmail.com

[Gonzalo Garramuno]

I found out the lines that break my program in Fl_cocoa.mm.

@@ -1249,9 +1253,6 @@ - (void)view_did_resize:(NSNotification *)notif
float s = Fl::screen_driver()->scale(window->screen_num());
NSRect r = [view frame];
Fl_Cocoa_Window_Driver::driver(window)->view_resized(1);
- if (Fl_Cocoa_Window_Driver::driver(window)->through_resize())
- Fl_Cocoa_Window_Driver::driver(window)->resize(X, Y, lround(r.size.width/s), lround(r.size.height/s));
- else

window->resize(X, Y, lround(r.size.width/s), lround(r.size.height/s));

Fl_Cocoa_Window_Driver::driver(window)->view_resized(0);

---
Gonzalo Garramuno
ggar...@gmail.com

[Manolo]

OK. I see it. It occurs also with the demo program test/shape when clicking on the green fullscreen button.

I believe it's now fixed in the git repo at 939d536 .

@Gonzalo: please, report what you see with your app.

[Gonzalo Garramuno]

It seems it fixed the problems in my application. Thank you, Manolo. You are the best!

—
Gonzalo Garramuno
ggar...@gmail.com