Re: rendering-cleanup-next
Awesome! Some stuff I noticed looking through the branch: * A round of rebase/squash might be nice which would make it easier to review, for example c5c08bafb94e794a88ef5d650999f46b419429ed could squish into 9badb81a7ed62af1cdf11eb31c7e94293a683429 (I was pretty confused by the None there at first) * I'm skeptical of removing ability to set window background to None. This feature can be important to avoid visual artifacts on X11. The API should maybe change to not involve a NULL pixmap; conceptually, what this means is window system should not auto-clear or auto-paint the background on exposes - I don't know if any other platform has the concept. A new API could be gdk_window_set_paint_background(gboolean) rather than set_pixmap(NULL) I think d3802ca Remove calls that try to set GDK_NO_BG on their windows probably results in some things being more ugly. (flashing/flicker) This almost goes away as an issue if dropping all subwindows, but I think setting None on toplevels will still be important sometimes. I guess in future-composited-desktop-world this also fades in importance but I'm not sure on when/whether/details. Anyway - the commit message undefined behavior is not a good idea seems like you're missing the point of this - the idea is not to leave the window bg undefined, it's to wait and let the app repaint it, instead of first having window system repaint then the app repaints (which inherently flickers). * I think it would be nicer than send_expose, if you kept expose_event, but had gtk_widget_real_expose_event() do the GDK_EXPOSE case from gtk_main_do_event() and then also have the code in send_expose(). i.e. the default handler for expose_event should set things up and call draw(). It seems to me kind of bizarre to treat expose events differently from the others. Right now all GdkWindow events are on Widget. I would think what makes sense is to keep them all there, until widget-window becomes optional, and then move them all to some sort of GtkWidgetWithWindow iface where widgets that subscribe to a GdkWindow's events would get these but not widgets in general. The way you have it here, there's no way for people to customize the raw event handling. Say for example I'm doing a GL-based widget, or who knows what other weird thing, maybe my app uses Skia, maybe I want to get the raw expose event. If that makes sense for the app and I don't need draw-to-pdf, I should be able to do it. Maybe I have a legacy app that has a pile of Xlib drawing code. An alternative, which I think is hackier, would be to make gtk_cairo_get_event() public. Anyway the ultimate goal would be for only toplevel GtkWindow plus wonky widgets (GL, embeds) to have the event signals. But whichever widgets keep the event signals, should still have the expose signal, imo. * I still think passing width, height to draw() is weird. If I were just reading this API, I would *strongly* tend to guess that it was the damage rectangle or else the size I'm supposed to paint to, but it isn't. It's a redundant copy of allocation size. Plus, you're clipping to allocation _anyway_ so in most cases I don't even _need_ the allocation size. So the width/height here are just _extra_ typing, not saving me typing allocation.width,height. App developers basically _should_ ignore these. So why are they here? These parameters just confuse and raise questions. If they are defined to be allocation.width,height, they should not exist. App devs are going to think OK, it can't be that and think it has to be damage region, or think they are supposed to support drawing to an arbitrary size, or otherwise try to rationalize the existence of these pointless redundant parameters. But they don't have a rationale ;-) Things with no point are confusing. Gratuitous cognitive load. If I need to know the allocation, then I'll look at the allocation. * There's a fair bit of trailing whitespace in the patch. Maybe turn on trailing-whitespace-highlighting in your editor. * Shouldn't gtk_widget_is_drawable() just die? It seems to me the draw() method can be called when not mapped or visible. In fact it can be useful to do that if you're trying to render offscreen or to pdf. (Maybe we want to pedantically require visible, but I don't think we should have to be mapped, which implies in a toplevel window system window.) gtk_widget_draw() is documented as requiring the widget to be drawable, but I don't see why draw() in its current form needs widgets to be mapped. Havoc ___ gtk-devel-list mailing list gtk-devel-list@gnome.org http://mail.gnome.org/mailman/listinfo/gtk-devel-list
Re: rendering-cleanup-next
On Sat, 2010-09-11 at 07:03 +0200, Benjamin Otte wrote: Hey, snip PS: Attached is a rendering of the liststore example from gtk-demo to a pdf surface using Raleigh, Clearlooks and EasyListening. Just because I want to show off. It's missing the symbolic icons... ___ gtk-devel-list mailing list gtk-devel-list@gnome.org http://mail.gnome.org/mailman/listinfo/gtk-devel-list
Re: rendering-cleanup-next
Hi, On Sat, Sep 11, 2010 at 11:16 AM, Havoc Pennington h...@pobox.com wrote: I still think passing width, height to draw() is weird. btw, I guess the argument here (per IRC) is that people might be confused by allocation.x,y. But this is a really weak band-aid fix for that, which will be wrong in the long term. The solution to allocation x,y is to set the larger goal: widgets don't know their transform (currently GTK only supports translation transforms, of course). Widgets should always work in a self-relative coordinate system, on the rare weird occasion where they don't, they can use gtk_widget_translate_coordinates() which eventually could even support rotation and scaling, or there could be a flavor that gives you a matrix. The actual bug is that widget_size_allocate() receives an x,y and gtk_widget_get_allocation() gets an x,y. That's where the bug should be fixed, not by adding some confusing stuff to draw(). It would take some thought to fix the leakage of x,y on the allocation side, but I think that thought is what's needed, rather than band-aid pseudo-solutions in draw(). Roughly, the solution could be that: * size_allocate vfunc and wrapper change to (* size_allocate) (GtkWidget, int w, int h) * add gtk_widget_set_translation(widget,x,y)/get_translation() * containers now have to call gtk_widget_allocate_and_translate(widget, rectangle) convenience function instead of size_allocate(). This convenience function obviously calls size_allocate(rect.w, rect.h) and set_translation(rect.x, rect.y) This is a pretty breaking change, but porting apps is no harder than changing expose-event to draw. If you want to then be really sick and twisted, once you clear out GdkWindow you can support a full cairo_matrix_t instead of just translation... which would be logical and straightforward now that you've kicked the translation out of the allocation. Another possible solution is to keep the width,height to draw(), but _get rid of size allocate_; widgets would then be supposed to support any width,height at any time being passed to draw(), and they'd in practice always have to cache their last layout for performance, most likely. (they'd sort of make the first part of draw() do what they do now in size_allocate(), iff the allocation has changed, and cache the result). I don't think this is a very nice solution because it makes draw() methods multi-purpose and clunky. I think it's cleaner to have separate vfuncs for doing the layout and handling a paint on the layout, as GTK already has. Havoc ___ gtk-devel-list mailing list gtk-devel-list@gnome.org http://mail.gnome.org/mailman/listinfo/gtk-devel-list
Re: rendering-cleanup-next
Hi, On Sat, Sep 11, 2010 at 12:15 PM, Benjamin Otte o...@gnome.org wrote: What's you opinion on having gtk_widget_get_width() and gtk_widget_get_height() functions? They would just return widget-priv-allocation.width/height for now. Such functions would address your issues and would make writing draw functions pretty much as simple as they are now, without having to pass width and height. And in particular, they'd get rid of the need to think about allocations. Or would that be a too prominent API? Yep, I was thinking/assuming there would be exactly this. The only thing I'd add, I think they ought to be named something with allocated in there, for a couple reasons: - in many languages GtkSizeRequest::get_width() is already just callable as widget.get_width() - plain get_width() more naturally gets request, anyhow Clutter has a hack where get_width() returns the allocation if valid else the request, but this was a back compat hack / an attempt to be nice to naive users who don't want to think about request vs. allocation. The obvious API might be: get_allocated_width(), get_allocated_height(), get_allocated_size(); or even change get_allocation() to return width,height and add separate get_translation(). Something I just noticed, it looks like widgets are doing get_allocation and looking at allocation.x,y still because gtk_paint_* seems to assume that the cairo_t is not translated? I guess the natural new way for this to work would be for gtk_paint to pass the theme engine a cairo_t already translated and clipped to the box the theme element is supposed to be inside. Maybe the new theme branch has something more radical though. Havoc ___ gtk-devel-list mailing list gtk-devel-list@gnome.org http://mail.gnome.org/mailman/listinfo/gtk-devel-list
Re: rendering-cleanup-next
On Sat, Sep 11, 2010 at 6:29 PM, Havoc Pennington h...@pobox.com wrote: - in many languages GtkSizeRequest::get_width() is already just callable as widget.get_width() - plain get_width() more naturally gets request, anyhow Ugh, I'd always assume that widget.get_width() would give me the width of the widget, not the width the widget thought would be ideal but had nothing to do with reality. Also, a width getter would never return 2 values for me. Should we maybe rename it to gtk_size_request_get_natural/minimum_width()? Tristan? Something I just noticed, it looks like widgets are doing get_allocation and looking at allocation.x,y still because gtk_paint_* seems to assume that the cairo_t is not translated? gtk_paint_*() does - at least in my branch - draw relative to the passed in cairo_t. As almost all the paint functions take x,y,width,height anyway it doesn't really matter where the origin of the cairo_t is. You'll notice in all the Port to draw vfunc patches that I removed allocation.x/y from the x/y parameters. The original paint functions that took a GdkWindow were window-relative so had to take into account allocation.x/y. Benjamin ___ gtk-devel-list mailing list gtk-devel-list@gnome.org http://mail.gnome.org/mailman/listinfo/gtk-devel-list
Re: rendering-cleanup-next
Hi, On Sat, Sep 11, 2010 at 12:57 PM, Benjamin Otte o...@gnome.org wrote: gtk_paint_*() does - at least in my branch - draw relative to the passed in cairo_t. As almost all the paint functions take x,y,width,height anyway it doesn't really matter where the origin of the cairo_t is. You'll notice in all the Port to draw vfunc patches that I removed allocation.x/y from the x/y parameters. The original paint functions that took a GdkWindow were window-relative so had to take into account allocation.x/y. I'm looking at gtk_scale_draw for example on your branch http://git.gnome.org/browse/gtk+/tree/gtk/gtkscale.c?h=rendering-cleanup-next#n1159 Havoc ___ gtk-devel-list mailing list gtk-devel-list@gnome.org http://mail.gnome.org/mailman/listinfo/gtk-devel-list
Re: rendering-cleanup-next
On Sat, Sep 11, 2010 at 7:05 PM, Havoc Pennington h...@pobox.com wrote: I'm looking at gtk_scale_draw for example on your branch http://git.gnome.org/browse/gtk+/tree/gtk/gtkscale.c?h=rendering-cleanup-next#n1159 gtk_scale_get_layout_offsets() returns values relative to widget-window, so takes into account the allocation. This is actually a rather ugly situation right now: Everything in the widget but the draw function (key press events etc) uses coordinates relative to the GdkWindow, only the draw function doesn't. So when calling internal get_offsets() style APIs, the draw function always has to subtract the allocation again. I cannot really change the behavior though, because when calling gtk_widget_draw() the widget needs to paint the contents of all GDK windows it owns, so the drawing needs to be relative to oe point, not multiple different ones (the origins of all the windows). And I think it's best to rely on allocation.x/y, even if all the other events don't. Benjamin ___ gtk-devel-list mailing list gtk-devel-list@gnome.org http://mail.gnome.org/mailman/listinfo/gtk-devel-list
Re: rendering-cleanup-next
Hi, On Sat, Sep 11, 2010 at 1:11 PM, Benjamin Otte o...@gnome.org wrote: This is actually a rather ugly situation right now: Everything in the widget but the draw function (key press events etc) uses coordinates relative to the GdkWindow, only the draw function doesn't. So when calling internal get_offsets() style APIs, the draw function always has to subtract the allocation again. Oh I see, got it. Seems like the same basic issue as size_allocate taking the x,y. The big picture goal would be to make widgets always use self-relative coordinates. If there were a facility to get events without having a GdkWindow, it ought to be designed to go ahead and translate the stuff in the events relative to the widget. I had a proposal for that here btw: http://mail.gnome.org/archives/gtk-devel-list/2010-August/msg00223.html Anyway, certainly ought to land the new rendering branch first. It's so tantalizingly within reach to have the clipping, scrolling, event-getting without windows, plus all coords are widget-relative though. Hoping it makes 3.0! Havoc ___ gtk-devel-list mailing list gtk-devel-list@gnome.org http://mail.gnome.org/mailman/listinfo/gtk-devel-list
Re: rendering-cleanup-next
Whew, lots of things to reply to. Here we go... On Sat, Sep 11, 2010 at 5:16 PM, Havoc Pennington h...@pobox.com wrote: A round of rebase/squash might be nice which would make it easier to review, for example c5c08bafb94e794a88ef5d650999f46b419429ed could squish into 9badb81a7ed62af1cdf11eb31c7e94293a683429 (I was pretty confused by the None there at first) Ooops. I had assumed the first commit had landed in master already. I'll squash that in the next rebase. I'm skeptical of removing ability to set window background to None. This feature can be important to avoid visual artifacts on X11. The API should maybe change to not involve a NULL pixmap; conceptually, what this means is window system should not auto-clear or auto-paint the background on exposes - I don't know if any other platform has the concept. A new API could be gdk_window_set_paint_background(gboolean) rather than set_pixmap(NULL) The problem here is that I want to get rid of proxying the ugly X11 background API via GDK. Neither Windows nor Quartz have anything similar. So my approach was to use a cairo_pattern_t in the API and then say The backends are responsible for mapping it to the system backgrounds as well as possible. as we overpaint it during exposes anyway. So hat we do in the gdk_x11_window_set_background() call is open for debate. I think it would be nicer than send_expose, if you kept expose_event, but had gtk_widget_real_expose_event() do the GDK_EXPOSE case from gtk_main_do_event() and then also have the code in send_expose(). i.e. the default handler for expose_event should set things up and call draw(). It seems to me kind of bizarre to treat expose events differently from the others. Right now all GdkWindow events are on Widget. I would think what makes sense is to keep them all there, until widget-window becomes optional, and then move them all to some sort of GtkWidgetWithWindow iface where widgets that subscribe to a GdkWindow's events would get these but not widgets in general. First of all, expose events are already different from all other events today in GTK2. One of them is sent via gtk_widget_send_expose(), the other via gtk_widget_send_event(). Then, I don't think that it's wise to treat expose handling the same way as input handling anyway, because it's just different. Also, I do think that having two events (draw and expose-event) on GtkWidget is confusing, in particular because one of them is only emitted sometimes (gtk_container_propagate_draw() would not cause expose events). What I want to have is a GdkWindow::event signal instead of the magic gdk_window_set_user_data() function. Then your code could just connect to that signal and do whatever it wants with expose events. There's a fair bit of trailing whitespace in the patch. Maybe turn on trailing-whitespace-highlighting in your editor. I'm of the if the tools don't fix it, it isn't broken school of thought. So I don't spend much time taking care of formatting things properly. Also, I don't think GTK has a very strict policy on code formatting these days other than make sure it looks like all the other code. Shouldn't gtk_widget_is_drawable() just die? It seems to me the draw() method can be called when not mapped or visible. In fact it can be useful to do that if you're trying to render offscreen or to pdf. (Maybe we want to pedantically require visible, but I don't think we should have to be mapped, which implies in a toplevel window system window.) gtk_widget_draw() is documented as requiring the widget to be drawable, but I don't see why draw() in its current form needs widgets to be mapped. This was a somewhat harder decision for me to take. I kept it as-is for now for one simple reason: It's easier to relax this requirement later than it is to make it stricter. That said, from my experiences a widget must be drawable for it to render properly. As long as a widget is not visible, it will not be allocated a proper size and render wrong. If it isn't realized, it hasn't created its windows and will therefor mess up its rendering. So realized and visible are very required already. Dropping the mapped requirement would be possible, but the current code only maps and realizes child widgets when mapping. A realized and visible treeview for example does not render headers. This could be changed, but would require redefining how children get realized/mapped with their parents. So the only sane way to get a proper rendering from a widget currently seems to be to require that it is drawable. Benjamin ___ gtk-devel-list mailing list gtk-devel-list@gnome.org http://mail.gnome.org/mailman/listinfo/gtk-devel-list
Re: rendering-cleanup-next
Hi, On Sat, Sep 11, 2010 at 11:46 AM, Havoc Pennington h...@pobox.com wrote: * size_allocate vfunc and wrapper change to (* size_allocate) (GtkWidget, int w, int h) Another possible addition here, which is in both Clutter and HippoCanvas, would be an ORIGIN_CHANGED flag. (Clutter uses a flags arg, Hippo uses just a boolean.) One purpose of this flag is that window widgets need to know when the absolute origin changes. This allows them to keep the window positioned properly. You would need this to remove knowledge of windows from the GtkWidget/GtkContainer/GtkSizeRequest/GtkSizeGroup core and confine such knowledge to certain GtkWidgetWithWindow classes that had to keep the GdkWindow positioned themselves. (While right now the GTK core takes care of that.) A related cleanup would be to make the widget translation always be with respect to the parent container, rather than with respect to the nearest GdkWindow. At that point you could traverse widgets for most purposes without caring whether they had a window. Havoc ___ gtk-devel-list mailing list gtk-devel-list@gnome.org http://mail.gnome.org/mailman/listinfo/gtk-devel-list
Re: questions re: aux info, size request
On Sat, Sep 11, 2010 at 7:58 PM, Havoc Pennington h...@pobox.com wrote: Anyhow: it would be a shame to ship the ignore set size request smaller than min size by accident. Should I open a bug? Or should I change (and document) set_size_request to increase only? Sounds good to me. ___ gtk-devel-list mailing list gtk-devel-list@gnome.org http://mail.gnome.org/mailman/listinfo/gtk-devel-list
Re: padding cleanup
Hi, On Sat, Sep 11, 2010 at 11:41 PM, Matthias Clasen matthias.cla...@gmail.com wrote: Turns out this is unrelated to your branch. I've bisected this to Oops, I just burnt a hole in my lap bisecting this too. At least _one_ of those checkouts could have managed not to rebuild the whole tree... I got to this commit though, one after yours, as the guilty one (for me ccacd3a was OK) commit 709e05cdb20fd6d2360b7158286d6c3176c300c0 Author: Benjamin Otte o...@redhat.com Date: Sat Aug 7 02:18:06 2010 +0200 style: Use _gtk_pango_fill_layout() Havoc ___ gtk-devel-list mailing list gtk-devel-list@gnome.org http://mail.gnome.org/mailman/listinfo/gtk-devel-list
Re: questions re: aux info, size request
On Sat, 2010-09-11 at 21:05 -0400, Matthias Clasen wrote: On Sat, Sep 11, 2010 at 7:58 PM, Havoc Pennington h...@pobox.com wrote: Anyhow: it would be a shame to ship the ignore set size request smaller than min size by accident. Should I open a bug? Or should I change (and document) set_size_request to increase only? Sounds good to me. While on this topic, there's this XXX comment I left dangling in gtksizegroup.c: http://git.gnome.org/browse/gtk+/tree/gtk/gtksizegroup.c#n677 I was thinking maybe that for all the widgets in a group to be effectively the same size, maybe we should be basing the minimum base requests from the natural requests of all widgets in a group. Currently a size group bumps up the minimum required size to match the minimum size of all widgets in the group, the problem with this is maybe some widgets will be allocated a bigger size (while requiring the same minimum size); if they were to require the natural size of the group as a minimum, they would more effectively be allocated the same size (the problem with that approach is that grouped widgets might be allocated a dramatically large size unintentionally). But if set_size_request() were to limit the natural size; combining it with sizegroups that demand/require the natural size as a minimum might work nicely. Thoughts ? Cheers, -Tristan ___ gtk-devel-list mailing list gtk-devel-list@gnome.org http://mail.gnome.org/mailman/listinfo/gtk-devel-list
Re: questions re: aux info, size request
Hi,
On Sun, Sep 12, 2010 at 12:45 AM, Tristan Van Berkom
trista...@openismus.com wrote:
While on this topic, there's this XXX comment I left dangling
in gtksizegroup.c:
http://git.gnome.org/browse/gtk+/tree/gtk/gtksizegroup.c#n677
I was thinking maybe that for all the widgets in a group to be
effectively the same size, maybe we should be basing the minimum
base requests from the natural requests of all widgets in a group.
I would expect that GtkSizeGroup caused all widgets in the group to
request a min size of the largest min size in the group, and a natural
size of the largest natural size in the group. Then it Just Works,
right? Is there a reason that only one of min or natural can be picked
for size group?
But if set_size_request() were to limit the natural size; combining
it with sizegroups that demand/require the natural size as a minimum
might work nicely.
To be clear, I was proposing a set_natural_size() or something (i.e. I
think making set_size_request set natural would be too incompatible)
It might be clearer to rename set_size_request to set_minimum_size()
but that function is maybe too heavily used to rename... at least
without keeping the old name also.
btw along these lines, I can't remember if I filed the attached patch.
Natural size much less useful without it.
Havoc
From 7ddeb49f1643799794bdc7d96a55fe9a885cd39f Mon Sep 17 00:00:00 2001
From: Havoc Pennington h...@pobox.com
Date: Mon, 6 Sep 2010 11:18:35 -0400
Subject: [PATCH] Make GtkWindow default to natural size not minimum size
Otherwise your ellipsized labels all start out ellipsized, unless
you manually gtk_window_set_default_size().
This probably makes it important to clamp the window's default
size to the size of the monitor's work area.
---
gtk/gtkwindow.c |4 ++--
tests/testgtk.c |1 +
2 files changed, 3 insertions(+), 2 deletions(-)
diff --git a/gtk/gtkwindow.c b/gtk/gtkwindow.c
index 5f75dae..b50a9ab 100644
--- a/gtk/gtkwindow.c
+++ b/gtk/gtkwindow.c
@@ -5725,9 +5725,9 @@ gtk_window_compute_configure_request_size (GtkWindow *window,
if (window-need_default_size)
{
- gtk_widget_get_child_requisition (widget, requisition);
+ gtk_size_request_get_size (GTK_SIZE_REQUEST (widget), NULL, requisition);
- /* Default to requisition */
+ /* Default to natural requisition */
*width = requisition.width;
*height = requisition.height;
diff --git a/tests/testgtk.c b/tests/testgtk.c
index cddb7df..8ea1117 100644
--- a/tests/testgtk.c
+++ b/tests/testgtk.c
@@ -8431,6 +8431,7 @@ create_window_sizing (GtkWidget *widget)
gtk_widget_get_screen (widget));
label = gtk_label_new (NULL);
gtk_label_set_markup (GTK_LABEL (label), span foreground=\purple\bigWindow being resized/big/span\nBlah blah blah blah\nblah blah blah\nblah blah blah blah blah);
+ gtk_label_set_ellipsize (GTK_LABEL (label), PANGO_ELLIPSIZE_END);
gtk_container_add (GTK_CONTAINER (target_window), label);
gtk_widget_show (label);
--
1.7.0.4
___
gtk-devel-list mailing list
gtk-devel-list@gnome.org
http://mail.gnome.org/mailman/listinfo/gtk-devel-list
Re: questions re: aux info, size request
On Sun, 2010-09-12 at 01:01 -0400, Havoc Pennington wrote: Hi, On Sun, Sep 12, 2010 at 12:45 AM, Tristan Van Berkom trista...@openismus.com wrote: While on this topic, there's this XXX comment I left dangling in gtksizegroup.c: http://git.gnome.org/browse/gtk+/tree/gtk/gtksizegroup.c#n677 I was thinking maybe that for all the widgets in a group to be effectively the same size, maybe we should be basing the minimum base requests from the natural requests of all widgets in a group. I would expect that GtkSizeGroup caused all widgets in the group to request a min size of the largest min size in the group, and a natural size of the largest natural size in the group. Then it Just Works, right? Is there a reason that only one of min or natural can be picked for size group? Hmmm your right this behaviour is probably a bit buggy (at the time my target was more to make widgets actually all receive the same size, it seemed less important to synchronize every natural size in the group). Ofcourse the way sizegroups are mishmashed into the request api it will be a bit tricky to fix but I'll consider the current implementation buggy and go ahead and perfect that. But if set_size_request() were to limit the natural size; combining it with sizegroups that demand/require the natural size as a minimum might work nicely. To be clear, I was proposing a set_natural_size() or something (i.e. I think making set_size_request set natural would be too incompatible) It might be clearer to rename set_size_request to set_minimum_size() but that function is maybe too heavily used to rename... at least without keeping the old name also. Ok I see, so we end up with: - set_size_request() Can be used to increase the minimum request - set_natural_size() Could be added to explicitly set the natural size to anything greater than the minimum size (greater or less than the widget's original natural request) Sounds good :) Cheers, -Tristan ___ gtk-devel-list mailing list gtk-devel-list@gnome.org http://mail.gnome.org/mailman/listinfo/gtk-devel-list
