View Issue Details

IDProjectCategoryView StatusLast Update
0030892LazarusLCLpublic2017-08-21 13:24
ReporterTony WhymanAssigned ToJesus Reyes 
PrioritynormalSeverityminorReproducibilityalways
Status resolvedResolutionfixed 
PlatformAMD64OSWindowsOS Version7
Product Version1.6Product Build 
Target Version1.8Fixed in Version1.7 (SVN) 
Summary0030892: Clipping Region Error in TCustomGrid Descendents (Windows only)
DescriptionThis bug seems to be restricted to drawing to a bitmap (Memory DC) canvas under Windows. The same code under Linux/GTK2 seems to work fine.

The problem is illustrated by the attached small application. This has a single form with two panels. The top panle comprises an edit control. an image and a string grid. The bottom panel is initially an empty image.

Click on the "Image Copy" button and the top panel is rendered to a bitmap using its "PaintTo" method and the bitmap them displayed as the contents of the bottom panel.

Under Linux, the image is an exact copy of the top panel. Under Windows, the edit control and image are fine, while the string grid is no more than an empty box.
Steps To ReproduceRun attached program under Windows.
Additional InformationThe problem seems to be in the clip region. The attached patch appears to correct the problem.
TagsNo tags attached.
Fixed in Revision53791, 54084
LazTarget1.8
WidgetsetWin32/Win64
Attached Files
  • GridBug.zip (229,344 bytes)
  • grids.patch (499 bytes)
    diff -rupN lazarus/lcl/grids.pas lazarus.update/lcl/grids.pas
    --- lazarus/lcl/grids.pas	2016-01-18 23:04:52.000000000 +0000
    +++ lazarus.update/lcl/grids.pas	2016-11-08 17:21:17.155114672 +0000
    @@ -4018,7 +4018,7 @@ var
     
         Canvas.SaveHandleState;
         try
    -      Rgn := CreateRectRgn(R.Left, R.Top, R.Right, R.Bottom);
    +      Rgn := CreateRectRgn(R.Left, R.Top, Left+R.Right,Top+ R.Bottom);
           SelectClipRgn(Canvas.Handle, Rgn);
           DrawCell(aCol, aRow, R, gds);
           DeleteObject(Rgn);
    
    grids.patch (499 bytes)
  • StringGridBug.zip (228,869 bytes)
  • stringridbug.patch (797 bytes)
    diff -rupN /home/tony/lazarus/svn/lcl/grids.pas svn/lcl/grids.pas
    --- /home/tony/lazarus/svn/lcl/grids.pas	2017-01-11 11:18:09.451904291 +0000
    +++ svn/lcl/grids.pas	2017-01-11 11:22:16.283438125 +0000
    @@ -4137,8 +4137,6 @@ var
       ClipArea: Trect;
     
       procedure DoDrawCell;
    -  var
    -    Rgn: HRGN;
       begin
         with FGCache do begin
           if (aCol=HotCell.x) and (aRow=HotCell.y) and not IsPushCellActive() then begin
    @@ -4152,10 +4150,9 @@ var
     
         Canvas.SaveHandleState;
         try
    -      Rgn := CreateRectRgn(R.Left, R.Top, R.Right, R.Bottom);
    -      SelectClipRgn(Canvas.Handle, Rgn);
    +      with R do
    +        InterSectClipRect(Canvas.Handle,Left, Top, Right, Bottom);
           DrawCell(aCol, aRow, R, gds);
    -      DeleteObject(Rgn);
         finally
           Canvas.RestoreHandleState;
         end;
    
    stringridbug.patch (797 bytes)
  • grids2.pas.patch (740 bytes)
    Index: lcl/grids.pas
    ===================================================================
    --- lcl/grids.pas	(revision 54052)
    +++ lcl/grids.pas	(working copy)
    @@ -4146,8 +4146,6 @@
       ClipArea: Trect;
     
       procedure DoDrawCell;
    -  var
    -    Rgn: HRGN;
       begin
         with FGCache do begin
           if (aCol=HotCell.x) and (aRow=HotCell.y) and not IsPushCellActive() then begin
    @@ -4161,10 +4159,8 @@
     
         Canvas.SaveHandleState;
         try
    -      Rgn := CreateRectRgn(R.Left, R.Top, R.Right, R.Bottom);
    -      SelectClipRgn(Canvas.Handle, Rgn);
    +      InterSectClipRect(Canvas.Handle, R.Left, R.Top, R.Right, R.Bottom);
           DrawCell(aCol, aRow, R, gds);
    -      DeleteObject(Rgn);
         finally
           Canvas.RestoreHandleState;
         end;
    
    grids2.pas.patch (740 bytes)

Relationships

related to 0025448 closedMichl GetFormImage supplies black Bitmap 

Activities

Tony Whyman

2016-11-08 18:32

reporter  

GridBug.zip (229,344 bytes)

Tony Whyman

2016-11-08 18:32

reporter  

grids.patch (499 bytes)
diff -rupN lazarus/lcl/grids.pas lazarus.update/lcl/grids.pas
--- lazarus/lcl/grids.pas	2016-01-18 23:04:52.000000000 +0000
+++ lazarus.update/lcl/grids.pas	2016-11-08 17:21:17.155114672 +0000
@@ -4018,7 +4018,7 @@ var
 
     Canvas.SaveHandleState;
     try
-      Rgn := CreateRectRgn(R.Left, R.Top, R.Right, R.Bottom);
+      Rgn := CreateRectRgn(R.Left, R.Top, Left+R.Right,Top+ R.Bottom);
       SelectClipRgn(Canvas.Handle, Rgn);
       DrawCell(aCol, aRow, R, gds);
       DeleteObject(Rgn);
grids.patch (499 bytes)

Michl

2016-12-28 18:22

developer   ~0097128

Fixed in trunk revision 53791. Please test and close if ok.

Tony Whyman

2017-01-11 12:33

reporter   ~0097409

Unfortunately I need to reopen this bug as I've found a case not addressed by the previously proposed fix.

Tony Whyman

2017-01-11 12:33

reporter  

StringGridBug.zip (228,869 bytes)

Tony Whyman

2017-01-11 12:35

reporter   ~0097410

I've uploaded a new file "StringGridBug.zip" that illustrates a case not addressed by the previously proposed fix. This modifies the previous example by placing the stringgrid inside a panel. In this case, the string grid is no longer copied to the image.

Tony Whyman

2017-01-11 12:35

reporter  

stringridbug.patch (797 bytes)
diff -rupN /home/tony/lazarus/svn/lcl/grids.pas svn/lcl/grids.pas
--- /home/tony/lazarus/svn/lcl/grids.pas	2017-01-11 11:18:09.451904291 +0000
+++ svn/lcl/grids.pas	2017-01-11 11:22:16.283438125 +0000
@@ -4137,8 +4137,6 @@ var
   ClipArea: Trect;
 
   procedure DoDrawCell;
-  var
-    Rgn: HRGN;
   begin
     with FGCache do begin
       if (aCol=HotCell.x) and (aRow=HotCell.y) and not IsPushCellActive() then begin
@@ -4152,10 +4150,9 @@ var
 
     Canvas.SaveHandleState;
     try
-      Rgn := CreateRectRgn(R.Left, R.Top, R.Right, R.Bottom);
-      SelectClipRgn(Canvas.Handle, Rgn);
+      with R do
+        InterSectClipRect(Canvas.Handle,Left, Top, Right, Bottom);
       DrawCell(aCol, aRow, R, gds);
-      DeleteObject(Rgn);
     finally
       Canvas.RestoreHandleState;
     end;
stringridbug.patch (797 bytes)

Tony Whyman

2017-01-11 12:42

reporter   ~0097411

Last edited: 2017-01-11 12:42

View 2 revisions

I've proposed a new fix in "stringgridbug.patch". This patch is against "trunk".

In order to find a more general fix, I looked for inspiration at TCanvas.TextRect. This similarly sets up a clipping region, but uses "InterSectClipRect" to do this instead of creating a region and selecting it into the DC. If you look at the code for InterSectClipRect, this also creates a region and selects it into the DC, but additionally includes a co-ordinate transform by calling "LPtoDP" before creating the region.

It looks like the error in TCustomGrid.DrawRow was to overlook the need for a co-ordinate transform and relied upon logical and device co-ordinates being the same. This assumption breaks when a bitmap is used as the canvas.

The proposed fix replaces the creation of the region and selecting it into the DC with a single call to InterSectClipRect. This seems to be the correct approach and fixes the issue shown up in both examples.

The bug probably has a wider scope that just drawing on a bitmap and applies to any case where the logical and device co-ordinates differ.

Michl

2017-02-03 18:21

developer  

grids2.pas.patch (740 bytes)
Index: lcl/grids.pas
===================================================================
--- lcl/grids.pas	(revision 54052)
+++ lcl/grids.pas	(working copy)
@@ -4146,8 +4146,6 @@
   ClipArea: Trect;
 
   procedure DoDrawCell;
-  var
-    Rgn: HRGN;
   begin
     with FGCache do begin
       if (aCol=HotCell.x) and (aRow=HotCell.y) and not IsPushCellActive() then begin
@@ -4161,10 +4159,8 @@
 
     Canvas.SaveHandleState;
     try
-      Rgn := CreateRectRgn(R.Left, R.Top, R.Right, R.Bottom);
-      SelectClipRgn(Canvas.Handle, Rgn);
+      InterSectClipRect(Canvas.Handle, R.Left, R.Top, R.Right, R.Bottom);
       DrawCell(aCol, aRow, R, gds);
-      DeleteObject(Rgn);
     finally
       Canvas.RestoreHandleState;
     end;
grids2.pas.patch (740 bytes)

Michl

2017-02-03 18:28

developer   ~0097950

@Jesus: I think, the patch from Tony Whyman works correct. It fixes the drawing for Win32 (Windows XP, 7 and 10 tested) and it seems not to break the drawing for other widgetsets (GTK2 and Qt4 tested).

I don't like the with ... statements, so I added a new patch grid2.pas.patch what is the same as the patch from Tony Whyman.

It is your construction area, so I don't apply it. You have to decide ;)

Jesus Reyes

2017-02-03 19:57

developer   ~0097953

Thanks for the patch and for testing, lets test it in the wild a little before targeting it to 1.6.4

Issue History

Date Modified Username Field Change
2016-11-08 18:32 Tony Whyman New Issue
2016-11-08 18:32 Tony Whyman File Added: GridBug.zip
2016-11-08 18:32 Tony Whyman File Added: grids.patch
2016-11-08 18:37 Jesus Reyes Assigned To => Jesus Reyes
2016-11-08 18:37 Jesus Reyes Status new => assigned
2016-12-28 18:22 Michl Fixed in Revision => r53791
2016-12-28 18:22 Michl LazTarget => 1.6.4
2016-12-28 18:22 Michl Note Added: 0097128
2016-12-28 18:22 Michl Status assigned => resolved
2016-12-28 18:22 Michl Fixed in Version => 1.7 (SVN)
2016-12-28 18:22 Michl Resolution open => fixed
2016-12-28 18:22 Michl Target Version => 1.6.3 (SVN)
2016-12-28 18:23 Michl Relationship added related to 0025448
2017-01-11 12:33 Tony Whyman Note Added: 0097409
2017-01-11 12:33 Tony Whyman Status resolved => assigned
2017-01-11 12:33 Tony Whyman Resolution fixed => reopened
2017-01-11 12:33 Tony Whyman File Added: StringGridBug.zip
2017-01-11 12:35 Tony Whyman Note Added: 0097410
2017-01-11 12:35 Tony Whyman File Added: stringridbug.patch
2017-01-11 12:42 Tony Whyman Note Added: 0097411
2017-01-11 12:42 Tony Whyman Note Edited: 0097411 View Revisions
2017-02-03 18:21 Michl File Added: grids2.pas.patch
2017-02-03 18:28 Michl Note Added: 0097950
2017-02-03 19:57 Jesus Reyes Fixed in Revision r53791 => 53791, 54084
2017-02-03 19:57 Jesus Reyes LazTarget 1.6.4 => 1.8
2017-02-03 19:57 Jesus Reyes Note Added: 0097953
2017-02-03 19:57 Jesus Reyes Status assigned => resolved
2017-02-03 19:57 Jesus Reyes Resolution reopened => fixed
2017-02-03 19:57 Jesus Reyes Target Version 1.6.3 (SVN) => 1.8