View Issue Details

IDProjectCategoryView StatusLast Update
0017882LazarusLCLpublic2012-07-08 12:54
ReporterPhilAssigned ToJesus Reyes 
PrioritynormalSeveritymajorReproducibilityalways
Status resolvedResolutionfixed 
PlatformMacOSOS XOS Version10.6
Product Version0.9.29 (SVN)Product Build28159 
Target Version1.0.0Fixed in Version1.1 (SVN) 
Summary0017882: TStringGrid pages wrong with fixed rows
DescriptionPaging a grid appears to be counting the fixed rows too in determining how many rows to scroll.

This happens on Windows too.

Please see example project.
TagsNo tags attached.
Fixed in Revisionr37897
LazTarget1.0
WidgetsetGTK, GTK 2, Win32/Win64, WinCE, Carbon, Cocoa, QT, fpGUI, CustomDrawn
Attached Files
  • TestStringGrid2.zip (2,465 bytes)
  • grids.vert-scroll-page.diff (780 bytes)
    Index: lcl/grids.pas
    ===================================================================
    --- lcl/grids.pas	(revision 37842)
    +++ lcl/grids.pas	(working copy)
    @@ -4345,8 +4345,8 @@
         SB_LINEDOWN:   C := CTL + NextRowHeight(FTopleft.Y, 1);
         SB_LINEUP:     C := CTL - NextRowHeight(FTopleft.Y-1, -1);
           // Scrolls one page of lines up / down
    -    SB_PAGEDOWN:   C := CTL + FGCache.ClientHeight;
    -    SB_PAGEUP:     C := CTL - FGCache.ClientHeight;
    +    SB_PAGEDOWN:   C := CTL + (FGCache.ClientHeight - (FGCache.FixedHeight - FFixedRows * FGridLineWidth));
    +    SB_PAGEUP:     C := CTL - (FGCache.ClientHeight - (FGCache.FixedHeight - FFixedRows * FGridLineWidth));
           // Scrolls to the current scroll bar position
         SB_THUMBPOSITION:
           C := Message.Pos;
    

Activities

2010-11-09 01:01

 

TestStringGrid2.zip (2,465 bytes)

Phil

2010-11-26 17:49

reporter   ~0043580

This bug renders the TStringGrid very non-standard as well as non-Delphi compatible. All grids scroll the number of unlocked rows, not the number of visible rows.

Thanks.

-Phil

Jesus Reyes

2010-12-09 00:29

developer   ~0044106

can you please explain with specificity what you mean?, if you say something is wrong but you do not explain how should it work is difficult to reach some agreement.

You have provided an example but you do not say what to do with the example. If you had say press here, type that and you will get this but it's wrong it should be this other way, we could would probably know what to do.

When I implemented the paging on grid I didn't know there was an standard for that, if such standard is documented can you please provide some link?

Phil

2010-12-09 01:42

reporter   ~0044107

Think about it: when you page up and down in a grid (all grids work like this): If there are 2 locked rows and 10 editable rows, then usually the grid will page 9 or 10 rows at a time. 9 would be a good choice if you want to show the first/last row as the last/first row in the new page.

In this example, TStringGrid pages 11 or 12 rows at a time because it appears to include the locked rows in the page count. This means you have to scroll back one or two rows to see the entire "page".

Please just try out other grids and see how they work when there are locked (fixed) rows (or columns).

Jesus Reyes

2010-12-09 02:34

developer   ~0044109

I'm sorry I don't have any other grid here to test at the moment.

I can tell you the grid do not take in count fixed rows for calculating how many rows it will page, only visible rows, if you don't believe me take a look at the code @grids.pas:6238-6247 (r28645) there visiblegrid is used which holds only the current visible (non fixed) cells

Following the example in your note you mention 10+2 = 12 rows which means for example first editable cell is index 2 (only talking here about vertical scrolling) and the last row is index 11, I modified your test project so the grid shows just those rows, as the focus is initially in first row (index 2) I pressed PageDn key and the cursor moved to last row (index 11). 11-2=9, just the number of rows you said would be fine. So I'm confused, if the grid should use sometimes 9 and sometimes 10 rows it's even more confusing, how the grid would know the right number?

Phil

2010-12-09 22:00

reporter   ~0044132

I'm sorry, but don't you see that with the example project I uploaded, that when you page down the first time, instead of rows 0-1 (locked) and rows 2-14 (unlocked), you now see rows 17-29 (just tested it on Windows). What happened to rows 15 and 16? You never see them when paging down. That's what I'm saying is wrong: that's not the way a grid is supposed to behave.

With Delphi it scrolls one less than the number of unlocked rows (also just tested it with the example project). That way the user has a kind of context for where they ended up - i.e. the last unlocked row is not the top unlocked row. That tells them that they haven't scrolled past any rows. Think of a grid with names - if John Smith is in the bottom row when they user pages down and after paging John Smith is now in the top row, the user knows he didn't skip any names.

However, scrolling the exact number of unlocked rows is also acceptable, as I tried to explain above.

It almost sounds as though this has been fixed since I reported it. I just tested with Lazarus from Nov 18, but I'll try a newer version.

Thanks.

-Phil

Phil

2010-12-09 22:49

reporter   ~0044134

I just tested with Laz 28656 on Windows and don't see any difference from what I reported.

Thanks.

-Phil

Tomasz Wieckowski

2010-12-09 23:23

reporter   ~0044135

When I first run your project (WinXP) I see rows from 0 to 14 (0,1 fixed)
when "page down" I see rows from 3 to 15 and next 16 to 28 so it's ok (page up also ok).
On Gtk, start from 0 to 13, page down -> 3 to 14 and 14 to 25 so it's also ok.
I can't reproduce it.

Jesus Reyes

2010-12-10 06:45

developer   ~0044142

Here it is exactly the same as Tomasz has described. And not the jump of (2-14) -> (17-29) Phil describe.

Anyway I noted that the calc was being made using visiblegrid, but visiblegrid include also partially visible col/rows at the borders, even if just one pixel of last row/col which is visible (this is the case of Phil's test under windows) so in r28664 I modified the grid to count only fully visible cells. Please test.

Phil

2010-12-11 20:13

reporter   ~0044183

Would it help if I posted a video that shows what I'm seeing?

Thanks.

-Phil

Jesus Reyes

2010-12-12 07:00

developer   ~0044189

sure, but first you need to give feedback if you have tried r28664 or later...

Zeljan Rikalo

2012-02-11 20:08

developer   ~0056761

@Phil is this fixed with current trunk ?

Zeljan Rikalo

2012-03-16 14:20

developer   ~0057726

Jesus ?

Bart Broersma

2012-06-29 16:06

developer   ~0060738

Lazarus 1.1 r37467 FPC 2.6.0 i386-win32-win32/win64

If I use PgDn Key then the last visible row becomes the first unlocked:
2-13
12-23
22-33 etc.

If I use the scrollbar to "page" (click below the thumbtrack) I get
2-13
15-26
28-39

The latter may be the problem Phil is describing?

2012-07-05 21:13

 

grids.vert-scroll-page.diff (780 bytes)
Index: lcl/grids.pas
===================================================================
--- lcl/grids.pas	(revision 37842)
+++ lcl/grids.pas	(working copy)
@@ -4345,8 +4345,8 @@
     SB_LINEDOWN:   C := CTL + NextRowHeight(FTopleft.Y, 1);
     SB_LINEUP:     C := CTL - NextRowHeight(FTopleft.Y-1, -1);
       // Scrolls one page of lines up / down
-    SB_PAGEDOWN:   C := CTL + FGCache.ClientHeight;
-    SB_PAGEUP:     C := CTL - FGCache.ClientHeight;
+    SB_PAGEDOWN:   C := CTL + (FGCache.ClientHeight - (FGCache.FixedHeight - FFixedRows * FGridLineWidth));
+    SB_PAGEUP:     C := CTL - (FGCache.ClientHeight - (FGCache.FixedHeight - FFixedRows * FGridLineWidth));
       // Scrolls to the current scroll bar position
     SB_THUMBPOSITION:
       C := Message.Pos;

Bart Broersma

2012-07-05 21:15

developer   ~0060869

Attached diff tries to scroll (up/down) only nr. of non-fixed rows.
Not perfect yet, especially if the last row is partly visible (needs another subtraction for tha, but I do not know how to calculate that one).

Bart Broersma

2012-07-08 12:54

developer   ~0060900

Page scrolling using scrollbar now uses the same algorithm to determine how far to sroll by, as the implementation in KeyDown for PgUp and PgDn keys (and a similar algorithm for horizontal scrolling).
PgUp and PgLeft scrolling may still be inaccurate if rows (resp. cols) above (resp. to the left) have different dimensions than those in visible grid.

Please test and close if OK

Issue History

Date Modified Username Field Change
2010-11-09 01:01 Phil New Issue
2010-11-09 01:01 Phil File Added: TestStringGrid2.zip
2010-11-09 01:01 Phil Widgetset => Carbon
2010-11-16 12:43 Vincent Snijders LazTarget => 1.0
2010-11-16 12:43 Vincent Snijders Assigned To => Jesus Reyes
2010-11-16 12:43 Vincent Snijders Status new => assigned
2010-11-16 12:43 Vincent Snijders Target Version => 1.0.0
2010-11-26 17:49 Phil Note Added: 0043580
2010-12-09 00:29 Jesus Reyes Note Added: 0044106
2010-12-09 01:42 Phil Note Added: 0044107
2010-12-09 02:34 Jesus Reyes Note Added: 0044109
2010-12-09 22:00 Phil Note Added: 0044132
2010-12-09 22:49 Phil Note Added: 0044134
2010-12-09 23:23 Tomasz Wieckowski Note Added: 0044135
2010-12-10 06:45 Jesus Reyes Note Added: 0044142
2010-12-11 20:13 Phil Note Added: 0044183
2010-12-12 07:00 Jesus Reyes Note Added: 0044189
2012-02-11 20:08 Zeljan Rikalo Note Added: 0056761
2012-02-11 20:08 Zeljan Rikalo Status assigned => feedback
2012-03-16 14:20 Zeljan Rikalo Note Added: 0057726
2012-03-16 14:20 Zeljan Rikalo Status feedback => acknowledged
2012-03-16 14:28 Vincent Snijders Status acknowledged => assigned
2012-06-29 16:06 Bart Broersma Note Added: 0060738
2012-07-05 21:13 Bart Broersma File Added: grids.vert-scroll-page.diff
2012-07-05 21:15 Bart Broersma Note Added: 0060869
2012-07-08 12:54 Bart Broersma Fixed in Revision => r37897
2012-07-08 12:54 Bart Broersma Widgetset Carbon => GTK, GTK 2, Win32/Win64, WinCE, Carbon, Cocoa, QT, fpGUI, CustomDrawn
2012-07-08 12:54 Bart Broersma Status assigned => resolved
2012-07-08 12:54 Bart Broersma Fixed in Version => 1.1 (SVN)
2012-07-08 12:54 Bart Broersma Resolution open => fixed
2012-07-08 12:54 Bart Broersma Note Added: 0060900