View Issue Details
| ID | Project | Category | View Status | Date Submitted | Last Update |
|---|---|---|---|---|---|
| 0037219 | Lazarus | LCL | public | 2020-06-16 10:53 | 2020-08-06 23:09 |
| Reporter | Joeny Ang | Assigned To | Juha Manninen | ||
| Priority | normal | Severity | minor | Reproducibility | always |
| Status | resolved | Resolution | fixed | ||
| Product Version | 2.1 (SVN) | ||||
| Summary | 0037219: GTK2: TextRect and regions | ||||
| Description | TCanvas.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 | ||||
| Tags | No tags attached. | ||||
| Fixed in Revision | r63436, r63665 | ||||
| LazTarget | - | ||||
| Widgetset | GTK 2 | ||||
| Attached Files |
| ||||
| related to | 0037409 | resolved | Mattias Gaertner | Dangling pointer access in TGtk2WidgetSet.CombineRgn |
|
|
|
|
|
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 :) 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;
|
|
|
Applied, thanks. I removed the commented lines. They can be restored from revision control if needed. |
|
|
Great! Thanks :) |
|
|
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 :) |
|
|
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. |
|
|
@wp, ok, thanks :) |
|
|
> 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);"? |
|
|
> 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 :) |
|
|
Uhh... not as easy as it looks... I'll do some more tests. :) |
|
|
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;
{------------------------------------------------------------------------------
|
|
|
Applied the last patch. Thanks! |
| 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 |
