GTK3 does not currently invalidate size caches until the next frame. This is a result of both performance optimizations and the fact that we now use a frame clock so we can ensure we don't do any unnecessary work.

Created attachment 308623 [details] [review]
Patch

This is more or less as expected - the size_request call will return the same value until we've gone through a complete frame cycle and incorporated all the style changes into the geometry calculation.

If you do these kind of changes in an actual UI, you'll see that the label does resize as expected, and if you query the size request at a later time, it does reflect that.

I'll attach a rewritten program that demonstrates this - note that it always prints out the size according to the _previous_ font change.

Created attachment 308634 [details]
test

Review of attachment 308623 [details] [review]:

We are working towards a model where style changes invalidate, and we revalidate things on demand. Adding back these immediate revalidations would be a step in the wrong direction.

Created attachment 308642 [details] [review]
GtkWidget: Fix gtk_widget_size_request not being affected by font change

This is a regression compared to GTK2 which returned proper size values after
changing the font.

The problem is that calling gtk_widget_size_request after changing the font
with gtk_widget_modify_font doesn't work correctly when called more than once.
The preferred size of the label stays the same after the initial computation,
event if the size of the font changed.

This didn't work because of two things:
- The cache for the size request was outdated. This is fixed with a call to
gtk_widget_queue_resize.
- The pango context was outdated, containing the old font description. This is
fixed with a call to gtk_widget_update_pango_context.

Sorry, same patch. I was just testing the new fix for bug 658539. I will do a proper reply a bit later.

Hi Matthias. Thank you for looking at the patch. I am not a GTK expert so sorry if what I am suggesting is not right. Perhaps I can describe a bit more the problem from the point of view of Eclipse (SWT is the name of the graphics library) and we can think of alternate solutions.

SWT has widgets and its own layout management. At any point, clients of SWT can request the preferred sizes of widgets (computeSize). The layouts rely on gtk_widget_get_preferred_size (among other things) to compute the size of individual widgets in order to allocate proper size for composites (i.e. containers) and align things according to the chosen type of layout. That's just one typical use of computeSize/gtk_widget_get_preferred_size. But clients are free to call computeSize (which calls gtk_widget_get_preferred_size) at any time for other purposes.

For example, the original bug I referred to was deducing the optimal font size for a given label width, before the widget is first displayed. As far as I know, this is valid SWT code.

Clients of SWT are also free to ask containers to layout at any time, which can be useful if widgets are added and removed dynamically, for example. With the current problem described in this bug, it is possible for clients to call layout (indirectly gtk_widget_get_preferred_size) more than once and the subsequent layout calls will produce wrong widgets placement. In fact, I have tested this theory with a small program that creates a label in a window, calls layout, changes the font size then "packs" the window to preferred size:

  Display display = new Display();

  Shell shell = new Shell(display, SWT.DIALOG_TRIM);
  shell.setLayout(new FillLayout());

  Label lblName = new Label(shell, SWT.NONE);
  lblName.setText("Test Words");
  // Force the container to layout early, preferred size gets cached here by GTK3
  shell.layout(true);

  Font f = new Font(display, new FontData("Arial", 28, SWT.BOLD));
  lblName.setFont(f);
  f.dispose();

  // Resize the whole window to its preferred size. This will be wrong!
  shell.pack();

  shell.open();

Under GTK2, the window nicely resizes so that "Test Words" fits perfectly. Under GTK3, only "Test" is visible and "Words" is cropped. If I remove the shell.layout call, then it works properly because gtk_widget_get_preferred_size doesn't get cached. As you can see, this can be very error prone. You could said the the solution would be to not call gtk_widget_get_preferred_size more than once, but I don't see that being possible without breaking SWT contracts. I only see two options right now. Either:
1. GTK provides a function to invalidate the cached states so that a subsequent call to gtk_widget_get_preferred_size can be valid again. (Perhaps this already exists and I missed it?)
or
2. GTK takes care clearing the cache when things change (this patch).


I was going for #2 for a few reasons:
- I don't think gtk_widget_override_font gets usually called often so I don't think performance would be much impacted
- Althouth it is documented that "GtkWidget caches a small number of results to avoid re-querying for the same sizes in one allocation cycle", it is not specified how the caches behave and whether or not they will be invalidated when other states change. I think that caches should be transparent to callers unless their behavior is well documented and that they can be controlled in predictable ways (solution #1).
- In particular, both gtk_widget_get_preferred_size and gtk_widget_override_font are public functions than can be called at any time, in any order and it's not documented or obvious that this should not work in certain combinations.

I actually have found a work around that works without modifications to GTK and is questionable at best. I noticed that the pango context is updated when gtk_widget_get_direction is called. So whenever there is a font change is SWT, I change the direction and set it back to the original value. This works now but could stop working at any time since it is not specified that calling gtk_widget_get_direction would update the pango context right away.

Let me know what you think. Perhaps you have another idea how to solve this.

How about just calling gtk_widget_update_pango_context in gtk_widget_override_font and SWT can take care of calling gtk_widget_queue_resize (when necessary)? Would that be more acceptable?

I have to admit that I'd rather find a way for you to trigger both the context update and the queue_resize yourself. We could add this call to override_font now, but it is only a matter of time until this sort of synchronous measuring breaks again in some other place.

One way you could trigger the context update is to call gtk_widget_set_text_direction (twice, change it back and forth)

(In reply to Matthias Clasen from comment #10)
> I have to admit that I'd rather find a way for you to trigger both the
> context update and the queue_resize yourself. We could add this call to
> override_font now, but it is only a matter of time until this sort of
> synchronous measuring breaks again in some other place.

Is it explained in the documentation that "synchronous measuring" is not supposed to be supported? I found this in the GtkWidget:

"The size a widget is finally allocated can of course differ from the size it has requested. For this reason, GtkWidget caches a small number of results to avoid re-querying for the same sizes in one allocation cycle."

It is not clear from that statement which function results will be cached and how it will affect callers of gtk_widget_* functions. From what I understand, it is valid to call most gtk_widget_* functions before the widget is first displayed but it's unclear when things will return outdated values.

I have another suggestion though. Perhaps the documentation for gtk_widget_get_preferred_size could be clearer and mention that values can be cached and that calling gtk_widget_queue_resize will invalidate the caches. In that spirit, update_pango_context could be called by gtk_widget_queue_resize since it does affect the values of the preferred size. 

> One way you could trigger the context update is to call
> gtk_widget_set_text_direction (twice, change it back and forth)

Yes that's what I did in SWT. There is no guarantee thought that this will work in the future as it is not specified that this clears any cached values (context update_pango_context / gtk_widget_queue_resize).

The gtk_widget_set_text_direction work around has been applied in Eclipse. I have a feeling that we'll be hitting this kind of problem again and again but we'll see.