View Issue Details
ID | Project | Category | View Status | Date Submitted | Last Update |
---|---|---|---|---|---|
0034461 | Patches | LCL | public | 2018-10-26 11:45 | 2020-03-11 15:57 |
Reporter | BBaz | Assigned To | Zeljan Rikalo | ||
Priority | normal | Severity | minor | Reproducibility | always |
Status | closed | Resolution | fixed | ||
Summary | 0034461: TreeItem text is centered instead of left justified | ||||
Description | The text of a tree item is drawn in the center of its draw rectangle. Most of the time the draw rectangle is accurately set so the text appears to be left justified but this is not the case. Under certain circumstances this is visible (Qt4 widget set + treeview inside a anchordocking form) because the draw rectangle is bigger than it should. See pictures showing the problem here https://forum.lazarus.freepascal.org/index.php/topic,42999.msg300279.html#msg300279 | ||||
Additional Information | Please include in next Lazarus 2.0 release candidate | ||||
Tags | No tags attached. | ||||
Fixed in Revision | 59647 | ||||
LazTarget | - | ||||
Widgetset | GTK 2, Win32/Win64, QT, QT5 | ||||
Attached Files |
|
related to | 0018285 | closed | Paul Ishenin | Lazarus | Treeview do not draw text in middle by default |
related to | 0031037 | resolved | Ondrej Pokorny | Lazarus | Mac: fixed dpi 72 should be changed to Windows-value 96 |
related to | 0034625 | closed | Zeljan Rikalo | Lazarus | Qt/Qt5: Wrong PPI calculation on Mac after r57679 |
related to | 0034627 | assigned | Zeljan Rikalo | Lazarus | Wrong treeview tooltip hint position after 0034461 / r34461 |
|
fix_treeitem_text_centered.patch (1,080 bytes)
From 0f945726ee5243ed194b313e1ed5531d8f637056 Mon Sep 17 00:00:00 2001 From: Basile Burg <basile.b@gmx.com> Date: Fri, 26 Oct 2018 11:36:08 +0200 Subject: [PATCH] fix tree item text justification, it was centered --- lcl/include/treeview.inc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lcl/include/treeview.inc b/lcl/include/treeview.inc index 2802785d..055c27e6 100644 --- a/lcl/include/treeview.inc +++ b/lcl/include/treeview.inc @@ -5238,9 +5238,9 @@ var end; if (tvoThemedDraw in Options) then - ThemeServices.DrawText(Canvas, Details, AText, NodeRect, DT_CENTER or DT_VCENTER or DT_SINGLELINE or DT_NOPREFIX, 0) + ThemeServices.DrawText(Canvas, Details, AText, NodeRect, DT_LEFT or DT_VCENTER or DT_SINGLELINE or DT_NOPREFIX, 0) else - DrawText(Canvas.Handle, PChar(AText), -1, NodeRect, DT_CENTER or DT_VCENTER or DT_SINGLELINE or DT_NOPREFIX); + DrawText(Canvas.Handle, PChar(AText), -1, NodeRect, DT_LEFT or DT_VCENTER or DT_SINGLELINE or DT_NOPREFIX); if NeedUnderline then begin -- 2.17.2 |
|
Sorry my description mentioned Qt4 but it's well just Qt5 that was affected. |
|
Sorry first patch put items too much to the left. Second patch add a DPI-aware space. |
|
fix_treeitem_text_centered2.patch (1,169 bytes)
From c9518a8cfae9f22cb35616b1dc1109c457172d85 Mon Sep 17 00:00:00 2001 From: Basile Burg <basile.b@gmx.com> Date: Fri, 26 Oct 2018 11:36:08 +0200 Subject: [PATCH] fix tree item text justification, it was centered --- lcl/include/treeview.inc | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/lcl/include/treeview.inc b/lcl/include/treeview.inc index 2802785d..7122d391 100644 --- a/lcl/include/treeview.inc +++ b/lcl/include/treeview.inc @@ -5237,10 +5237,11 @@ var Canvas.Font.Color := FHotTrackColor; end; + NodeRect.Offset(ScaleX(2, 96), 0); if (tvoThemedDraw in Options) then - ThemeServices.DrawText(Canvas, Details, AText, NodeRect, DT_CENTER or DT_VCENTER or DT_SINGLELINE or DT_NOPREFIX, 0) + ThemeServices.DrawText(Canvas, Details, AText, NodeRect, DT_LEFT or DT_VCENTER or DT_SINGLELINE or DT_NOPREFIX, 0) else - DrawText(Canvas.Handle, PChar(AText), -1, NodeRect, DT_CENTER or DT_VCENTER or DT_SINGLELINE or DT_NOPREFIX); + DrawText(Canvas.Handle, PChar(AText), -1, NodeRect, DT_LEFT or DT_VCENTER or DT_SINGLELINE or DT_NOPREFIX); if NeedUnderline then begin -- 2.17.2 |
|
Maybe DT_CENTER is there because of icon ? There must be reason why only Qt/Qt5 shows this problem / and / or why eg Gtk draws DT_LEFT while DT_CENTER is added into flags. |
|
> Maybe DT_CENTER is there because of icon ? I don't see the relationship between the icon and the text justification > There must be reason why only Qt/Qt5 shows this problem / and / or why eg Gtk draws DT_LEFT while DT_CENTER is added into flags. About gtk i think it's correct because the rectangle to draw has exactly the right width. About Qt5, yes thee's an unknown here and that's caused by the reparenting by AnchorDocking. Note that anyway i think that DT_LEFT will be more performant than DT_CENTER. |
|
IMO, DT_LEFT is more logical to me but must find out why DT_CENTER is there. |
|
Ok, DT_CENTER is added in r24816 when ThemeServices text drawing is introduced, so maybe, it's just typo. https://svn.freepascal.org/cgi-bin/viewvc.cgi/trunk/lcl/include/treeview.inc?root=lazarus&r1=24815&r2=24816 |
|
I think the answer lies in revision 24816: >>>> OLD CurTextRect.Right := x + TextWidth(Node.Text) + 1; Font.Color := InvertColor(Brush.Color); FillRect(CurTextRect); TextOut(x + 1, TextY, Node.Text); <<<<< The text and background color, where just drawn into the rect. >>>>> NEW CurTextRect.Right := x + TextWidth(Node.Text) + Indent div 2; DrawNodeText(NodeSelected, CurTextRect, Node.Text); <<<<< The rect is extended by half of indent (indent is the amount of pixels a childnode is moved to the right from its parent) Together with dt_center, this means, that the background extends by a quarter of indent on either side. It may make sense to have a bit of extra padding on both sides. Not sure why/if that should rely on "indent"? |
|
|
|
https://bugs.freepascal.org/view.php?id=18285 is not broken by the patch. Text is drawn after image, whatever is its size |
|
0018285 is about DT_VCENTER. (vertical) This is about DT_CENTER (horizontal) I agree DT_CENTER is a bad choice. But removing it, means to restore the horizontal padding by other means. The image shows, that after the patch, there is more blue padding on the right, than on the left. And maybe at the same time (not really part of this fix), making them independent of "Indent". And making them DPI aware (or bound to font size?) |
|
issue34461.diff (1,195 bytes)
Index: lcl/include/treeview.inc =================================================================== --- lcl/include/treeview.inc (revision 59594) +++ lcl/include/treeview.inc (working copy) @@ -5237,10 +5237,11 @@ Canvas.Font.Color := FHotTrackColor; end; + NodeRect.Offset(ScaleX(2, 96), 0); if (tvoThemedDraw in Options) then - ThemeServices.DrawText(Canvas, Details, AText, NodeRect, DT_CENTER or DT_VCENTER or DT_SINGLELINE or DT_NOPREFIX, 0) + ThemeServices.DrawText(Canvas, Details, AText, NodeRect, DT_LEFT or DT_VCENTER or DT_SINGLELINE or DT_NOPREFIX, 0) else - DrawText(Canvas.Handle, PChar(AText), -1, NodeRect, DT_CENTER or DT_VCENTER or DT_SINGLELINE or DT_NOPREFIX); + DrawText(Canvas.Handle, PChar(AText), -1, NodeRect, DT_LEFT or DT_VCENTER or DT_SINGLELINE or DT_NOPREFIX); if NeedUnderline then begin @@ -5363,7 +5364,7 @@ begin CurTextRect := NodeRect; CurTextRect.Left := x; - CurTextRect.Right := x + TextWidth(Node.Text) + RealIndent div 2; + CurTextRect.Right := x + TextWidth(Node.Text) + (FDefItemSpace * 2); DrawNodeText(NodeSelected, CurTextRect, Node.Text); end; |
|
Please test with my patch. It does not use Indent but internal FDefItemSpace which is applied for x coordinate. Text should be centered in this case. |
|
Please test and close if ok. This is candidate to be merged into 2.0.RC3 after testing. |
|
It does not look good under qt/qt5 on macOSX. Maybe because of virtual machine, not sure, but maybe because all DARWIN (carbon,cocoa, qt, qt5) target sets default PPI to 96, not 72 so our basic calculation is wrong. EDIT: I can confirm that problem on MacOS comes from hardcoded PixelsPerInch values for all supported targets (carbon,cocoa,qt and qt5). If it's default from ws (72) then drawing of treeview items is same on all widgetsets. EDIT 2: Hardcoded values are in all mentioned widgetsets inside Widgetset.InitApp function. |
|
Just a note: the bug can be observed on win32 as well. Some treeview items are shifted right by 1px. I assume it is the result of DT_CENTER and "CurTextRect.Right := x + TextWidth(Node.Text) + RealIndent div 2;" |
|
I've found root of problem in Qt/Qt5. It's longstanding bug in Qt with wrong result of QFontMetrics() on Mac - read it as QFontMetrics_width('some text') is wrong when it calculates via QFont_pixelSize() under Mac. I'll fix it in Qt LCL. Thanks for investigation, but I've already fixed that, treviewitem text drawing does not use DT_CENTER anymore, but DT_LEFT. |
|
> Thanks for investigation, but I've already fixed that, treviewitem text drawing does not use DT_CENTER anymore, but DT_LEFT. Yes, I know. I should have better used past tense: "the bug COULD be observed". Btw. why do you use "NodeRect.Offset(ScaleX(2, 96), 0);" in your patch - a scaled value of 2 instead of FDefItemSpace: OffsetRect(NodeRect, FDefItemSpace, 0); |
|
NodeRect.Offset(ScaleX(2, 96), 0); is just copy/pasted from reporter's patch, and I forgot to fix it. Thanks for mentioning that. |
Date Modified | Username | Field | Change |
---|---|---|---|
2018-10-26 11:45 | BBaz | New Issue | |
2018-10-26 11:45 | BBaz | File Added: fix_treeitem_text_centered.patch | |
2018-10-26 11:47 | BBaz | Note Added: 0111566 | |
2018-10-26 12:04 | BBaz | Note Added: 0111569 | |
2018-10-26 12:05 | BBaz | File Added: fix_treeitem_text_centered2.patch | |
2018-10-26 13:30 | Zeljan Rikalo | Assigned To | => Zeljan Rikalo |
2018-10-26 13:30 | Zeljan Rikalo | Status | new => assigned |
2018-10-26 13:32 | Zeljan Rikalo | Note Added: 0111573 | |
2018-10-26 13:44 | BBaz | Note Added: 0111574 | |
2018-10-26 14:08 | Zeljan Rikalo | Relationship added | related to 0018285 |
2018-10-26 14:09 | Zeljan Rikalo | Note Added: 0111577 | |
2018-10-26 14:24 | Zeljan Rikalo | Note Added: 0111578 | |
2018-10-26 14:58 | Martin Friebe | Note Added: 0111579 | |
2018-10-26 17:35 | BBaz | File Added: 0018285_still_ok.png | |
2018-10-26 17:36 | BBaz | Note Added: 0111581 | |
2018-10-26 17:44 | Martin Friebe | Note Added: 0111582 | |
2018-10-26 17:45 | Martin Friebe | Note Edited: 0111582 | View Revisions |
2018-11-18 18:24 | Zeljan Rikalo | File Added: issue34461.diff | |
2018-11-18 18:25 | Zeljan Rikalo | LazTarget | => - |
2018-11-18 18:25 | Zeljan Rikalo | Note Added: 0112049 | |
2018-11-18 18:25 | Zeljan Rikalo | Status | assigned => feedback |
2018-11-24 16:50 | Zeljan Rikalo | Fixed in Revision | => 59647 |
2018-11-24 16:50 | Zeljan Rikalo | Widgetset | QT => GTK 2, Win32/Win64, QT, QT5 |
2018-11-24 16:50 | Zeljan Rikalo | Note Added: 0112172 | |
2018-11-24 16:50 | Zeljan Rikalo | Status | feedback => resolved |
2018-11-24 16:50 | Zeljan Rikalo | Resolution | open => fixed |
2018-11-30 18:54 | Zeljan Rikalo | Note Added: 0112283 | |
2018-11-30 19:05 | Zeljan Rikalo | Note Edited: 0112283 | View Revisions |
2018-11-30 19:13 | Zeljan Rikalo | Note Edited: 0112283 | View Revisions |
2018-11-30 19:18 | Zeljan Rikalo | Relationship added | related to 0031037 |
2018-12-01 09:56 | Zeljan Rikalo | Relationship added | related to 0034625 |
2018-12-01 12:00 | Ondrej Pokorny | Note Added: 0112288 | |
2018-12-01 14:08 | Zeljan Rikalo | Note Added: 0112291 | |
2018-12-01 15:31 | Ondrej Pokorny | Note Added: 0112296 | |
2018-12-01 17:52 | Zeljan Rikalo | Relationship added | related to 0034627 |
2018-12-01 17:59 | Zeljan Rikalo | Note Added: 0112305 | |
2020-03-11 15:57 | BBaz | Status | resolved => closed |