Community
Participate
Working Groups
HEAD StyledCellLabelProvider uses a lot of CPU in TableItem#getBounds(..) for each call to update(ViewerCell), even if owner draw is disabled. At least when owner draw is disabled, the manual refresh is unnecessary. To see one instance of the performance problem, disable colored labels, run the JUnit test attached to bug 196140 comment 2, switch to Show Tests in Hierarchy, and then toggle Show Failures Only. This took about 2s in 3.3.2, but more than 20s in HEAD. Adding if (isOwnerDrawEnabled()) in StyledCellLabelProvider.update(ViewerCell) solved the problem for me, but I'm not sure whether that's the right fix. Maybe it should be done in OwnerDrawLabelProvider, because that class already allows to toggle owner traw with setOwnerDrawEnabled(..). I'm investigating whether the JUnit view could do anything to improve the situation when colored labels are enabled. I think the brute-force redrawing of every updated cell could also be improved.
I tried to avoid updating of individual items: I set the TableViewer's input to null, remove the filter, and then set the input again. But this also triggers a call to #getBounds(..) for every item in the table. I really don't know how I could solve the performance problems from bug 228017 while keeping the labels colored (apart from using a virtual table, which is not an option for 3.4).
(In reply to comment #0) > Adding > if (isOwnerDrawEnabled()) > in StyledCellLabelProvider.update(ViewerCell) solved the problem for me, [..] That breaks when owner draw gets disabled. When I toggle the preference, colored items don't lose their color until they are redrawn.
This is the code in OwnerDrawLabelProvider: public void update(ViewerCell cell) { // Force a redraw Rectangle cellBounds = cell.getBounds(); cell.getControl().redraw(cellBounds.x, cellBounds.y, cellBounds.width, cellBounds.height, true); } Clearly no call to this code is needed if there is no owner draw is enabled (I'll fix this) And I'm wondering if it is required at all. item.setText(..) also seems to issue a refresh, even in owner draw mode. Question to Steve. Is that intended or a bug?
ok, as Markus mentions. The only time we really need the redraw at the moment is when the only change is a change in the colored ranges.
It issues a refresh for the area that would be changed if there was no owner draw (on Windows, this can refresh the whole row). I think SWT should be changed to guaratee that the entire row is refreshed.
Martin, are you going to take this defect?
> I think SWT should be > changed to guaratee that the entire row is refreshed. So if I understand this right, we don't need to force a redraw in owner draw mode if we know that at least the text, the image, for- and background color or font has changed on the item, correct? What bugs me a bit: Most label providers are also font and color providers and so practically every update sets everything (text, image, font, ...) on an item. Each set does a redraw. Is there any way to set them all at once, or delay the redraw until all is set?
> Most label providers are also font and color providers and > so practically every update sets everything (text, image, font, ...) on an > item. Each set does a redraw. I just found out that's not true, at least not on WinXP. Each set just invalidates the corresponding rectangle and WinXP paints everything later.
Created attachment 97037 [details] suggested fix A patch to avoid the redraw code in OwnerDrawLabelProvider. It compares the old and new style ranges and if there's a difference, it makes sure that there is a refreh by forcing a label change on the item.
... or we could figure out what is going in in SWT and fix it there.
Martin, your patch does not remove the call to getBounds, was that intentional?
I am going to investigate this some more.
There are two problems here, one having to do with getBounds, and another one related to removing all but one table item. I have opened new bugs against SWT: bug 228285 for getBounds, and bug 228286 for remove. I am keeping this bug open - depending on the resolution of bug 228285, we should try to avoid calling getBounds.
(In reply to comment #11) > Martin, your patch does not remove the call to getBounds, was that intentional? As only implementors of update(cell) know if they really changed anything, I think we can't solve this in general. And as OwnerDrawLabelProvider.update has been around since 3.3, I'm hesitating to remove the code there. But it would be nice to avoid the call to cell.getBounds() in update as for computing the bounds so much is involved. It would be nicer to use something like item.repaint() and let SWT decide how this is done most efficiently. My patch for the DelegatingStyledCellLabelProvider avoids the call to super.update (and getBounds) by making the assumption that changing the text of the Tree/TableItem will repaint the item (I hope that assumption is correct).
Boris, can you release the patch from comment 9?
I've taken Martin's request to release the patch as his +1. Released to HEAD. Will be in the 20080507 I build.
