0

In my JTable I have already implemented the selection-preserving logic during updates. Unfortunately I think I have come across some kind of bug. The JTable is updated by a background thread that fetches new data and adds/modifies/removes it from the table model.

The problem shows itself only when an update is performed while the user is dragging the mouse to select rows in the table. What puzzles me the most is that:

  • if I start from a row and move the mouse upward to select the preceding rows, everything works perfectly: the selection is preserved across updates and the user action continues correctly
  • if I start from a row and move the mouse downward to select the following rows, it breaks: when the update fires, the current selection is lost and the new selection starts from the row that is under the pointer when the update fires.

Any clues as why this might be happening?

This is a trimmed down version of the code I'm talking about.

I am using a manually synchronized TreeSet to update the data in background threads (see update). When I'm done, I invoke atomicSwapData that creates the array that will be used by getValueAt.

In updateTable you can see how I implemented the selection-preserving logic I was talking about before. As you can see I tried using the EventQueue.invokeLater method but the problem still appears. Please note that all methods may be called by any thread, not just the EDT.

class MyDataModel extends AbstractTableModel {
    TreeSet<MyDataItem> ht = new TreeSet<MyDataItem>();
    MyDataItem[] _ht = MyDataItem.emptyArray();

    public Object getValueAt(int row, int col) {
        MyDataItem qdi;
        synchronized (_ht) {
            qdi = _ht[row];
        }
        // return one of the members of qdi, based on col
    }

    public void update(String qm, Collection<MyDataItem> toAdd) {
        HashSet<MyDataItem> toDrop = new HashSet<MyDataItem>();
        synchronized (ht) { 
            // do something with ht
        }
        swapData();
    }

    private void swapData() {
        EventQueue.invokeLater(new Runnable() {
            public void run() {
                synchronized(_ht) {
                    MyDataItem[] tmp = ht.toArray(MyDataItem.emptyArray());
                    synchronized(tmp) {
                        _ht = tmp;
                    }
                } 
            }
        });
        updateTable();
    }

    private void updateTable() {
        EventQueue.invokeLater(new Runnable() {
            public void run() {
                synchronized (_ht) {
                    int[] rowIndices = table.getSelectedRows();
                    fireTableDataChanged();
                    for (int row: rowIndices)
                        table.addRowSelectionInterval(row, row);
                }
            }
        });
    }
}
CAFxX
  • 28,060
  • 6
  • 41
  • 66
  • Are you saying you are updating the `TableModel` on another thread then the EDT. Start by fixing that (e.g. use a `SwingWorker` to fetch the new data on a background thread but update the `TableModel` on the EDT once the new data is available) – Robin Mar 28 '12 at 08:51
  • It's probably cleaner to implement it how you're describing it, but I got around it by using synchronized access to the object holding the data inside the TableModel. – CAFxX Mar 28 '12 at 09:38
  • 2
    please edit you question with [SSCCE](http://sscce.org/) , no way :-) – mKorbel Mar 28 '12 at 10:01
  • _already implemented the selection-preserving logic during updates_ sounds fishy: usually the table does so by itself, there is no need to do anything special. My guess would be incorrect update notification - nothing but wild, of course, until you show a SSCCE :-) – kleopatra Mar 28 '12 at 11:37
  • 1
    @CAFxX this code hasn't something with AbstractTableModel, I'm missed there all important methods for creating, holding, modify a JTable based on this model – mKorbel Mar 29 '12 at 12:37
  • what is updateTable doing? If it does what the name suggests (directly trying to refresh the view), that would be the comletely wrong thing ... It _must_ notify its listeners with the appropriate TableModel events, nothing else. – kleopatra Mar 29 '12 at 12:51
  • @kleopatra "To create a concrete TableModel as a subclass of AbstractTableModel you need only provide implementations for the following three methods: getRowCount(), getColumnCount(), getValueAt(int row, int column)". That's exactly what I did. http://docs.oracle.com/javase/1.5.0/docs/api/javax/swing/table/AbstractTableModel.html – CAFxX Mar 29 '12 at 13:22
  • sure, that's enough if the model is immutable/unmodifiable - which yours isn't. So you have to call the appropriate fireXX after _doing something with ht_ – kleopatra Mar 29 '12 at 14:53

2 Answers2

1

For reference, this example continually updates the TableModel from a background thread using EventQueue.invokeLater(). It does not exhibit the anomaly you describe.

Community
  • 1
  • 1
trashgod
  • 203,806
  • 29
  • 246
  • 1,045
  • See the updated question. Using invokeLater does not seem to solve the problem. – CAFxX Mar 29 '12 at 12:36
  • Yes, updating the `TableModel` on the EDT is a necessary, but not sufficient, condition. Do you need to [wrap](http://docs.oracle.com/javase/6/docs/api/java/util/TreeSet.html) your set? – trashgod Mar 29 '12 at 14:58
  • Isn't using `synchronized(ht) { ... }` enough? – CAFxX Mar 29 '12 at 14:59
  • See [*Concurrent Collections*](http://docs.oracle.com/javase/6/docs/api/java/util/concurrent/package-summary.html). – trashgod Mar 29 '12 at 16:36
0

standard painting for a jtable is from top to bottom this might be a reason why it breaks in one direction but not the other.

You will have to be abit more clear and define your 'breaks'

Does it just paints the selection wrong ? then you should be able to fix it by forcing your table to do a full repaint after the update fires

Peter
  • 5,728
  • 20
  • 23