[Lazarus] Patch for fixing the window resizes when switching components

classic Classic list List threaded Threaded
14 messages Options
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[Lazarus] Patch for fixing the window resizes when switching components

Free Pascal - Lazarus mailing list
Hi all,

So i finally figured out why this happens and the reason is the ShowWindow(Handle, SW_SHOWNORMAL) call made when selecting components (this is done to activate the designer form).

Under Gtk2 this call always calls gtk_window_unmaximize (among others) but Gtk2 seems to have a bug that always tries to unmaximize the window regardless of its status - with some window managers (like Window Maker) this can cause the window to be resized. The bug is most likely with Gtk2 not ignoring the call for unmaximized windows, but since Gtk2's development has practically stopped (most recent changes are documentation and build fixes) and since Lazarus needs to work with 10+ year old versions of Gtk2, this needs to be addressed on the LCL side.

Fortunately LCL already tracks the window state in the WindowState property, so at least for TCustomForm descendants (which would be almost all cases this is needed - 100% of the cases in design time where the bug is mostly visible) we can workaround this by not calling gtk_window_unmaximize for forms that aren't maximized.

The description and patch can be found in Mantis here, as well as a video that shows the issue:

https://bugs.freepascal.org/view.php?id=31832

The patch was made against SVN but should apply with 1.8RC1 since i originally wrote it there.

Kostas

--
_______________________________________________
Lazarus mailing list
[hidden email]
http://lists.lazarus-ide.org/listinfo/lazarus
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [Lazarus] Patch for fixing the window resizes when switching components

Free Pascal - Lazarus mailing list
I see the patch was added and Lazarus seems to work fine now, but the check was altered a little. The current check assumes that the Control will never be nil, but is this the case? I ask because i see in other places in the same source code file that there are checks for that - almost all calls to GetLCLObject either check for nil or use "if object is something then use object" (with the "is" operator returning false for nil, essentially making it an indirect test for nil).

This might be just an instance defensive programming, but i think it is a good idea to bring this up.


On Thu, May 18, 2017 at 12:20 AM, Kostas Michalopoulos <[hidden email]> wrote:
Hi all,

So i finally figured out why this happens and the reason is the ShowWindow(Handle, SW_SHOWNORMAL) call made when selecting components (this is done to activate the designer form).

Under Gtk2 this call always calls gtk_window_unmaximize (among others) but Gtk2 seems to have a bug that always tries to unmaximize the window regardless of its status - with some window managers (like Window Maker) this can cause the window to be resized. The bug is most likely with Gtk2 not ignoring the call for unmaximized windows, but since Gtk2's development has practically stopped (most recent changes are documentation and build fixes) and since Lazarus needs to work with 10+ year old versions of Gtk2, this needs to be addressed on the LCL side.

Fortunately LCL already tracks the window state in the WindowState property, so at least for TCustomForm descendants (which would be almost all cases this is needed - 100% of the cases in design time where the bug is mostly visible) we can workaround this by not calling gtk_window_unmaximize for forms that aren't maximized.

The description and patch can be found in Mantis here, as well as a video that shows the issue:

https://bugs.freepascal.org/view.php?id=31832

The patch was made against SVN but should apply with 1.8RC1 since i originally wrote it there.

Kostas


--
_______________________________________________
Lazarus mailing list
[hidden email]
http://lists.lazarus-ide.org/listinfo/lazarus
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [Lazarus] Patch for fixing the window resizes when switching components

Free Pascal - Lazarus mailing list
On Thu, 18 May 2017 02:43:10 +0300
Kostas Michalopoulos via Lazarus <[hidden email]> wrote:

> I see the patch was added and Lazarus seems to work fine now, but the check
> was altered a little. The current check assumes that the Control will never
> be nil, but is this the case?

The "is" operator checks for nil.

 if Assigned(Control) and (Control is TCustomForm) then

<=>

 if Control is TCustomForm then


Mattias
--
_______________________________________________
Lazarus mailing list
[hidden email]
http://lists.lazarus-ide.org/listinfo/lazarus
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [Lazarus] Patch for fixing the window resizes when switching components

Free Pascal - Lazarus mailing list
Oops, indeed, i was tripped up by the not in "(not (Control is TCustomForm)) or ..." part.



On Thu, May 18, 2017 at 10:28 AM, Mattias Gaertner via Lazarus <[hidden email]> wrote:
On Thu, 18 May 2017 02:43:10 +0300
Kostas Michalopoulos via Lazarus <[hidden email]> wrote:

> I see the patch was added and Lazarus seems to work fine now, but the check
> was altered a little. The current check assumes that the Control will never
> be nil, but is this the case?

The "is" operator checks for nil.

 if Assigned(Control) and (Control is TCustomForm) then

<=>

 if Control is TCustomForm then


Mattias
--
_______________________________________________
Lazarus mailing list
[hidden email]
http://lists.lazarus-ide.org/listinfo/lazarus


--
_______________________________________________
Lazarus mailing list
[hidden email]
http://lists.lazarus-ide.org/listinfo/lazarus
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [Lazarus] Patch for fixing the window resizes when switching components

Free Pascal - Lazarus mailing list
In reply to this post by Free Pascal - Lazarus mailing list
On 18.05.2017 01:43, Kostas Michalopoulos via Lazarus wrote:

> I see the patch was added and Lazarus seems to work fine now, but the
> check was altered a little. The current check assumes that the Control
> will never be nil, but is this the case? I ask because i see in other
> places in the same source code file that there are checks for that -
> almost all calls to GetLCLObject either check for nil or use "if object
> is something then use object" (with the "is" operator returning false
> for nil, essentially making it an indirect test for nil).
>
> This might be just an instance defensive programming, but i think it is
> a good idea to bring this up.
>

Yes, this is clearly gtk2 bug, but maybe we can take another approach
instead of getting LCL object and using "is". We should check if
GdkWindow of widget is valid and then check it's state and in case it's
= GDK_WINDOW_STATE_MAXIMIZED call gtk_window_unmaximize()


if GDK_IS_WINDOW(PGtkWindow(OurWidget)^.window) and
(gdk_window_get_state(PGtkWindow(OurWidget)^.window) =
GDK_WINDOW_STATE_MAXIMIZED) then
   gtk_window_unmaximize() ...
Same for unfullscreen and other "un" routines which we use inside that
procedure.

What other devels think about this ? Should I correct that patch in this
way and fix future problems with eg gtk_window_unfullscreen() which
isn't covered by this patch atm ?

zeljko






--
_______________________________________________
Lazarus mailing list
[hidden email]
http://lists.lazarus-ide.org/listinfo/lazarus
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [Lazarus] Patch for fixing the window resizes when switching components

Free Pascal - Lazarus mailing list
zeljko via Lazarus wrote:

> On 18.05.2017 01:43, Kostas Michalopoulos via Lazarus wrote:
>
> if GDK_IS_WINDOW(PGtkWindow(OurWidget)^.window) and
> (gdk_window_get_state(PGtkWindow(OurWidget)^.window) =
> GDK_WINDOW_STATE_MAXIMIZED) then
>   gtk_window_unmaximize() ...
> Same for unfullscreen and other "un" routines which we use inside that
> procedure.
>
> What other devels think about this ? Should I correct that patch in this
> way and fix future problems with eg gtk_window_unfullscreen() which
> isn't covered by this patch atm ?

I prefer using the gtk/gdk functions and avoid getting (win)controls
from the widgetsets

Marc

--
_______________________________________________
Lazarus mailing list
[hidden email]
http://lists.lazarus-ide.org/listinfo/lazarus
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [Lazarus] Patch for fixing the window resizes when switching components

Free Pascal - Lazarus mailing list
In reply to this post by Free Pascal - Lazarus mailing list
On Thu, 18 May 2017 13:20:52 +0200
zeljko via Lazarus <[hidden email]> wrote:

>[...]
> if GDK_IS_WINDOW(PGtkWindow(OurWidget)^.window) and
> (gdk_window_get_state(PGtkWindow(OurWidget)^.window) =
> GDK_WINDOW_STATE_MAXIMIZED) then
>    gtk_window_unmaximize() ...
> Same for unfullscreen and other "un" routines which we use inside that
> procedure.
>
> What other devels think about this ? Should I correct that patch in this
> way and fix future problems with eg gtk_window_unfullscreen() which
> isn't covered by this patch atm ?

Your solution looks more correct. But since this issue can only be
reproduced on some systems, can you test it?

Mattias
--
_______________________________________________
Lazarus mailing list
[hidden email]
http://lists.lazarus-ide.org/listinfo/lazarus
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [Lazarus] Patch for fixing the window resizes when switching components

Free Pascal - Lazarus mailing list
In reply to this post by Free Pascal - Lazarus mailing list
On 18.05.2017 13:38, Marc Weustink via Lazarus wrote:

> zeljko via Lazarus wrote:
>> On 18.05.2017 01:43, Kostas Michalopoulos via Lazarus wrote:
>>
>> if GDK_IS_WINDOW(PGtkWindow(OurWidget)^.window) and
>> (gdk_window_get_state(PGtkWindow(OurWidget)^.window) =
>> GDK_WINDOW_STATE_MAXIMIZED) then
>>   gtk_window_unmaximize() ...
>> Same for unfullscreen and other "un" routines which we use inside that
>> procedure.
>>
>> What other devels think about this ? Should I correct that patch in this
>> way and fix future problems with eg gtk_window_unfullscreen() which
>> isn't covered by this patch atm ?
>
> I prefer using the gtk/gdk functions and avoid getting (win)controls
> from the widgetsets

I've attached new patch https://bugs.freepascal.org/view.php?id=31832 
...maybe we need one more check if GdkWindow is toplevel but that's all,
approach is clear gtk2/gdk instead involving lcl in this part of code
for no reason.

zeljko

--
_______________________________________________
Lazarus mailing list
[hidden email]
http://lists.lazarus-ide.org/listinfo/lazarus
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [Lazarus] Patch for fixing the window resizes when switching components

Free Pascal - Lazarus mailing list
In reply to this post by Free Pascal - Lazarus mailing list
On 18.05.2017 13:43, Mattias Gaertner via Lazarus wrote:

> On Thu, 18 May 2017 13:20:52 +0200
> zeljko via Lazarus <[hidden email]> wrote:
>
>> [...]
>> if GDK_IS_WINDOW(PGtkWindow(OurWidget)^.window) and
>> (gdk_window_get_state(PGtkWindow(OurWidget)^.window) =
>> GDK_WINDOW_STATE_MAXIMIZED) then
>>    gtk_window_unmaximize() ...
>> Same for unfullscreen and other "un" routines which we use inside that
>> procedure.
>>
>> What other devels think about this ? Should I correct that patch in this
>> way and fix future problems with eg gtk_window_unfullscreen() which
>> isn't covered by this patch atm ?
>
> Your solution looks more correct. But since this issue can only be
> reproduced on some systems, can you test it?


Just attached new patch which checks if our GdkWindow is toplevel, so
it's correct now. Tested on Fedora 24 gtk2 64bit and it works for me :)
IMHO, nothing bad can happen with this patch since we check at the
beginning if window is mapped or not, so there's nothing to
unmaximize,unfullscreen or uniconify without mapped GdkWindow.

zeljko



--
_______________________________________________
Lazarus mailing list
[hidden email]
http://lists.lazarus-ide.org/listinfo/lazarus
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [Lazarus] Patch for fixing the window resizes when switching components

Free Pascal - Lazarus mailing list
In reply to this post by Free Pascal - Lazarus mailing list
On 18.05.2017 13:43, Mattias Gaertner via Lazarus wrote:

> On Thu, 18 May 2017 13:20:52 +0200
> zeljko via Lazarus <[hidden email]> wrote:
>
>> [...]
>> if GDK_IS_WINDOW(PGtkWindow(OurWidget)^.window) and
>> (gdk_window_get_state(PGtkWindow(OurWidget)^.window) =
>> GDK_WINDOW_STATE_MAXIMIZED) then
>>    gtk_window_unmaximize() ...
>> Same for unfullscreen and other "un" routines which we use inside that
>> procedure.
>>
>> What other devels think about this ? Should I correct that patch in this
>> way and fix future problems with eg gtk_window_unfullscreen() which
>> isn't covered by this patch atm ?
>
> Your solution looks more correct. But since this issue can only be
> reproduced on some systems, can you test it?

Pls, do not apply that patch yet. Must check what happens with
GdkWindowState.

zeljko

--
_______________________________________________
Lazarus mailing list
[hidden email]
http://lists.lazarus-ide.org/listinfo/lazarus
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [Lazarus] Patch for fixing the window resizes when switching components

Free Pascal - Lazarus mailing list
In reply to this post by Free Pascal - Lazarus mailing list
On 18.05.2017 13:43, Mattias Gaertner via Lazarus wrote:

> On Thu, 18 May 2017 13:20:52 +0200
> zeljko via Lazarus <[hidden email]> wrote:
>
>> [...]
>> if GDK_IS_WINDOW(PGtkWindow(OurWidget)^.window) and
>> (gdk_window_get_state(PGtkWindow(OurWidget)^.window) =
>> GDK_WINDOW_STATE_MAXIMIZED) then
>>    gtk_window_unmaximize() ...
>> Same for unfullscreen and other "un" routines which we use inside that
>> procedure.
>>
>> What other devels think about this ? Should I correct that patch in this
>> way and fix future problems with eg gtk_window_unfullscreen() which
>> isn't covered by this patch atm ?
>
> Your solution looks more correct. But since this issue can only be
> reproduced on some systems, can you test it?

Mattias, please if you merge something (at least related to
resolved/closed issues) add it to
http://wiki.freepascal.org/Lazarus_1.8_fixes_branch
(I've added r54958 to the merged revisions for RC2)

zeljko


--
_______________________________________________
Lazarus mailing list
[hidden email]
http://lists.lazarus-ide.org/listinfo/lazarus
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [Lazarus] Patch for fixing the window resizes when switching components

Free Pascal - Lazarus mailing list
In reply to this post by Free Pascal - Lazarus mailing list
On 18.05.2017 13:38, Marc Weustink via Lazarus wrote:

> zeljko via Lazarus wrote:
>> On 18.05.2017 01:43, Kostas Michalopoulos via Lazarus wrote:
>>
>> if GDK_IS_WINDOW(PGtkWindow(OurWidget)^.window) and
>> (gdk_window_get_state(PGtkWindow(OurWidget)^.window) =
>> GDK_WINDOW_STATE_MAXIMIZED) then
>>   gtk_window_unmaximize() ...
>> Same for unfullscreen and other "un" routines which we use inside that
>> procedure.
>>
>> What other devels think about this ? Should I correct that patch in this
>> way and fix future problems with eg gtk_window_unfullscreen() which
>> isn't covered by this patch atm ?
>
> I prefer using the gtk/gdk functions and avoid getting (win)controls
> from the widgetsets

I've removed workaround introduced in r54958 with pure gdk/gtk
implementation in r54975.
Please guys test, since I cannot reproduce problem here (even without
any patch): Fedora 24 64bit, KDE Plasma (kwin is window manager).
I need info from users from Mint 16..18 and Cinnamon DE which uses
mutter or metacity if this finally fixes problem.
Positive feedback = merge to 1.8RC2

Important note: Gtk2 changes window state in async mode under x11, so
statement like this won't work (it works ok under qt or win32) and you
can expect problems with such code :)

with TForm2.Create(Self) do
begin
   Show;
   LCLIntf.ShowWindow(Handle, SW_SHOWMAXIMIZED);
   LCLIntf.ShowWindow(Handle, SW_RESTORE);
end;


zeljko


--
_______________________________________________
Lazarus mailing list
[hidden email]
http://lists.lazarus-ide.org/listinfo/lazarus
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [Lazarus] Patch for fixing the window resizes when switching components

Free Pascal - Lazarus mailing list
Yes the new code works perfectly fine here and is a better solution. My original thought was to use pure Gtk but i forgot about gdk_ and only looked for something like gtk_window_get_state - which i couldn't find :-).

On Thu, May 18, 2017 at 8:37 PM, zeljko via Lazarus <[hidden email]> wrote:
On 18.05.2017 13:38, Marc Weustink via Lazarus wrote:
zeljko via Lazarus wrote:
On 18.05.2017 01:43, Kostas Michalopoulos via Lazarus wrote:

if GDK_IS_WINDOW(PGtkWindow(OurWidget)^.window) and
(gdk_window_get_state(PGtkWindow(OurWidget)^.window) =
GDK_WINDOW_STATE_MAXIMIZED) then
  gtk_window_unmaximize() ...
Same for unfullscreen and other "un" routines which we use inside that
procedure.

What other devels think about this ? Should I correct that patch in this
way and fix future problems with eg gtk_window_unfullscreen() which
isn't covered by this patch atm ?

I prefer using the gtk/gdk functions and avoid getting (win)controls
from the widgetsets

I've removed workaround introduced in r54958 with pure gdk/gtk implementation in r54975.
Please guys test, since I cannot reproduce problem here (even without any patch): Fedora 24 64bit, KDE Plasma (kwin is window manager).
I need info from users from Mint 16..18 and Cinnamon DE which uses mutter or metacity if this finally fixes problem.
Positive feedback = merge to 1.8RC2

Important note: Gtk2 changes window state in async mode under x11, so statement like this won't work (it works ok under qt or win32) and you can expect problems with such code :)

with TForm2.Create(Self) do
begin
  Show;
  LCLIntf.ShowWindow(Handle, SW_SHOWMAXIMIZED);
  LCLIntf.ShowWindow(Handle, SW_RESTORE);
end;


zeljko



--
_______________________________________________
Lazarus mailing list
[hidden email]
http://lists.lazarus-ide.org/listinfo/lazarus


--
_______________________________________________
Lazarus mailing list
[hidden email]
http://lists.lazarus-ide.org/listinfo/lazarus
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [Lazarus] Patch for fixing the window resizes when switching components

Free Pascal - Lazarus mailing list
On 18.05.2017 20:24, Kostas Michalopoulos via Lazarus wrote:
> Yes the new code works perfectly fine here and is a better solution. My
> original thought was to use pure Gtk but i forgot about gdk_ and only
> looked for something like gtk_window_get_state - which i couldn't find :-).

Thanks for feedback :)

zeljko

--
_______________________________________________
Lazarus mailing list
[hidden email]
http://lists.lazarus-ide.org/listinfo/lazarus
Loading...