zertovitch / gwindows

GWindows: GUI framework for MS Windows
https://sf.net/projects/gnavi/
21 stars 5 forks source link

Missing Common Control notification callbacks #15

Closed NicoPy closed 10 months ago

NicoPy commented 11 months ago

I need WM_ERASEBKGND and WM_PAINT callbacks for a custom Status_Bar_Type.

These callbacks are declared for GWindows.Window.Window_Type but not for GWindows.Base.Base_Window_Type. StatusBar_Type is derived from GWindows.Base.Base_Window_Type.

What is the best way to add WM_ERASEBKGND and WM_PAINT management to my Status Bar ?

NicoPy commented 11 months ago

I used On_Message in my custom Status Bar. Obviously, this is the simplest way.

Still curious to know why WM_ERASEBKGND and WM_PAINT messages are not managed by GWindows.Base.Base_Window_Type.

zertovitch commented 11 months ago

I would say solution 2 (Add WM_ERASEBKGND and WM_PAINT management to GWindows.Base.Base_Window_Type) is best. Generally, I miss sometimes some methods in Common_Controls (and derived) for that reason: methods are present in Window_Type but not in Base_Window_Type, from which Common_Control_Type is derived. However, I'll ask on the gnavi-discuss mailing list. Perhaps there is a subtelty there (the Windows API is tricky).

NicoPy commented 11 months ago

Thanks Gautier.

I just realized that GWindows.Window.Window_Type is derived from GWindows.Base.Base_Window_Type. So, if adding WM_ERASEBKGND and WM_PAINT management to GWindows.Base.Base_Window_Type, do we have to remove it from GWindows.Window.Window_Type ?

zertovitch commented 11 months ago

Yep, we could actually just move On_Erase_Background_Handler, Fire_On_Erase_Background, On_Erase_Background and On_Erase_Background_Event code to GWindows.Base. Same for *Paint. And move the code around when WM_ERASEBKGND => and when WM_PAINT = in On_Message from GWindows.Windows to GWindows.Base. That's for the theory. But I suspect a nasty banana skin lurking around (some stuff that distinguish the Window_Type from Base_Window_Type. Anyway, I could test that approach on a complex graphical software like TeXCAD and see if something explodes - or not. In the meantime, I've put a question here: https://sourceforge.net/p/gnavi/mailman/message/47766290/ Perhaps a GWindows guru will react...

NicoPy commented 11 months ago

Thanks for your help Gautier.

zertovitch commented 11 months ago

Looking at the code around WM_ERASEBKGND and WM_PAINT in GWindows.Windows, it is doing something even when On_Erase_Background or On_Paint are doing nothing. Especially, it seems to assume there is a canvas with the window, and perhaps the "base windows" haven't always a canvas. So I guess, unless we get advice from a Windows API professional, it is better not to touch the mechanisms of GWindows.Base and GWindows.Windows. Perhaps we can give a try by porting the code from GWindows.Windows to GWindows.Common_Controls. It is less elegant (code duplication) but likely safer.

NicoPy commented 11 months ago

OK. I'll have some time for this soon. So, let's go 👍

NicoPy commented 11 months ago

I tried... and failed 😢

I duplicated the concerned code from gwindows-windows.adb to gwindows-common_controls.adb. In the On_Message procedure, there is :

      case message is
...
         when WM_PAINT =>
            declare
               PS : PAINTSTRUCT_Type;
               CV : GWindows.Drawing.Canvas_Type;
               ST : GWindows.Drawing.Canvas_State_Type;
            begin
               BeginPaint (Handle (Window), PS);
               GWindows.Drawing.Handle (CV, PS.HDC);
               ST := GWindows.Drawing.Save_State (CV);
               On_Paint (Window_Type'Class (Window),
                         CV,
                         PS.rcPaint);
               GWindows.Drawing.Load_State (CV, ST);
               GWindows.Drawing.Handle (CV, Null_Handle);
               EndPaint (Handle (Window), PS);

               Return_Value := 0;
            end;

         when WM_ERASEBKGND =>
            declare
               CV : GWindows.Drawing.Canvas_Type;
            begin
               GWindows.Drawing.Handle (CV, To_Handle (wParam));

               On_Erase_Background (Window_Type'Class (Window),
                                    CV,
                                    (0, 0,
                                     Client_Area_Width (Window),
                                     Client_Area_Height (Window)));

               Return_Value := 0;
            end;
...
         when others =>
            On_Message (GWindows.Base.Base_Window_Type (Window),
                        message,
                        wParam,
                        lParam,
                        Return_Value);
      end case;

The default handlers are never called. This is a problem since the default handlers are the system handlers which are in charge of drawing the system controls. As a result, all non owner-drawn controls are displayed as an empty flat surface.

For the WM_ERASEBKGND message, one workaround to this problem is to add a parameter to the handler to tell if the default handler has to be called.

         when WM_ERASEBKGND =>
            declare
               use GWindows.Types;
               CV : GWindows.Drawing.Canvas_Type;
               call_default : Boolean;
            begin
               GWindows.Drawing.Handle (CV, To_Handle (wParam));

               On_Erase_Background (Common_Control_Type'Class (Window),
                                    CV,
                                    (0, 0,
                                     Client_Area_Width (Window),
                                     Client_Area_Height (Window)),
                                     call_default);

               Return_Value := 0;

               if call_default then
                 --  Call parent method:
                 Base.On_Message
                    (Base.Base_Window_Type (Window),
                     message,
                     wParam,
                     lParam,
                     Return_Value);
               end if;
            end;

When On_Erase_Background handler sets call_default to True, the default handler is called. This is the responsibility of the On_Erase_Background handler to set call_default to the correct value. The problem is that the handler signature is different from the other On_Erase_Background handlers. This is inconsistent.

Another problem comes with the ON_PAINT handler. GWindows prepares a Canvas before calling the On_Paint handler :

               BeginPaint (Handle (Window), PS);
               GWindows.Drawing.Handle (CV, PS.HDC);
               ST := GWindows.Drawing.Save_State (CV);
               On_Paint (Window_Type'Class (Window),
                         CV,
                         PS.rcPaint);
               GWindows.Drawing.Load_State (CV, ST);
               GWindows.Drawing.Handle (CV, Null_Handle);
               EndPaint (Handle (Window), PS);

The BeginPaint \ EndPaint procedures pair makes the call to the default handler not working. This is because the update area is marked as validated, that is, it not necessary to redraw the area. One solution is to invalidate this area using InvalidateRect :

         when WM_PAINT =>
            declare
               PS : PAINTSTRUCT_Type;
               CV : GWindows.Drawing.Canvas_Type;
               ST : GWindows.Drawing.Canvas_State_Type;
            begin
               BeginPaint (Handle (Window), PS);
               GWindows.Drawing.Handle (CV, PS.HDC);
               ST := GWindows.Drawing.Save_State (CV);
               On_Paint (Common_Control_Type'Class (Window),
                         CV,
                         PS.rcPaint,
                         call_default);
               GWindows.Drawing.Load_State (CV, ST);
               GWindows.Drawing.Handle (CV, GWindows.Types.Null_Handle);
               EndPaint (Handle (Window), PS);

               Return_Value := 0;

               if call_default then
                 InvalidateRect (Handle (Window),
                                 PS.rcPaint'Unrestricted_Access,
                                 Integer (PS.fErase));

                 --  Call parent method:
                 Base.On_Message
                    (Base.Base_Window_Type (Window),
                     message,
                     wParam,
                     lParam,
                     Return_Value);
               end if;
            end;

Again, the On_Paint handler has to be modified to add the call_default parameter.

If you think all this is acceptable, I'll clean my code and publish it.

zertovitch commented 10 months ago

Just curious... Why isn't it an issue for Window_Type ? Anyway, the proposal seems reasonable. If you want to reproduce the GWindows "method or call-back choice" structure, the default method is calling the call-back (if defined), or usually doing nothing otherwise, except some cases like precisely GWindows.Windows.On_Erase_Background which does something, with a blatant graphical resource leak (B, see below). Another issue... Anyway, I guess the "something" following if Window.On_*_Event = null then on the default methods you intend to introduce in Common_Controls would be Call_Default := True, in order to have that value well determined by default. For testing your changes, you have perhaps noticed gwindows_samples.gpr, gwindows_contrib.gpr and gwindows_test.gpr...

   procedure On_Erase_Background
     (Window : in out Window_Type;
      Canvas : in out GWindows.Drawing.Canvas_Type;
      Area   : in     GWindows.Types.Rectangle_Type)
   is
      use GWindows.Drawing;
   begin
      if Window.On_Erase_Background_Event = null then
         if Window.Background_Color_Sys then
            Fill_Rectangle (Canvas,
                            Area,
                            GWindows.Colors.COLOR_BTNFACE);
         else
            declare
               use GWindows.Drawing_Objects;

               B : Brush_Type;
            begin
               Create_Solid_Brush (B, Window.Background_Color);
               Fill_Rectangle (Canvas, Area, B);
            end;
         end if;
      else
         Fire_On_Erase_Background (Window, Canvas, Area);
      end if;
   end On_Erase_Background;
zertovitch commented 10 months ago

The resource leak mentioned in previous message is fixed here: https://github.com/zertovitch/gwindows/commit/e46b5f496f947f24d4a6184a58a8d749e47a91c1 .

NicoPy commented 10 months ago

Just curious... Why isn't it an issue for Window_Type ?

Good question. I think the window is just the client area, excluding the frame. By default, there's nothing in the client area.

If you want to reproduce the GWindows "method or call-back choice" structure, the default method is calling the call-back (if defined), or usually doing nothing otherwise, except some cases like precisely GWindows.Windows.On_Erase_Background which does something, with a blatant graphical resource leak (B, see below). Another issue...

I have duplicated all of this mechanism and modified the minimum needed for my tests.

Anyway, I guess the "something" following if Window.On_*_Event = null then on the default methods you intend to introduce in Common_Controls would be Call_Default := True, in order to have that value well determined by default.

Correct.

For testing your changes, you have perhaps noticed gwindows_samples.gpr, gwindows_contrib.gpr and gwindows_test.gpr

Well... no. I'll have a look at them. Thanks.

The resource leak mentioned in previous message is fixed here: https://github.com/zertovitch/gwindows/commit/e46b5f496f947f24d4a6184a58a8d749e47a91c1 .

Why not just deleting the brush after the call to Fill_Rectangle ?

Note : In my code, I added a parameter to the callback procedure. Setting this parameter might be forgotten by the programmer. With a function you can't forget to return a value. Wouldn't it be better to use a function for the callback ?

zertovitch commented 10 months ago

Regarding the brush: ouch, yes of course, it is simpler!... Changed here: https://github.com/zertovitch/gwindows/commit/dbdfb25eb46841bcf86f0a63b440cb82718667c1 . Thanks!

Regarding the call-back, I prefer keeping as procedure. Main reason is that there are good chances that some users of GWindows are still using very old versions of GNAT that were until some point available free of charge and with an unlimited license... So for the framework I prefer to stick with Ada 95 that forbids functions with in out or out parameters. Then comes the debate about functions that change their parameters...

But you could have your Call_Default parameter as in out, so it would be set to True by the case ... when WM_ERASEBKGND => code even before calling the On_Erase_Background method. That way you ensure that Call_Default is True until someone (in a derived method or by setting a call-back) decides to change it.

NicoPy commented 10 months ago

Regarding the call-back, I prefer keeping as procedure.

OK. No problem.

But you could have your Call_Default parameter as in out, so it would be set to True by the case ... when WM_ERASEBKGND => code even before calling the On_Erase_Background method. That way you ensure that Call_Default is True until someone (in a derived method or by setting a call-back) decides to change it.

I thought about it. Another way is to use an enum like (Not_Set, Yes, No). The call_default variable is set to Not_Set before calling On_Paint. When it is still set to Not_Set after the call to On_Paint handler, an exception is raised. This way, we enforce the handler to set call_default to either Yes or No. The drawback is that we have to declare another public type.

zertovitch commented 10 months ago

The enum seems an elegant solution. No problem with another type, there are many of them in the framework, for very localized uses.

NicoPy commented 10 months ago

Ok. So, let go this way 😃

zertovitch commented 10 months ago

Solved through PR https://github.com/zertovitch/gwindows/pull/18