All Projects FPC fpcprojects fpprofilerfpGUI Lazarus Packages PatchesLazarus CCR Mantis pas2js

View Issue Details

Jump to NotesJump to History
IDProjectCategoryView StatusDate SubmittedLast Update
0037219LazarusLCLpublic2020-06-16 10:532020-11-28 10:24
ReporterJoeny Ang Assigned ToJuha Manninen  
PrioritynormalSeverityminorReproducibilityalways
Status closedResolutionfixed 
Product Version2.1 (SVN) 
Summary0037219: GTK2: TextRect and regions
DescriptionTCanvas.TextRect() yields different behaviors if a region is defined. No problems with TextOut().

Test:
In the attached test project, nothing is supposed to be written inside the rectangle when Region is checked.

Observations:
- TextStyle.Clipping = True
  If Rect is completely inside the region that is clipped, the text will be displayed, unclipped.
- TextStyle.Clipping = False
  After the first TextRect(), the region is removed (?); so succeeding canvas output will have no region defined.
- TextStyle.Clipping = False; TextStyle.Opaque = True
  Correct behavior

TagsNo tags attached.
Fixed in Revisionr63436, r63665
LazTarget-
WidgetsetGTK 2
Attached Files

Relationships

related to 0037409 resolvedMattias Gaertner Dangling pointer access in TGtk2WidgetSet.CombineRgn 
related to 0038062 closedJuha Manninen TListView (and others ?) not drawing text on uncover 

Activities

Joeny Ang

2020-06-16 10:53

reporter  

gtk2-textrect-and-regions-test-01..zip (111,909 bytes)

Joeny Ang

2020-06-19 10:27

reporter   ~0123474

Last edited: 2020-06-19 10:39

View 2 revisions

 Found out the following after doing some digging:
- NullRegion is equal to "no defined region", which shouldn't be. Without a defined region, the whole canvas can be drawn onto; with a NullRegion assigned, it should not be drawable.
- RestoreDC() does not always retore the saved region.

Patch:
- CreateEmptyRegion(): will return a region with an imaginary rect (-1, -1, 0, 0) defined. This will still be a NullRegion but will not allow any writing to the cavnas.
- CombineRgn(): after combining regions, will check if result is a NullRegion (0, 0, 0, 0), if so, recreate it using CreateEmptyRegion
- RestoreDC(): removed the check for region change

Not a really nice workaround, but works :)
combinergn-test-01.zip (110,885 bytes)
gtk2-textrect-and-regions.patch (2,188 bytes)    
--- lcl/interfaces/gtk2/gtk2winapi.inc
+++ lcl/interfaces/gtk2/gtk2winapi.inc
@@ -1866,6 +1866,13 @@
     Result := RegionType(D);
     //DebugLn('TGtk2WidgetSet.CombineRgn B Mode=',dbgs(fnCombineMode),
     //  ' S1=',GDKRegionAsString(S1),' S2=',GDKRegionAsString(S2),' D=',GDKRegionAsString(D),'');
+    if (Result = NullRegion) and
+      ((RegionType(S1) <> NullRegion) or ((RegionType(S2) <> NullRegion))) then
+    begin
+      DeleteObject(Dest);
+      Dest := CreateEmptyRegion;
+      Result := RegionType(D);
+    end;
   end;
 end;
 
@@ -7537,7 +7544,7 @@
   DevCtx: TGtkDeviceContext absolute DC;
 
   SavedDevCtx: TGtkDeviceContext;
-  ClipRegionChanged: Boolean;
+//  ClipRegionChanged: Boolean;
 begin
   if not IsValidDC(DC) then Exit(False);
   if SavedDC <= 0 then Exit(False);
@@ -7548,14 +7555,14 @@
 
     // TODO copy bitmap too
 
-    ClipRegionChanged := DevCtx.ClipRegion <> SavedDevCtx.ClipRegion;
-    
+//    ClipRegionChanged := DevCtx.ClipRegion <> SavedDevCtx.ClipRegion;
+
     // clear the GDIObjects in pSavedDC, so they are not freed by DeleteDC
     Result := DevCtx.CopyDataFrom(SavedDevCtx, True, True, True);
     DevCtx.SavedContext := SavedDevCtx.SavedContext;
     SavedDevCtx.SavedContext := nil;
 
-    if ClipRegionChanged then
+//    if ClipRegionChanged then
       DevCtx.SelectRegion;
 
     // free saved DC
--- lcl/interfaces/gtk2/gtk2widgetset.inc
+++ lcl/interfaces/gtk2/gtk2widgetset.inc
@@ -6046,10 +6046,24 @@
 function TGtk2WidgetSet.CreateEmptyRegion: hRGN;
 var
   GObject: PGdiObject;
+  R: TGDKRectangle;
+  RegionObj: PGdkRegion;
+  RRGN: PGDKRegion;
 begin
   GObject := NewGDIObject(gdiRegion);
-  GObject^.GDIRegionObject := gdk_region_new;
+  R.x := -1;
+  R.y := -1;
+  R.width := 1;
+  R.height := 1;
+  RRGN := gdk_region_new;
+  RegionObj := PGdkRegion(gdk_region_union_with_rect(RRGN,@R));
+  GObject^.GDIRegionObject := RegionObj;
+  gdk_region_destroy(RRGN);
   Result := HRGN({%H-}PtrUInt(GObject));
+
+//  GObject := NewGDIObject(gdiRegion);
+//  GObject^.GDIRegionObject := gdk_region_new;
+//  Result := HRGN({%H-}PtrUInt(GObject));
   //DebugLn('TGtk2WidgetSet.CreateEmptyRgn A RGN=',DbgS(Result));
 end;
gtk2-textrect-and-regions.patch (2,188 bytes)   
save-restore-dc-test-01.zip (111,503 bytes)

Juha Manninen

2020-06-24 11:14

developer   ~0123550

Applied, thanks.
I removed the commented lines. They can be restored from revision control if needed.

Joeny Ang

2020-06-25 02:52

reporter   ~0123587

Great! Thanks :)

Joeny Ang

2020-07-24 16:33

reporter   ~0124306

Last edited: 2020-07-24 16:33

View 2 revisions

 Hi, recent issue 0037409 has affected this. The code added to TGtk2WidgetSet.CombineRgn() was commented out due to a "dangling pointer access" by the line with the arrow:

    if (Result = NullRegion) and
      ((RegionType(S1) <> NullRegion) or ((RegionType(S2) <> NullRegion))) then
    begin
      DeleteObject(Dest);
      Dest := CreateEmptyRegion;
      Result := RegionType(D);     <-----
    end;


Can we just remove the line? since Result was already NullRegion before this code was called anyway.

Thanks :)

wp

2020-07-24 17:20

developer   ~0124307

Please don't add new information to resolved reports -- there is a high chance that nobody will see them. You should re-open the report so that the report becomes visible again.

Joeny Ang

2020-07-25 03:22

reporter   ~0124321

@wp, ok, thanks :)

Juha Manninen

2020-07-25 09:38

developer   ~0124323

> Can we just remove the line? since Result was already NullRegion before this code was called anyway.

You mean to uncomment the code that was commented out in r63635, then remove the line "Result := RegionType(D);"?

Joeny Ang

2020-07-25 10:57

reporter   ~0124325

> You mean to uncomment the code that was commented out in r63635, then remove the line "Result := RegionType(D);"?

Yes, I got it wrong passing "D" as parameter to "RegionType", it should have been "Result := RegionType({%H-}PGdiObject(Dest)^.GDIRegionObject);".... but this will return a SimpleRegion instead of a NullRegion for (-1, -1, 0, 0). We need it to be NullRegion for this workaround to work.

Or another way is to modify RegionType() to treat (-1, -1, 0, 0) regions as NullRegion. I think this is better :)

Joeny Ang

2020-07-25 11:13

reporter   ~0124326

Uhh... not as easy as it looks... I'll do some more tests. :)

Joeny Ang

2020-07-27 09:15

reporter   ~0124356

After going through the code of GDKRegion (which is by the way deprecated in GTK3), turns out that the result of CreateEmptyRegion() is a valid NullRegion.
The problem was within TGtkDeviceContext.SelectRegion() wherein the region will not be selected if it is of type ERROR or NULLREGION.

Patch:
- removed the check in TGtkDeviceContext.SelectRegion() to allow NULLREGION to be selected
- removed the workaround in TGtk2WidgetSet.CreateEmptyRegion() and TGtk2WidgetSet.CombineRgn()
gtk2-textrect-and-regions-pt2.patch (1,687 bytes)    
--- lcl/interfaces/gtk2/gtk2devicecontext.inc.63657
+++ lcl/interfaces/gtk2/gtk2devicecontext.inc
@@ -1256,7 +1256,7 @@
   if ClipRegion <> nil then
   begin
     RGNType := RegionType(ClipRegion^.GDIRegionObject);
-    if (RGNType <> ERROR) and (RGNType <> NULLREGION) then
+    if RGNType <> ERROR then
       gdk_gc_set_clip_region(FGC,  ClipRegion^.GDIRegionObject);
   end;
 
--- lcl/interfaces/gtk2/gtk2widgetset.inc.63657
+++ lcl/interfaces/gtk2/gtk2widgetset.inc
@@ -6047,19 +6047,9 @@
 function TGtk2WidgetSet.CreateEmptyRegion: hRGN;
 var
   GObject: PGdiObject;
-  R: TGDKRectangle;
-  RegionObj: PGdkRegion;
-  RRGN: PGDKRegion;
 begin
   GObject := NewGDIObject(gdiRegion);
-  R.x := -1;
-  R.y := -1;
-  R.width := 1;
-  R.height := 1;
-  RRGN := gdk_region_new;
-  RegionObj := PGdkRegion(gdk_region_union_with_rect(RRGN,@R));
-  GObject^.GDIRegionObject := RegionObj;
-  gdk_region_destroy(RRGN);
+  GObject^.GDIRegionObject := gdk_region_new;
   Result := HRGN({%H-}PtrUInt(GObject));
   //DebugLn('TGtk2WidgetSet.CreateEmptyRgn A RGN=',DbgS(Result));
 end;
--- lcl/interfaces/gtk2/gtk2winapi.inc.63657
+++ lcl/interfaces/gtk2/gtk2winapi.inc
@@ -1871,13 +1871,6 @@
   Result := RegionType(D);
   //DebugLn('TGtk2WidgetSet.CombineRgn B Mode=',dbgs(fnCombineMode),
   //  ' S1=',GDKRegionAsString(S1),' S2=',GDKRegionAsString(S2),' D=',GDKRegionAsString(D),'');
-  {if (Result = NullRegion) and
-    ((RegionType(S1) <> NullRegion) or ((RegionType(S2) <> NullRegion))) then
-  begin
-    DeleteObject(Dest);
-    Dest := CreateEmptyRegion;
-    Result := RegionType(Dest);
-  end;}
 end;
 
 {------------------------------------------------------------------------------
gtk2-textrect-and-regions-pt2.patch (1,687 bytes)   

Juha Manninen

2020-07-28 22:56

developer   ~0124370

Applied the last patch. Thanks!

Joeny Ang

2020-09-23 03:44

reporter   ~0125768

Thanks :)

Juha Manninen

2020-11-11 10:13

developer   ~0126833

Reopening because of 0038062.
Joeny Ang, can you please look at it.

Juha Manninen

2020-11-13 10:42

developer   ~0126874

... and resolving again.

Joeny Ang

2020-11-28 10:24

reporter   ~0127227

:)

Issue History

Date Modified Username Field Change
2020-06-16 10:53 Joeny Ang New Issue
2020-06-16 10:53 Joeny Ang File Added: gtk2-textrect-and-regions-test-01..zip
2020-06-19 10:27 Joeny Ang Note Added: 0123474
2020-06-19 10:27 Joeny Ang File Added: combinergn-test-01.zip
2020-06-19 10:27 Joeny Ang File Added: gtk2-textrect-and-regions.patch
2020-06-19 10:27 Joeny Ang File Added: save-restore-dc-test-01.zip
2020-06-19 10:39 Joeny Ang Note Edited: 0123474 View Revisions
2020-06-24 11:13 Juha Manninen Assigned To => Juha Manninen
2020-06-24 11:13 Juha Manninen Status new => assigned
2020-06-24 11:14 Juha Manninen Status assigned => resolved
2020-06-24 11:14 Juha Manninen Resolution open => fixed
2020-06-24 11:14 Juha Manninen Fixed in Revision => r63436
2020-06-24 11:14 Juha Manninen LazTarget => -
2020-06-24 11:14 Juha Manninen Widgetset GTK 2 => GTK 2
2020-06-24 11:14 Juha Manninen Note Added: 0123550
2020-06-25 02:52 Joeny Ang Note Added: 0123587
2020-07-24 16:33 Joeny Ang Note Added: 0124306
2020-07-24 16:33 Joeny Ang Note Edited: 0124306 View Revisions
2020-07-24 17:20 wp Status resolved => feedback
2020-07-24 17:20 wp Resolution fixed => open
2020-07-24 17:20 wp Note Added: 0124307
2020-07-25 03:22 Joeny Ang Note Added: 0124321
2020-07-25 03:22 Joeny Ang Status feedback => assigned
2020-07-25 09:24 Juha Manninen Relationship added related to 0037409
2020-07-25 09:38 Juha Manninen Note Added: 0124323
2020-07-25 10:57 Joeny Ang Note Added: 0124325
2020-07-25 11:13 Joeny Ang Note Added: 0124326
2020-07-27 09:15 Joeny Ang Note Added: 0124356
2020-07-27 09:15 Joeny Ang File Added: gtk2-textrect-and-regions-pt2.patch
2020-07-28 22:56 Juha Manninen Status assigned => resolved
2020-07-28 22:56 Juha Manninen Resolution open => fixed
2020-07-28 22:56 Juha Manninen Fixed in Revision r63436 => r63436, r63665
2020-07-28 22:56 Juha Manninen Widgetset GTK 2 => GTK 2
2020-07-28 22:56 Juha Manninen Note Added: 0124370
2020-09-23 03:44 Joeny Ang Status resolved => closed
2020-09-23 03:44 Joeny Ang Note Added: 0125768
2020-11-11 09:55 Juha Manninen Relationship added related to 0038062
2020-11-11 10:13 Juha Manninen Status closed => assigned
2020-11-11 10:13 Juha Manninen Resolution fixed => open
2020-11-11 10:13 Juha Manninen Note Added: 0126833
2020-11-13 10:42 Juha Manninen Status assigned => resolved
2020-11-13 10:42 Juha Manninen Resolution open => fixed
2020-11-13 10:42 Juha Manninen Widgetset GTK 2 => GTK 2
2020-11-13 10:42 Juha Manninen Note Added: 0126874
2020-11-28 10:24 Joeny Ang Status resolved => closed
2020-11-28 10:24 Joeny Ang Note Added: 0127227