TableListBox BUG

General JUCE discussion
1

Using “tip” as of Thursday 26th 2012 on Mac OS X (Lion) - although the bug should be independent of platform.

Note: This is only a bug if, as the docs state, TableListBox can have a “unique” custom component on each “cell”.

Issue: “Ghost” components show up on cells.

Steps to repro:

  • Create a TableListBox with more rows than it can display (so that it adds a scroll bar).
  • Implement “refreshComponentForCell()” and return a custom component “only for row one, column one”
  • scroll down

Results: The row that was offscreen, which now, after scrolling, it shows up, contains the custom component of the very first row.

Expected results: The only row that should have a custom component is the very first one.

Detailed explanation: Currently the code has an optimization that assumes that for a given columnID, all of the rows will have the same custom component. When scrolling, JUCE simply reuses components that were previously visible and now they are not. The problem with this is that when scrolling, the update ends up calling “refreshComponentForCell()” reusing a row that contains a component, passing the pointer to the component of the first row. My code catches that and asserts, and returns nullptr, but even then, the row displays the component of the first row.
The current code works fine if all of the rows for that column id use the same component, it simply needs to be updated with the values of the new row. When the components on each row are “unique”, this whole scheme fails.

Cheers

2

Sorry, maybe I’m being a bit slow, but I’m struggling to see your point… Are you saying that when you use the table according to its guidelines, then it fails? Or do you just mean that you want to do something a bit off-piste that doesn’t fit with the way it works?

3

Hi Jules, thanks for responding.

I believe I’m using it according to it’s guidelines, but I would understand if you say it is a “corner case”.
I’ll try to be more specific:
Say you have a TableListBox with 3 rows and 3 columns, yet the component it’s sized in such way that only rows 1 and 2 are visible (which means, the TableListBox will show a scroller). Then, only the cell in row 1, column 1 returns a custom component (say a button). Now scroll down. Voilá. Row 3 column 1 will also show the button.

When debugging, I noticed that this button is the same one that had been created for row 1, and it shows even if I return nullptr when called in refreshComponentForCell().
I believe this happens because of a nice optimization you have that only creates as many Row components as they are visible, and then you “recycle” them when scrolling. Most of the times, this is sufficient, as all of the rows would have the same type of custom component for a given column, so updating the state of that custom component does the trick. This breaks with my case.

Hope it makes more sense. If not, let me know and I can send you a small test app that shows what I’m seeing.

Thanks again,
pitic

4

I ran into this too. There’s an old thread with a slightly outdated fix, but this works:

// in juce_tablelistbox.cpp: update()

if (newRow != row || isNowSelected != isSelected)
{
      row = newRow;
      isSelected = isNowSelected;
      columnComponents.clear(); // add this line
      repaint();
}
1 Like
5

Well… daven’s suggestion will certainly sort this out, but the fact that you’re having problems looks to me like your refreshComponentForCell() method isn’t doing its job correctly.

refreshComponentForCell will always be called for all the components that are being recycled. If any of them are being left behind unexpectedly, then it kind of implies that your refreshComponentForCell is failing to delete the unwanted ones…?

6

Both interesting responses:
Dave, thanks for the hint. As a last resort I may use your suggestion, but I would rather a) not change juce’s source code, and b) if it actually happens to be a bug, it would be best if it gets fixed for everyone.
Jules, you left me speechless. It is true that I am not deleting the components, but when and how would I know it is their turn to get deleted? I simply check if that cell should have a custom component (based on my underlying data), if yes, then I create a new one if needed, and then update the “coordinates” of the component. If no, then I return nullptr. I’m not sure I could know that “it’s time to delete one of the custom components”… here is some code that shows how I’m handling the refreshComponentForCell call.


    if (this->ShouldHaveCustomComponent(rowNumber, columnId) )
    {
        MyCustomComponent* myCell = (MyCustomComponent*)existingComponentToUpdate;
        
        // If an existing component is being passed-in for updating, lets re-use it, otherwise create one.
        if (myCell == nullptr)
            myCell = new MyCustomComponent();
        
        // Update our cell with its position within the grid
        myCell->setCellLocation(rowNumber, columnId );
        
        return myCell;
    }
    else
    {
        // for any other column, just return 0, as we'll be painting these columns directly.
        jassert (existingComponentToUpdate == nullptr);
        return nullptr;
    }

thanks

7

Isn’t it asserting on the line

jassert (existingComponentToUpdate == nullptr);

?

The way it works is that if there’s a component which gets recycled for a cell, then it’ll be passed to your callback, and you should delete it if you don’t want to use it. I can’t see any code paths that don’t either give you the chance to deal with the component yourself, or delete it.

8

Yes, it asserts right there, and even though after the assert it returns the nullptr, the component shows up on that row.

As you suggest, when that happens no I’ve added a line to delete the component, and now it works like a charm.

Somehow I misunderstood from the docs that returning nullptr would do the trick. Sorry for the bug-less bug report. I’ll try to dig deeper next time.

Cheers.

9

Ok, thanks… Maybe I need to make the docs a bit more explicit?

10

Perhaps this could help…
Original comment:

/**
...
If you don't need a custom component for the specified cell, then return 0.
...
    */

You could append to the end of that line: “and delete existingComponentToUpdate if not null.”

Thanks again.

11

Thanks, I’ll do that!