Opened 4 years ago

Closed 4 years ago

#12206 closed defect (fixed)

wxGrid::AutoSizeColumns() asserts with 0 rows

Reported by: mghie Owned by: vadz
Priority: normal Milestone:
Component: wxGrid Version: stable-latest
Keywords: grid autosize regression Cc:
Blocked By: Blocking:
Patch: no

Description

wxWidgets trunk, updated 2010-07-06

This is a regression compared to version 2.8, where calling wxGrid::AutoSizeColumns() worked fine even when there were no rows in the grid - it would then size the columns according to the header text widths.

Change History (5)

comment:1 Changed 4 years ago by vadz

  • Keywords regression added
  • Milestone 2.9.1 deleted

What's the easiest way to reproduce this?

comment:2 Changed 4 years ago by mghie

Yes, sorry for not providing this in the first place.

Use the grid sample, let the BugsGridTable::GetNumberRows() method return 0. That's line 1261 for the 2.8 branch (where it works, a correctly sized header-only grid appears), and it's line 1507 for the trunk, where it asserts, and on pressing "Cancel" it shows a grid with default-width columns.

comment:3 Changed 4 years ago by vadz

  • Owner set to vadz
  • Status changed from new to accepted

Thanks, I see the bug and it's just due to the use of an uninitialized (or, rather, carefully initialized with invalid value to suppress compiler warnings...) variable. The following patch should fix it:

  • src/generic/grid.cpp

    a b wxGrid::AutoSizeColOrRow(int colOrRow, bool setAsMin, wxGridDirection direction) 
    78657865    HideCellEditControl(); 
    78667866    SaveEditControlValue(); 
    78677867 
    7868     // initialize both of them just to avoid compiler warnings 
    7869     int row = -1, 
     7868    // initialize both of them just to avoid compiler warnings even if only 
     7869    // really needs to be initialized here 
     7870    int row, 
     7871        col; 
     7872    if ( column ) 
     7873    { 
     7874        row = -1; 
     7875        col = colOrRow; 
     7876    } 
     7877    else 
     7878    { 
     7879        row = colOrRow; 
    78707880        col = -1; 
     7881    } 
    78717882 
    78727883    wxCoord extent, extentMax = 0; 
    78737884    int max = column ? m_numRows : m_numCols; 
    78747885    for ( int rowOrCol = 0; rowOrCol < max; rowOrCol++ ) 
    78757886    { 
    78767887        if ( column ) 
    7877         { 
    78787888            row = rowOrCol; 
    7879             col = colOrRow; 
    7880         } 
    78817889        else 
    7882         { 
    7883             row = colOrRow; 
    78847890            col = rowOrCol; 
    7885         } 
    78867891 
    78877892        // we need to account for the cells spanning multiple columns/rows: 
    78887893        // while they may need a lot of space, they don't need all of it in 

Please let me know if you have any problems with it, if none arise I'll commit this in a few days.

comment:4 Changed 4 years ago by mghie

Thank you very much, your patch takes care of the issue without any negative side effects that I could observe. AFAICS this can be closed as fixed when the patch is committed.

comment:5 Changed 4 years ago by VZ

  • Resolution set to fixed
  • Status changed from accepted to closed

(In [64885]) Don't use uninitialized variables in wxGrid::AutoSize().

The code in wxGrid::AutoSizeColOrRow() was using uninitialized col/row
variable if the grid had no rows/columns, resulting in assertion failures when
trying to automatically size the columns of an empty grid.

Do initialize them now and also remove duplicate assignments to the variables
which never change inside the loop.

Closes #12206.

Note: See TracTickets for help on using tickets.