Recent

Author Topic: Code cleanup at nested procedure SendPaintMessage  (Read 3194 times)

lagprogramming

  • Sr. Member
  • ****
  • Posts: 407
Code cleanup at nested procedure SendPaintMessage
« on: April 29, 2023, 11:37:12 am »
lcl/interfaces/customdrawn/wincallback.inc has nested procedure SendPaintMessage(ControlDC: HDC);
The procedure has useless variable declarations and assignments to PaintRegion and needParentPaint.

Patch for improvement is:
Code: Pascal  [Select][+][-]
  1. diff --git a/lcl/interfaces/customdrawn/wincallback.inc b/lcl/interfaces/customdrawn/wincallback.inc
  2. index 4305f0a8cc..716dc08ab9 100644
  3. --- a/lcl/interfaces/customdrawn/wincallback.inc
  4. +++ b/lcl/interfaces/customdrawn/wincallback.inc
  5. @@ -157,14 +157,12 @@ Var
  6.    var
  7.      DC: HDC;
  8.      lBitmap, lMask: HBITMAP;
  9. -    PaintRegion: HRGN;
  10.      PS : TPaintStruct;
  11.      PaintMsg: TLMPaint;
  12.      WindowOrg: Windows.POINT;
  13.      WindowWidth, WindowHeight: Integer;
  14.      DCIndex: integer;
  15.      parLeft, parTop: integer;
  16. -    needParentPaint: boolean;
  17.      BufferWasSaved: Boolean;
  18.      lRawImage: TRawImage;
  19.    begin
  20. @@ -174,9 +172,6 @@ Var
  21.      DebugLn(Format('[SendPaintMessage]: Control:%s:%s', [lWinControl.Name, lWinControl.ClassName]));
  22.      {$ENDIF}
  23.  
  24. -    // create a paint message
  25. -    needParentPaint := False;
  26. -
  27.      LCLIntf.GetWindowSize(HWND(WindowInfo), WindowWidth, WindowHeight);
  28.  
  29.      // Start the double buffering by checking if we need to increase the buffer
  30. @@ -228,7 +223,6 @@ Var
  31.          DC := Windows.BeginPaint(Window, @PS);
  32.        end else begin
  33.          DC := ControlDC;
  34. -        PaintRegion := 0;
  35.        end;
  36.  
  37.        // Draw the form


Josh

  • Hero Member
  • *****
  • Posts: 1344
Re: Code cleanup at nested procedure SendPaintMessage
« Reply #2 on: April 29, 2023, 02:53:11 pm »
Code: Pascal  [Select][+][-]
  1. procedure SendPaintMessage(ControlDC: HDC);
  2.   var
  3.     DC: HDC;
  4.     lBitmap, lMask: HBITMAP;
  5.     PaintRegion: HRGN; xx defined but not used
  6.     PS : TPaintStruct; ?? not defined but used
  7.     PaintMsg: TLMPaint; xxx not defined not used
  8.     WindowOrg: Windows.POINT; xx not defined not used
  9.     WindowWidth, WindowHeight: Integer;
  10.     DCIndex: integer; xx not defined not used
  11.     parLeft, parTop: integer; xx
  12.     needParentPaint: boolean; xx defined not used
  13.     BufferWasSaved: Boolean; xx not defined not used
  14.     lRawImage: TRawImage;
  15.   begin
« Last Edit: April 29, 2023, 02:56:56 pm by Josh »
The best way to get accurate information on the forum is to post something wrong and wait for corrections.

lagprogramming

  • Sr. Member
  • ****
  • Posts: 407
Re: Code cleanup at nested procedure SendPaintMessage
« Reply #3 on: April 29, 2023, 03:59:01 pm »
Indeed.
I wonder why those message filters have been applied to customdrawn. There are simply too many lines that don't do anything except for making the code harder to understand.
I'm also looking forward at
https://forum.lazarus.freepascal.org/index.php/topic,63222.msg478541.html#msg478541

AlexTP

  • Hero Member
  • *****
  • Posts: 2488
    • UVviewsoft
Re: Code cleanup at nested procedure SendPaintMessage
« Reply #4 on: April 29, 2023, 08:22:50 pm »
@lagprogramming
See reply from developers on bugtracker
Quote
Unfortunately, Felipe who had started the CustomDrawn widgetset, has left the team, and the code is unmaintained currently. Of course, this can change when somebody else get motivated to take this under his wings. We should not do anything to complicate such a future work.
Attempting to address the many hints and warnings emitted for the customdrawn interface today it became clear the this widgetset is probably much less than half-finished. There are sections copied from other widgetsets, or empty procedures to be filled with life later. All this can be understand as reminders of what has to be done when the widgetset is to be completed.
Therefore, I strongly pledge to not touch these trivial optimizations as proposed here.
Code cleanup should be performed when something is finished, but not along the way.

lagprogramming

  • Sr. Member
  • ****
  • Posts: 407
Re: Code cleanup at nested procedure SendPaintMessage
« Reply #5 on: May 01, 2023, 07:50:28 am »
I understand wp's concerns. None of my patches add confusion to a potential new CustomDrawn maintainer because it was a factor that I've taken care from the very beginning. You won't see functions or procedures renamed, the Windows and QT code that has been copied and not implemented is still there and so on. So, there is no problem in applying that patch.
Regarding a potential new maintainer for CustomDrawn, it's known that the code contains lots of useless text lines, both code and comments. The very first thing a potential new maintainer will do is to clean up that code and that will take a while. The second thing will be to remove the obsolete code for FPC compatibility. Example, CustomDrawn has bugs related to clipping. In order to fix these bugs lcl/lazcanvas.pas needs to be modified too. LazCanvas uses property ClipRegion: TFPCustomRegion for compatibility with FPC 2.6 and DOESN'T use the GetClipRect routine for newer FPC versions. Regarding the FPC version, lcl/lclversion.pas writes that at least 2.4.2 is required, except for wince which supports fpc 2.2.0+ too. I have doubts a new maintainer will write code to fix the clipping bug taking care of FPC compatibility too(I mean writing code that uses the ClipRegion property). And as you can see, lcl/lazcanvas.pas is not in the customdrawn subdirectory, which means it can't be treated as an pre-alpha code. https://forum.lazarus.freepascal.org/index.php/topic,63222.0.html
Those who wait for a new CustomDrawn maintainer should know that it's easier and faster to port an application to msegui or fpgui than to fix the necessary bugs in CustomDrawn.
On the other hand, again I agree with wp that the patches that try to clean up the code won't significantly improve CustomDrawn status. The entire approach has poor results, but not because of me or because of the status of existing CustomDrawn files. Situations like this one were known to appear many years ago when it was proposed to test and use modifications of Fpc and Lazarus, modifications that were not delivered as ordinary diff files. Instead of the common patch/diff applications, a closed source application would have been used. The fact that the application that modifed the files was closed source was considered unacceptable, the benefits being ignored. Now the community uses forks, which at that time was considered something very bad. Also, even linux kernel allows closed source 3rd party addons(drivers). So, even if independent FPC/Lazarus developers would try to make source modifications available to the ordinary Fpc/Lazarus users, those modifications are not easily accessible.
My proposal is to apply the patch as it is, no harm will be done, and I'll try to see if after all these years the community got open to also use closed source software. Adding a new CustomDrawn package in OPM might be another idea.

wp

  • Hero Member
  • *****
  • Posts: 12471
Re: Code cleanup at nested procedure SendPaintMessage
« Reply #6 on: May 01, 2023, 11:11:29 am »
Long read, but sorry, I don't understand what you are talking about. What is the issue with GetClipRect? And what does CustomDrawn have to do with close source software?

The real problem is that CustomDrawn at the moment does not have a maintainer, and there's nobody among the currently active developers who knows about the exact status of the project and who understands the details. My impression is that the project is at a very preliminary stage. Any cosmetic change such as removal of unused variables or empty procedures is an unnecessary complication for a future maintainer because it removes information left by its previous maintainer, be it intentional or not. Better be safe than sorry.

lagprogramming

  • Sr. Member
  • ****
  • Posts: 407
Re: Code cleanup at nested procedure SendPaintMessage
« Reply #7 on: May 01, 2023, 12:58:12 pm »
Long read, but sorry, I don't understand what you are talking about. What is the issue with GetClipRect? And what does CustomDrawn have to do with close source software?

The real problem is that CustomDrawn at the moment does not have a maintainer, and there's nobody among the currently active developers who knows about the exact status of the project and who understands the details. My impression is that the project is at a very preliminary stage. Any cosmetic change such as removal of unused variables or empty procedures is an unnecessary complication for a future maintainer because it removes information left by its previous maintainer, be it intentional or not. Better be safe than sorry.

Should the development of CustomDrawn be stopped because it has no active maintainer!?
Why nobody wants to be the active maintainer, especially considering that it has much more potential than any other WS!?
In order to fix CustomDrawn clipping bugs for example, the file lcl/lazcanvas.pas needs to be fixed too. TLazCanvas Savestate and RestoreState save only the clipping value. ResetCanvasState doesn't work either for obvious reasons when looking at the code. This means that an independent developer can't release his modifications to CustomDrawn as a package in OPM. What else is available!? A diff window in Lazarus and the possibility of forking the entire Lazarus project. I have doubts there is a significant interest in such solutions.
It is expected that CustomDrawn will remain in the same pre-alpha status, at least for a while.

wp

  • Hero Member
  • *****
  • Posts: 12471
Re: Code cleanup at nested procedure SendPaintMessage
« Reply #8 on: May 01, 2023, 01:06:52 pm »
You are misunderstanding me... I am only against cosmetic changes at this stage. No problem with justified, "real" bugs. Send a patch for the TLazCanvas.SaveState/RestoreState along with a good description why this is a bug, and with a small project to verify it, I'll have a look if I feel competent - well maybe you already did, but I am tired of seeking the forum when we have a bugtracker and you refuse to use it.

Fred vS

  • Hero Member
  • *****
  • Posts: 3414
    • StrumPract is the musicians best friend
Re: Code cleanup at nested procedure SendPaintMessage
« Reply #9 on: May 01, 2023, 01:32:07 pm »
...
Those who wait for a new CustomDrawn maintainer should know that it's easier and faster to port an application to msegui or fpgui than to fix the necessary bugs in CustomDrawn.
...

Hello.

What is the status of CustomDrawn for Mac OSX (I dont have a Mac to test it) ?
If, by chance, CustomDrawn can produce working applications on last Mac OSX, fpGUI and MSEgui would be very interested to use the CustomDrawn-Mac code to make fpGUI and MSEgui last OSX compatible.

 ;)

Fre;D
I use Lazarus 2.2.0 32/64 and FPC 3.2.2 32/64 on Debian 11 64 bit, Windows 10, Windows 7 32/64, Windows XP 32,  FreeBSD 64.
Widgetset: fpGUI, MSEgui, Win32, GTK2, Qt.

https://github.com/fredvs
https://gitlab.com/fredvs
https://codeberg.org/fredvs

 

TinyPortal © 2005-2018