After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 437493 - GtkEntry -- bg draw: use geomtry, don't re-calculate [PATCH]
GtkEntry -- bg draw: use geomtry, don't re-calculate [PATCH]
Status: RESOLVED FIXED
Product: gtk+
Classification: Platform
Component: Widget: Other
2.10.x
Other All
: Normal minor
: ---
Assigned To: gtk-bugs
gtk-bugs
Depends on:
Blocks:
 
 
Reported: 2007-05-10 16:16 UTC by Ricardo Cruz
Modified: 2008-04-25 21:58 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Patch (483 bytes, patch)
2007-05-10 16:17 UTC, Ricardo Cruz
committed Details | Review
background frame truncated without the patch (50.27 KB, image/png)
2008-04-24 23:27 UTC, Ricardo Cruz
  Details

Description Ricardo Cruz 2007-05-10 16:16:28 UTC
Please describe the problem:
GtkEntry allows styles to draw a custom background on the text area (which is used by SphereCrystal, for instance) by calling gtk_paint_flat_box() and passing "entry_bg", on gtk_entry_expose().

Now, the issue is that instead of getting the text_area GdkWindow geometry, it calculates the width and height it has. Besides, being cpu wasted, you run into trouble when you inherit a GtkEntry and add extra windows next to the text_area, but inside the window used for the frame. I do this for a search entry similar to that of Evolution or Banshee.

Steps to reproduce:


Actual results:


Expected results:


Does this happen every time?


Other information:
Comment 1 Ricardo Cruz 2007-05-10 16:17:07 UTC
Created attachment 87964 [details] [review]
Patch
Comment 2 Björn Lindqvist 2008-04-22 22:45:45 UTC
Tested the patch and it seem to work fine. Although there are other places in gtkentry.c that uses get_text_area_size (), those places could too use gdk_drawable_get_size (entry->text_area, &width, &height).
Comment 3 Ricardo Cruz 2008-04-23 01:37:35 UTC
You need of course to calculate that inner rectangle area for realize and further allocates calls, so that you set the GdkWindow area in the first place.
I guess it's not needed for the layout stuff as calculated for the public API method since it seems to be meant to be used for events, so the widget would be realized and allocates done, but I'm not comfortable to comment on that -- either way, the programmer can account for it.

In the case to the expose event, it's an innocuous change because the GdkWindow just got an expose event, so it must be created and properly initialized. In gtktreeview.c, they also use the gdk_drawable_get_size() method from the expose event handler for the sub-windows. I would be surprised if this wasn't common...

This would allow us to tweak the GtkEntry widget without getting the background pixmap truncated, for those few styles that use it. However, it seems that the next GTK version will have API methods for GtkEntry to add inner icons, so tweaking the GtkEntry would no longer be necessary for search entries... So, there is even less demand for it now, but I'd patch this into GTK... (it saves a little bit of cpu afterall :)).
Comment 4 Björn Lindqvist 2008-04-23 10:44:44 UTC
I meant the calls to get_text_area_size () in gtk_entry_size_allocate (), update_im_cursor_location () and get_layout_position (). Presumably some, or all, of those calls could also be replaced with gdk_drawable_get_size ().

Which themes have you tested this patch with btw other than SphereCrystal? Maybe you can provide before and after screenshots showing how this patch fixes the GtkEntry problem?
Comment 5 Ricardo Cruz 2008-04-24 23:23:40 UTC
The purpose of size_allocate() is to set the position and size of the sub-windows, so we have to calculate the text area there. With regard to the rest, you likely don't want to do it. Even if it works right now, I'm not sure if those methods guarantee the text sub-window is allocated, so you would just make the code brittle for no serious gain.

By the way, the way I implement the search entry is by over-loading size_allocate, so that I shape the text sub-window in order make room for two icons on the sides. So the expose handler (paint event) should get the width from the sub-window, not calculate it again, otherwise it gets truncated as it tries to paint using the original width value.
I will append a screenshot of how it looks like. I can't compile and install GTK right now in order to produce a screenshot with this patched, but it's pretty obvious how it will look like. Since the width would be based on the changed value, the frame around the text background would be painted at the right side as well.

Many folks don't have this specific problem because they implemented search entries in quite some hacky ways -- sometimes, they have a lot more issues with pixmap styles like double frames as at least used to be the case in Evolution.
Comment 6 Ricardo Cruz 2008-04-24 23:27:18 UTC
Created attachment 109869 [details]
background frame truncated without the patch
Comment 7 Björn Lindqvist 2008-04-25 21:58:57 UTC
Thanks! I've committed your patch in r20045:

2008-04-25  Björn Lindqvist  <bjourne@gmail.com>

        * gtk/gtkentry.c: (gtk_entry_expose) Use existing window size when
        painting the flat box instead of recalculating it. (#437493,
        Ricardo Cruz)