Bug 197953 - [Viewers] Using FocusCellOwnerDrawHighlighter with OwnerDrawLabelProvider
Summary: [Viewers] Using FocusCellOwnerDrawHighlighter with OwnerDrawLabelProvider
Status: NEW
Alias: None
Product: Platform
Classification: Eclipse Project
Component: UI (show other bugs)
Version: 3.3   Edit
Hardware: PC Windows NT
: P3 normal with 2 votes (vote)
Target Milestone: ---   Edit
Assignee: Platform UI Triaged CLA
QA Contact:
URL:
Whiteboard:
Keywords: contributed
Depends on:
Blocks:
 
Reported: 2007-07-26 08:38 EDT by charles Prevot CLA
Modified: 2019-09-06 16:07 EDT (History)
5 users (show)

See Also:


Attachments
A solution to make them work together (10.07 KB, patch)
2007-07-26 08:41 EDT, charles Prevot CLA
no flags Details | Diff
Proposal of modified FocusCellOwnerDrawHighlighter (5.25 KB, text/plain)
2008-02-20 06:13 EST, Pavel Sklenak CLA
no flags Details
patch file against JFace-HEAD (4.74 KB, text/plain)
2008-02-20 10:19 EST, Pavel Sklenak CLA
no flags Details
FocusCellOwnerDrawBorderHighlighter (4.17 KB, text/plain)
2008-05-16 09:00 EDT, Pavel Sklenak CLA
no flags Details
Snippet for demonstrating FocusCellOwnerDrawBorderHighlighter (11.26 KB, text/x-java)
2008-05-16 09:02 EDT, Pavel Sklenak CLA
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description charles Prevot CLA 2007-07-26 08:38:27 EDT
FocusCellOwnerDrawHighlighter doesn't work with OwnerDrawlabelProvider. To make them work together I had to redefine the erase() method from OwnerDrawLabelProvider and test the event.detail for SWT.SELECTED in the paint() method.

More info: http://dev.eclipse.org/newslists/news.eclipse.platform/msg66900.html
Comment 1 charles Prevot CLA 2007-07-26 08:41:04 EDT
Created attachment 74677 [details]
A solution to make them work together
Comment 2 Boris Bokowski CLA 2007-08-03 08:13:36 EDT
Marking for investigation in M2.
Comment 3 Boris Bokowski CLA 2007-09-18 13:19:33 EDT
Moving to M3.
Comment 4 Boris Bokowski CLA 2007-10-29 00:52:03 EDT
Two comments:

			if ((event.detail & SWT.SELECTED) > 0)
				return;

When doing bitwise operations, it is safer to do a '!= 0' comparison. Depending on the value of SWT.SELECTED, the result of the bitwise and operation might be negative.

			Color colorByColumns = new Color(Display.getCurrent(), color.getRed(), event.index*255/_tableViewer.getTable().getColumnCount(), color.getBlue());

You are creating color objects in paint() without disposing of them at some later point. This will leak resources and may lead to a crash if you keep the program running for long enough.

That aside, thanks a lot for the contribution.  Do you think something like this should become part of JFace itself, or would it be enough to add another snippet to our collection?
Comment 5 charles Prevot CLA 2007-10-29 04:49:42 EDT
(In reply to comment #4)
> Two comments:
> 
>                         if ((event.detail & SWT.SELECTED) > 0)
>                                 return;
> 
> When doing bitwise operations, it is safer to do a '!= 0' comparison. Depending
> on the value of SWT.SELECTED, the result of the bitwise and operation might be
> negative.
OK, thank you for helping me debugging my code ;)
> 
>                         Color colorByColumns = new Color(Display.getCurrent(),
> color.getRed(), event.index*255/_tableViewer.getTable().getColumnCount(),
> color.getBlue());
> 
> You are creating color objects in paint() without disposing of them at some
> later point. This will leak resources and may lead to a crash if you keep the
> program running for long enough.
I thought a Color was like other Java objects managed by the garbage collector. Considering your comment, I am surely wrong. Do you have a suggestion where to put the dispose call ? 
> 
> That aside, thanks a lot for the contribution.  Do you think something like
> this should become part of JFace itself, or would it be enough to add another
> snippet to our collection?
In fact I had to write that code because of the display bugs I had in vista with the table viewer. I need to have a table of empty cells (ie. without text nor image) identified by their background color. But, on Vista, there is a highlighting feature (when you have the mouse over a row) that erases the Background color of a cell (provided by a color provider). Thats why I had to work with ownerdraw. 
I think a single snippet for this particular case is a good solution, but having
a way in JFace (or SWT) to manage this highlighting feature would be interesting too.
Comment 6 Boris Bokowski CLA 2007-10-29 14:41:19 EDT
 (In reply to comment #5)
> I thought a Color was like other Java objects managed by the garbage collector.
> Considering your comment, I am surely wrong. Do you have a suggestion where to
> put the dispose call ?

If the color is different for each table cell, a possible solution would be to maintain a list of color objects allocated for a table item, for example by calling TableItem.setData(String, colorArray). Then, hook a dispose listener on each item so that you can dispose of the color objects from the widgetDisposed() method.

I'm removing the target milestone because it is too late to put this in M3, but I'd be happy to add this snippet to our collection when it is ready.
Comment 7 Pavel Sklenak CLA 2008-02-16 06:04:12 EST
FocusCellOwnerDrawHighlighter doesn't support SWT.MULTI style. It seems only possible solution is to define TableCursor but using TableCursor together with OwnerDrawLabelProvider seems as crude solution.
Comment 8 Pavel Sklenak CLA 2008-02-20 06:13:57 EST
Created attachment 90165 [details]
Proposal of modified FocusCellOwnerDrawHighlighter

I tried to detect selected cell by arrows. I modified FocusCellOwnerDrawHighlighter to enable multiselection, and cell selection by keys
Comment 9 Thomas Schindl CLA 2008-02-20 08:02:16 EST
(In reply to comment #8)
> Created an attachment (id=90165) [details]
> Proposal of modified FocusCellOwnerDrawHighlighter
> 
> I tried to detect selected cell by arrows. I modified
> FocusCellOwnerDrawHighlighter to enable multiselection, and cell selection by
> keys
> 

Am I looking at the wrong place but I can't see a difference between the current FocusCellOwnerDrawHighlighter and the one you attached. Please provide your changes as a patch-file against current JFace-HEAD.

Comment 10 Pavel Sklenak CLA 2008-02-20 10:19:16 EST
Created attachment 90189 [details]
patch file against JFace-HEAD

(In reply to comment #9)
I adjusted changes to current head revision (previously was M20070910-0800b). 
Method's names are not appropriate after swiching markFocusedCell/removeSelectionInformation logic so it needs somehow also adjust it...
Comment 11 Boris Bokowski CLA 2008-02-21 11:19:06 EST
Tom, can you take this one? I don't have time to investigate.
Comment 12 Pavel Sklenak CLA 2008-05-16 09:00:25 EDT
Created attachment 100632 [details]
FocusCellOwnerDrawBorderHighlighter

I modified FocusCellOwnerDrawHighlighter to FocusCellOwnerDrawBorderHighlighter. Reason is that I need support for SWT.MULTI style. Also there is event sent when selected cell is changed (typically by using arrows)
Comment 13 Pavel Sklenak CLA 2008-05-16 09:02:00 EDT
Created attachment 100633 [details]
Snippet for demonstrating FocusCellOwnerDrawBorderHighlighter
Comment 14 Thomas Schindl CLA 2010-01-28 17:12:11 EST
multi change because of intenion of stepping back as platform-ui committer
Comment 15 Eclipse Webmaster CLA 2019-09-06 16:07:27 EDT
This bug hasn't had any activity in quite some time. Maybe the problem got resolved, was a duplicate of something else, or became less pressing for some reason - or maybe it's still relevant but just hasn't been looked at yet.

If you have further information on the current state of the bug, please add it. The information can be, for example, that the problem still occurs, that you still want the feature, that more information is needed, or that the bug is (for whatever reason) no longer relevant.