Bugzilla – Full Text Bug Listing
| Summary: | Inadequate width for event bounds in OwnerDrawLabelProvider.paint() | ||||||
|---|---|---|---|---|---|---|---|
| Product: | [Eclipse Project] Platform | Reporter: | Simeon Andreev <simeon.danailov.andreev> | ||||
| Component: | UI | Assignee: | Platform-UI-Inbox <Platform-UI-Inbox> | ||||
| Status: | CLOSED WONTFIX | QA Contact: | |||||
| Severity: | normal | ||||||
| Priority: | P3 | CC: | ericwill, loskutov, xixiyan | ||||
| Version: | 4.9 | ||||||
| Target Milestone: | --- | ||||||
| Hardware: | PC | ||||||
| OS: | Linux | ||||||
| See Also: | https://bugs.eclipse.org/bugs/show_bug.cgi?id=531882 | ||||||
| Whiteboard: | |||||||
| Attachments: |
|
||||||
The bug is still seen in: Eclipse SDK Version: 2019-03 (4.11) Build id: I20190108-1800 The workaround in our product is to hard-code paint width, which is not nice. Looks like in org.eclipse.swt.widgets.Table.rendererRender(long, long, long, long, long, long, long, long) there is a call to org.eclipse.swt.widgets.Widget.gtk_cell_renderer_get_preferred_size(long, long, int[], int[]). This used to return the bounds set by the measure call, but no longer does. This is a regression from the fix for bug 531882: https://git.eclipse.org/c/platform/eclipse.platform.swt.git/commit/?id=90b11d0439eaf39696ee5c80da07e42a77c9ce34 If I revert the following change, the behaviour is as it was in 4.8: diff --git a/bundles/org.eclipse.swt/Eclipse SWT/gtk/org/eclipse/swt/widgets/Table.java b/bundles/org.eclipse.swt/Eclipse SWT/gtk/org/eclipse/swt/widgets/Table.java index ce861fcdb5..405b605468 100644 --- a/bundles/org.eclipse.swt/Eclipse SWT/gtk/org/eclipse/swt/widgets/Table.java +++ b/bundles/org.eclipse.swt/Eclipse SWT/gtk/org/eclipse/swt/widgets/Table.java @@ -2868,7 +2868,7 @@ void sendMeasureEvent (long /*int*/ cell, long /*int*/ width, long /*int*/ heigh if (contentHeight [0] < rect.height) contentHeight [0] = rect.height; if (width != 0) C.memmove (width, contentWidth, 4); if (height != 0) C.memmove (height, contentHeight, 4); - GTK.gtk_cell_renderer_set_fixed_size (cell, -1, contentHeight [0]); + GTK.gtk_cell_renderer_set_fixed_size (cell, contentWidth [0], contentHeight [0]); } } } Hi Xi, what is the reason for the following change? https://git.eclipse.org/c/platform/eclipse.platform.swt.git/diff/bundles/org.eclipse.swt/Eclipse%20SWT/gtk/org/eclipse/swt/widgets/Table.java?id=90b11d0439eaf39696ee5c80da07e42a77c9ce34 See the attached snippet, its no longer possible to set a specific width during OwnerDrawLabelProvider.measure() event. E.g. see example in: http://www.vogella.com/tutorials/EclipseJFaceTableAdvanced/article.html#styledcelllabelprovider-and-ownerdrawlabelprovider In particular the example code here would not work as intended: colHelloAndIcon.setLabelProvider(new OwnerDrawLabelProvider() { @Override protected void measure(Event event, Object element) { Rectangle rectangle = ICON.getBounds(); event. setBounds(new Rectangle( event.x, event.y, rectangle.width + 200 , rectangle.height)); } Best regards and thanks, Simeon (In reply to Simeon Andreev from comment #3) > Hi Xi, > > what is the reason for the following change? > > https://git.eclipse.org/c/platform/eclipse.platform.swt.git/diff/bundles/org. > eclipse.swt/Eclipse%20SWT/gtk/org/eclipse/swt/widgets/Table. > java?id=90b11d0439eaf39696ee5c80da07e42a77c9ce34 > > See the attached snippet, its no longer possible to set a specific width > during OwnerDrawLabelProvider.measure() event. It was intended to match the behaviour on GTK2 and Mac where event.width in PaintItem stays the same regardless of what it was set in MeasureItem. i.e. The same event.width is send to both MeasureItem and PaintItem. In MeasureItem, the new modified event.width is only used once to set the size of the table column and never updated to be the one sent to PaintItem. I tried running the attached snippet on Mac and the paint event bound is always equal to 4 as well. Do you get the intended behaviour on a Mac using PaintItem? If so, there might be some other issues on GTK that I'm missing. OK, I do see that behaviour in GTK2 is same as in Eclipse 4.9 and onward. Any idea why the paint event has width of only 4? This seems to be much smaller than the actual cell width. Using the following example: https://wiki.eclipse.org/JFaceSnippets#Snippet010_-_Owner_Draw I don't really see a difference when setting different width, neither in 4.8 nor in 4.11. Setting the height does change the paint event height and also the row height. But the width does nothing. (In reply to Simeon Andreev from comment #5) > OK, I do see that behaviour in GTK2 is same as in Eclipse 4.9 and onward. > > Any idea why the paint event has width of only 4? This seems to be much > smaller than the actual cell width. I assume that the paint event has width of 4 has something to do with the OwnerDrawLabelProvider in jface and its sending the initial 2px boarders when the table wasn't filled with anything. This doesn't happen with a plain SWT snippet though. If you look at https://git.eclipse.org/c/platform/eclipse.platform.swt.git/tree/examples/org.eclipse.swt.snippets/src/org/eclipse/swt/snippets/Snippet230.java The event.width sent to MeasureItem is 56 to account for the text set in the table, and in MeasureItem we add the image width to the column. (In reply to Simeon Andreev from comment #6) > Using the following example: > https://wiki.eclipse.org/JFaceSnippets#Snippet010_-_Owner_Draw > > I don't really see a difference when setting different width, neither in 4.8 > nor in 4.11. Setting the height does change the paint event height and also > the row height. But the width does nothing. I'm not sure about what exactly does the measure function in OwnerDrawLabelProvider does, but it might not be the same as calling MeasureItem with a listener. I also don't see a difference when setting different width with in the snippet in both 4.8 and 4.11 and on Mac as well. The plain SWT snippet attached should does work though. All right, thanks. I'll move back to jface and take a look at the differences between the SWT snippet and the JFace snippet. Its irritating that the paint event doesn't have full width. Attaching an SWT listener to the viewers SWT table yields same event bounds as the ones seen in JFace:
public static void main(String[] args) {
Display display = new Display();
Shell shell = new Shell(display);
shell.setSize(400, 200);
shell.setLayout(new FillLayout());
final TableViewer v = new TableViewer(shell);
v.setLabelProvider(new OwnerDrawLabelProvider() {
@Override
protected void measure(Event event, Object element) {
System.out.println("Measure JFace: " + event.getBounds());
event.setBounds(defaultBounds);
}
@Override
protected void erase(Event event, Object element) {
}
@Override
protected void paint(Event event, Object element) {
System.out.println("Paint Face: " + event.getBounds());
}
});
int m = 2;
String[] contents = new String[m];
for (int i = 0; i < m; ++i) {
TableColumn column = new TableColumn(v.getTable(), SWT.NONE);
column.setWidth(100);
column.setText("column " + i);
contents[i] = "content " + i;
}
v.getTable().setHeaderVisible(true);
v.setContentProvider(new ArrayContentProvider());
v.setInput(contents);
Listener paintListener = event -> {
switch(event.type) {
case SWT.MeasureItem: {
System.out.println("Measure SWT: " + event.getBounds());
break;
}
case SWT.PaintItem: {
System.out.println("Paint SWT: " + event.getBounds());
break;
}
}
};
v.getTable().addListener(SWT.MeasureItem, paintListener);
v.getTable().addListener(SWT.PaintItem, paintListener);
shell.open();
while (!shell.isDisposed()) {
if (!display.readAndDispatch())
display.sleep();
}
display.dispose();
}
Removing this call returns the event bounds to normal: v.setLabelProvider(new OwnerDrawLabelProvider() {
I'll check why that is.
Also note that the event bounds seem to depend on the length of text in the cell. Longer text results in longer event width. Maybe related.
Looks like setting the input after setting a CellLabelProvider results in the too-small event width. Xi, any idea what changes the preferred cell width (since this is where the event width comes from)? I'm having trouble finding what operation of the label provider exactly changes this. Looks like setting the TableViewer input *after* specifying a CellLabelProvider results in clearing the entire table:
"main" #1 prio=5 os_prio=0 tid=0x00007ffff004f800 nid=0x3f8b at breakpoint[0x00007ffff7fcc000]
java.lang.Thread.State: RUNNABLE
at org.eclipse.swt.internal.gtk.GTK.gtk_list_store_set(GTK.java:4023)
at org.eclipse.swt.widgets.TableItem.clear(TableItem.java:227)
at org.eclipse.swt.widgets.Table.clear(Table.java:377)
at org.eclipse.jface.viewers.TableViewer.doClear(TableViewer.java:328)
at org.eclipse.jface.viewers.AbstractTableViewer.internalRefreshAll(AbstractTableViewer.java:695)
at org.eclipse.jface.viewers.AbstractTableViewer.internalRefresh(AbstractTableViewer.java:616)
at org.eclipse.jface.viewers.AbstractTableViewer.internalRefresh(AbstractTableViewer.java:608)
at org.eclipse.jface.viewers.AbstractTableViewer.lambda$0(AbstractTableViewer.java:570)
at org.eclipse.jface.viewers.AbstractTableViewer$$Lambda$17/1843368112.run(Unknown Source)
at org.eclipse.jface.viewers.StructuredViewer.preservingSelection(StructuredViewer.java:1449)
at org.eclipse.jface.viewers.StructuredViewer.preservingSelection(StructuredViewer.java:1410)
at org.eclipse.jface.viewers.AbstractTableViewer.inputChanged(AbstractTableViewer.java:570)
at org.eclipse.jface.viewers.ContentViewer.setInput(ContentViewer.java:289)
at org.eclipse.jface.viewers.StructuredViewer.setInput(StructuredViewer.java:1687)
at jface.bugs.TestOwnerDrawLabelProvider.main(TestOwnerDrawLabelProvider.java:72)
I am guessing this makes the preferred cell width to equal 4 from GTK+ perspective, although both SWT and JFace have correct cell bounds.
Behaviour in Windows 10 is similar. If I set the table viewer input before registering the label provider, I see drawing artefacts and "OK" event width. If I set the table viewer input after registering the label provider, I see no drawing artefacts and 4 px event width (0 px event width on Windows 10). I have no idea how the event bounds are supposed to be used to draw anything in such case. I do see that this snippet adds artificial 100 px to the event width then drawing flags: https://git.eclipse.org/c/platform/eclipse.platform.ui.git/plain/examples/org.eclipse.jface.snippets/Eclipse%20JFace%20Snippets/org/eclipse/jface/snippets/viewers/Snippet010OwnerDraw.java @Override protected void drawFlag(Event event) { Rectangle bounds = event.getBounds(); bounds.width += 100; Seems odd at best. (In reply to Simeon Andreev from comment #10) > Looks like setting the input after setting a CellLabelProvider results in > the too-small event width. > > Xi, any idea what changes the preferred cell width (since this is where the > event width comes from)? I'm having trouble finding what operation of the > label provider exactly changes this. The event width sent to PaintItem is send constantly in the rendererRender method in Table.java here: https://git.eclipse.org/c/platform/eclipse.platform.swt.git/tree/bundles/org.eclipse.swt/Eclipse%20SWT/gtk/org/eclipse/swt/widgets/Table.java#n3141 And in GTK, the width is gotten from calling GTK.gtk_cell_renderer_get_preferred_size: https://git.eclipse.org/c/platform/eclipse.platform.swt.git/tree/bundles/org.eclipse.swt/Eclipse%20SWT/gtk/org/eclipse/swt/widgets/Table.java#n3083 Looks like this is the behaviour on Windows 10, GTK3 and GTK2. So I don't think its meaningful to change it. I've suggested the following workaround for our developers: Rectangle textBounds = ((TableItem) event.item).getTextBounds(event.index); This is very similar to what is done in: org.eclipse.jface.viewers.StyledCellLabelProvider The feedback we got is that this is not nice, since its not independent of the tree/table type. The only other option I see is using the event.gc clipping. Both don't seem to be optimal, but I think they are preferable to touching API that has been as it is for quite some time. |
Created attachment 277476 [details] Snippet to reproduce the problem with. Run and observe no change in paint event bounds despite attempt to do so in measure method. Broken in: Eclipse SDK Version: 4.9 Build id: I20180716-2000 Not broken in: Eclipse SDK Version: Photon (4.8) Build id: I20180611-0500 Snippet: "TestOwnerDrawLabelProvider.java" Observe standard output, normally (in 4.8 and below) its e.g.: Measure: Rectangle {0, 0, 4, 19} Measure: Rectangle {0, 0, 4, 19} Measure: Rectangle {0, 0, 4, 19} Measure: Rectangle {0, 0, 4, 19} Erase: Rectangle {0, 0, 383, 21} Erase: Rectangle {0, 0, 383, 21} Paint: Rectangle {2, 0, 80, 21} Measure: Rectangle {0, 0, 4, 19} Erase: Rectangle {0, 0, 383, 21} Erase: Rectangle {0, 0, 383, 21} Paint: Rectangle {2, 0, 80, 21} Measure: Rectangle {0, 0, 4, 19} Erase: Rectangle {0, 0, 383, 21} Erase: Rectangle {0, 0, 383, 21} Paint: Rectangle {2, 0, 80, 21} Measure: Rectangle {0, 0, 4, 19} Erase: Rectangle {0, 0, 383, 21} Erase: Rectangle {0, 0, 383, 21} Paint: Rectangle {2, 0, 80, 21} Measure: Rectangle {0, 0, 4, 19} Erase: Rectangle {0, 0, 383, 21} Erase: Rectangle {0, 0, 383, 21} Paint: Rectangle {2, 0, 80, 21} Measure: Rectangle {0, 0, 4, 19} Erase: Rectangle {0, 0, 383, 21} Erase: Rectangle {0, 0, 383, 21} Paint: Rectangle {2, 0, 80, 21} Measure: Rectangle {0, 0, 4, 19} Erase: Rectangle {0, 0, 383, 21} Erase: Rectangle {0, 0, 383, 21} Paint: Rectangle {2, 0, 80, 21} Measure: Rectangle {0, 0, 4, 19} Erase: Rectangle {0, 0, 383, 21} Erase: Rectangle {0, 0, 383, 21} Paint: Rectangle {2, 0, 80, 21} Observe that the paint has bounds with width 80 after the measure method is ran. The measure method sets the width 80, this width then "arrives" at the paint event. In 4.9 the output is instead e.g.: Measure: Rectangle {0, 0, 4, 19} Measure: Rectangle {0, 0, 4, 19} Measure: Rectangle {0, 0, 4, 19} Measure: Rectangle {0, 0, 4, 19} Erase: Rectangle {0, 0, 383, 21} Measure: Rectangle {0, 0, 4, 19} Erase: Rectangle {0, 0, 383, 21} Paint: Rectangle {2, 0, 4, 21} Measure: Rectangle {0, 0, 4, 19} Erase: Rectangle {0, 0, 383, 21} Measure: Rectangle {0, 0, 4, 19} Erase: Rectangle {0, 0, 383, 21} Paint: Rectangle {2, 0, 4, 21} Measure: Rectangle {0, 0, 4, 19} Erase: Rectangle {0, 0, 383, 21} Measure: Rectangle {0, 0, 4, 19} Erase: Rectangle {0, 0, 383, 21} Paint: Rectangle {2, 0, 4, 21} Measure: Rectangle {0, 0, 4, 19} Erase: Rectangle {0, 0, 383, 21} Measure: Rectangle {0, 0, 4, 19} Erase: Rectangle {0, 0, 383, 21} Paint: Rectangle {2, 0, 4, 21} Measure: Rectangle {0, 0, 4, 19} Erase: Rectangle {0, 0, 383, 21} Measure: Rectangle {0, 0, 4, 19} Erase: Rectangle {0, 0, 383, 21} Paint: Rectangle {2, 0, 4, 21} Measure: Rectangle {0, 0, 4, 19} Erase: Rectangle {0, 0, 383, 21} Measure: Rectangle {0, 0, 4, 19} Erase: Rectangle {0, 0, 383, 21} Paint: Rectangle {2, 0, 4, 21} Observe that the paint event bounds width remains equal to 4. This breaks painting in the OwnerDrawLabelProvider that depends on the measurement done in the measure method. E.g. StyledCellLabelProvider.measure(Event, Object) adjusts the width of the event, this would not function in 4.9 and above.