Bug 394583

Comment 1 Alexander Kurtakov 2012-11-20 12:10:31 EST

Works perfect. Pushed to master http://git.eclipse.org/c/platform/eclipse.platform.swt.git/commit/?id=89b5f4500222e33648e01e47d68897bffeb1aa77

Comment 2 Silenio Quarti 2012-11-20 12:35:09 EST

This patch is not correct. It changes the size of the widget in computeSize(). Run this snippet and you will see that the text widget is not 200x200 after the call to computeSize().

import org.eclipse.swt.SWT;
import org.eclipse.swt.widgets.*;


public class TestSize {
public static void main(String[] args) {
	Display display = new Display();
	Shell shell = new Shell(display);

	Text text = new Text(shell, SWT.SINGLE);
	text.setBounds(10, 10, 200, 200);
	text.computeSize(SWT.DEFAULT, SWT.DEFAULT);

	shell.open();
	while (!shell.isDisposed()) {
		if (!display.readAndDispatch())
			display.sleep();
	}
	display.dispose();
}
}

Comment 3 Anatoly Spektor 2012-11-20 13:14:17 EST

(In reply to comment #2)
> This patch is not correct. It changes the size of the widget in
> computeSize(). Run this snippet and you will see that the text widget is not
> 200x200 after the call to computeSize().
> 
> import org.eclipse.swt.SWT;
> import org.eclipse.swt.widgets.*;
> 
> 
> public class TestSize {
> public static void main(String[] args) {
> 	Display display = new Display();
> 	Shell shell = new Shell(display);
> 
> 	Text text = new Text(shell, SWT.SINGLE);
> 	text.setBounds(10, 10, 200, 200);
> 	text.computeSize(SWT.DEFAULT, SWT.DEFAULT);
> 
> 	shell.open();
> 	while (!shell.isDisposed()) {
> 		if (!display.readAndDispatch())
> 			display.sleep();
> 	}
> 	display.dispose();
> }
> }

Yes you are absolutely right, because of this line handle changes its size:

 OS.gtk_widget_set_size_request(handle, wHint, hHint);

 If I remove this line, everything seems to work fine, I will double check and improve  patch.

Thanks for noticing!

Comment 4 Anatoly Spektor 2012-11-20 13:20:46 EST

(In reply to comment #3)
> (In reply to comment #2)
> > This patch is not correct. It changes the size of the widget in
> > computeSize(). Run this snippet and you will see that the text widget is not
> > 200x200 after the call to computeSize().
> > 
> > import org.eclipse.swt.SWT;
> > import org.eclipse.swt.widgets.*;
> > 
> > 
> > public class TestSize {
> > public static void main(String[] args) {
> > 	Display display = new Display();
> > 	Shell shell = new Shell(display);
> > 
> > 	Text text = new Text(shell, SWT.SINGLE);
> > 	text.setBounds(10, 10, 200, 200);
> > 	text.computeSize(SWT.DEFAULT, SWT.DEFAULT);
> > 
> > 	shell.open();
> > 	while (!shell.isDisposed()) {
> > 		if (!display.readAndDispatch())
> > 			display.sleep();
> > 	}
> > 	display.dispose();
> > }
> > }
> 
> Yes you are absolutely right, because of this line handle changes its size:
> 
>  OS.gtk_widget_set_size_request(handle, wHint, hHint);
> 
>  If I remove this line, everything seems to work fine, I will double check
> and improve  patch.
> 
> Thanks for noticing!

Here is fix for my previous patch:

http://fedorapeople.org/cgit/aspektor/public_git/eclipse.platform.swt.git/commit/?h=coolbar_clipping&id=e82bbbef9a0e5f3f161f575387b0e9bc08764cf7

Please let me know, if you find any problems with it.

Thanks!

Comment 5 Alexander Kurtakov 2012-11-20 14:15:27 EST

Sorry for that, I'll resist saying whether the new fix is right or wrong.

Comment 6 Silenio Quarti 2012-11-21 14:04:13 EST

Thanks Anatoly. I have committed this change because it is a improvement, but I believe there is still something wrong here. Run the controlexample in both GTK2 and GTK3. Note that on GTK3, the CoolBar widget is much wider than it is on GTK2.  That happens because the size of the Text widget on GTK2 is smaller.

Comment 7 Alexander Kurtakov 2012-11-21 15:15:30 EST

Gtk 3 itself would not allow setting size smaller than some minimum size (as coded in gtk itself) so any request for setting smaller size would simply be ignored. Is it possible that we hit similar case?

Comment 8 Silenio Quarti 2012-11-22 10:45:45 EST

(In reply to comment #7)
> Gtk 3 itself would not allow setting size smaller than some minimum size (as
> coded in gtk itself) so any request for setting smaller size would simply be
> ignored. Is it possible that we hit similar case?

I am wondering if this is caused the "expand and fill flags" changes:

http://developer.gnome.org/gtk3/3.5/ch24s02.html#idp129727376

Comment 9 Anatoly Spektor 2012-11-22 14:49:16 EST

(In reply to comment #8)
> (In reply to comment #7)
> > Gtk 3 itself would not allow setting size smaller than some minimum size (as
> > coded in gtk itself) so any request for setting smaller size would simply be
> > ignored. Is it possible that we hit similar case?
> 
> I am wondering if this is caused the "expand and fill flags" changes:
> 
> http://developer.gnome.org/gtk3/3.5/ch24s02.html#idp129727376

I think that the reason in different wide is because of the difference in how wide is calculated in both GTK+ 2 and GTK+ 3.

In Text.computeSize, when wHint and hHint are default (-1) my patch calculates with using OS.gtk_widget_get_preferred_size(); , whereas in GTK2 width is "hardcoded" here :

		if (width == 0) width = DEFAULT_WIDTH;
		if (height == 0) height = DEFAULT_HEIGHT;

  DEFAULT_WIDTH is set to 64, so 64 is passed to computeTrim, whereas gtk_widget_get_preferred_size set "preferred" size to 160. Height is the same coz in our case height is never equals 0.

What can be done ? 

One of the options would be not calculating GtkRequisition but leaving DEFAULT_WIDTH hard-coded, but in this case there is problem with computeTrim as it clips the end of the Text and I am not sure how to solve this issue.

Other option would be let gtk_widget_get_preferred_size to do its business, as  when user does not provide a size, he/she most probably relies on system to do it for them, and I would say that gtk_widget_get_preferred_size does this job pretty well, despite difference in the width.


I have also tried setting hexpand and vexpand when creating Text handle, and also when creating Composite handle, it did not gave me any results.

Silenio please let me know, if you think there is something else we could try regarding this issue.

Comment 10 Grant Gayed 2013-01-30 15:32:03 EST

When this is fixed please check whether Bug 399551 becomes fixed as well (possible duplicate).

Comment 11 Silenio Quarti 2013-03-20 12:38:51 EDT

I removed the previous changes. This bug got fixed with the changes for bug#400339 .