Re: rendering-cleanup-next
El 16/09/10 13:00, Florian Müllner escribió: El jue, 16-09-2010 a las 20:36 +1000, Andrew Cowie escribió: If someone write up a list of indent options and stick the command in a one line script somewhere prominent (like / :)) then we can just run the code formatter once over the entire tree, and then just tell people to run it in future. Of course that will severely limit the usefulness of git-blame ... The real fix for this is implement a git feature that allows you to commit in a style fashion so it doesn't affect git blame. It would be very tricky to do but basically the algorithm would assign to the last commit of each line a modification to make the commit be in the last form of the style-commit. After this is implemented and gnome git adopts this version of git, a style-commit would be done for the whole gtk+ project and then we're good. Another good thing to do after that is deploying the codeformatter in the server as a hook for any commit, so users don't need to run it. My 2 cents. Andres -- ___ 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 Thu, Sep 16, 2010 at 6:36 AM, Andrew Cowie and...@operationaldynamics.com wrote: On Sun, 2010-09-12 at 11:23 -0400, Matthias Clasen wrote: Anyhow, sure, if GTK has no policy that's fine. I assumed it had a sensible policy... We don't have a written-down policy, beyond 'fit in locally'. But I have become increasingly annoyed by trailing whitespace ... Can we maybe just use indent? I think mangling all git blame and history is too radical a move, unless the diff from indent was fairly small. I've needed the blame history several times just in doing a few gtk patches over the last couple weeks (I had to figure out why scrolledwindow needed to look at aux_info for example which went back to 1999 or 2000). No more _new_ broken whitespace is a good goal to start... Havoc ___ gtk-devel-list mailing list gtk-devel-list@gnome.org http://mail.gnome.org/mailman/listinfo/gtk-devel-list
Re: rendering-cleanup-next
El 16/09/10 15:24, Olav Vitters escribió: On Thu, Sep 16, 2010 at 01:07:15PM +0200, Andrés G. Aragoneses wrote: Another good thing to do after that is deploying the codeformatter in the server as a hook for any commit, so users don't need to run it. This is impossible with distributed version control. Hooks can only contain policy, should not change the commit (server would be out of sync with local copy, etc). Good point, then there you go another feature request for git: download commit-hooks on git-pull :) Andrés -- ___ gtk-devel-list mailing list gtk-devel-list@gnome.org http://mail.gnome.org/mailman/listinfo/gtk-devel-list
Re: rendering-cleanup-next
I just pushed an update to rendering-cleanup and rendering-cleanup-next that incorporates the suggestions from this thread. In particular: - I squelched commits The fixes from Kris for OS X should now be in their correct position and allow compiling random checkouts so git bisect works on OS X (hopefully). Also, I squelched the background handling fixes Havoc mentioned in his mail - GtkWidget::draw now doesn't pass width/height You just get the widget and a cairo_t now. To get the values previously passed as width and height, I introduced gtk_widget_get_allocated_width/height(). They return widget-priv-allocation.width/height, but are way nicer to use. - The draw signal handlers are properly guarded by cairo_save/restore() This was only discussed on IRC, but I thought it's worth mentioning here. I consider this API prtty much done at this point. There might be some implementation details that we might want to change later (like background handling), but we can do that after rendering-leanup-next hit master. So I consider it ready to merge. Comments? Benjamin ___ gtk-devel-list mailing list gtk-devel-list@gnome.org http://mail.gnome.org/mailman/listinfo/gtk-devel-list
Re: rendering-cleanup-next
On Tue, Sep 14, 2010 at 11:46 AM, Benjamin Otte o...@gnome.org wrote: I consider this API prtty much done at this point. There might be some implementation details that we might want to change later (like background handling), but we can do that after rendering-leanup-next hit master. So I consider it ready to merge. Comments? What about the expose_event / gtk_widget_send_expose_event stuff ? Do we want to merge what you have first and figure that out afterwards ? ___ gtk-devel-list mailing list gtk-devel-list@gnome.org http://mail.gnome.org/mailman/listinfo/gtk-devel-list
Re: rendering-cleanup-next
And did you want to add some of the checks and warnings in gtk_widget_draw_internal that were discussed (eg about not having up-to-date allocation) ? ___ gtk-devel-list mailing list gtk-devel-list@gnome.org http://mail.gnome.org/mailman/listinfo/gtk-devel-list
Re: rendering-cleanup-next
On Tue, Sep 14, 2010 at 7:46 PM, Matthias Clasen matthias.cla...@gmail.com wrote: What about the expose_event / gtk_widget_send_expose_event stuff ? Do we want to merge what you have first and figure that out afterwards ? I want to figure that out afterwards. It's something I haven't figured out completely yet. I consider it a part of rendering-cleanup part 4: The hackfest menace. And did you want to add some of the checks and warnings in gtk_widget_draw_internal that were discussed (eg about not having up-to-date allocation) ? I'm actually not sure about that. First, we don't have any code that defines if an allocation is valid or even defines what a valid allocation is. Or do we? gtk_widget_get_allocation() at least doesn't do anything there. But I guess we can add more warnings to gtk_widget_draw() while we start using it. (Or better: Fix those problems). Both of these problems at least belong to the implementation details that we might want to change later stage. I suspect we'll find a few useful knobs to tune and APIs to add when we port all those applications to it. So I consider it by no means finished yet. But it's certainly good enough to build a toolkit with. ;) 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 Tue, Sep 14, 2010 at 7:42 PM, Benjamin Otte o...@gnome.org wrote: I'm actually not sure about that. First, we don't have any code that defines if an allocation is valid or even defines what a valid allocation is. Or do we? gtk_widget_get_allocation() at least doesn't do anything there. yes, we have GTK_WIDGET_ALLOC_NEEDED(). draw() should whine if an alloc is needed. I don't think there's much question here. Drawing without an updated allocation is just a bug, plain and simple. Widgets need this guarantee. For example if I'm coding GtkLabel, I should be able to create the PangoLayout in size_allocate and assume that I have the right layout in draw(). If you don't require updating the allocation, I might draw() some old text that has been changed. (Not saying GtkLabel works this way, I didn't look, just that if it did work this way it would be correct and would have worked in GTK 2.x. And it's certainly easier to write a correct widget if we keep this invariant.) 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 Tue, Sep 14, 2010 at 7:42 PM, Benjamin Otte o...@gnome.org wrote: On Tue, Sep 14, 2010 at 7:46 PM, Matthias Clasen matthias.cla...@gmail.com wrote: What about the expose_event / gtk_widget_send_expose_event stuff ? Do we want to merge what you have first and figure that out afterwards ? I want to figure that out afterwards. It's something I haven't figured out completely yet. I consider it a part of rendering-cleanup part 4: The hackfest menace. The thing about this is someone will have to go git-digging to get back the deleted docs and stuff. We end up intermediately deleting expose-event then bringing it back then moving it to WidgetWithWindow thingy. I can do it after you merge if you like rather than trying to explain what I mean in email. Havoc ___ gtk-devel-list mailing list gtk-devel-list@gnome.org http://mail.gnome.org/mailman/listinfo/gtk-devel-list
Re: rendering-cleanup-next
Fixed in http://git.gnome.org/browse/gtk+/commit/?h=rendering-cleanup-nextid=99f0da58168e3db6cdf8c27c4239afc600bef058 Thanks for pointing out that flag, I never realized it exists. Benjamin On Wed, Sep 15, 2010 at 2:36 AM, Havoc Pennington h...@pobox.com wrote: Hi, On Tue, Sep 14, 2010 at 7:42 PM, Benjamin Otte o...@gnome.org wrote: I'm actually not sure about that. First, we don't have any code that defines if an allocation is valid or even defines what a valid allocation is. Or do we? gtk_widget_get_allocation() at least doesn't do anything there. yes, we have GTK_WIDGET_ALLOC_NEEDED(). draw() should whine if an alloc is needed. I don't think there's much question here. Drawing without an updated allocation is just a bug, plain and simple. Widgets need this guarantee. For example if I'm coding GtkLabel, I should be able to create the PangoLayout in size_allocate and assume that I have the right layout in draw(). If you don't require updating the allocation, I might draw() some old text that has been changed. (Not saying GtkLabel works this way, I didn't look, just that if it did work this way it would be correct and would have worked in GTK 2.x. And it's certainly easier to write a correct widget if we keep this invariant.) Havoc ___ gtk-devel-list mailing list gtk-devel-list@gnome.org http://mail.gnome.org/mailman/listinfo/gtk-devel-list
Re: rendering-cleanup-next
+ g_return_if_fail (GTK_WIDGET_ALLOC_NEEDED (widget)); g_return_if_fail( ! GTK_WIDGET_ALLOC_NEEDED (widget)); right? 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 11:16 -0400, Havoc Pennington wrote: * 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. I'm personally a tiny bit uneasy about dropping bg None, as in some cases its really required to do flicker-free stuff in X. However, with a modern Gtk+ these situations are quite rare, and I don't think any of these changes really cause any flicker, since: 1) In practice almost all non-toplevel GdkWindows in a typical Gtk+ app are client-side (i.e. have no corresponding native window). This means that mapping them does not make X clear the area to the background, and there is no flicker. 2) Even for windows with native windows there is often no flicker, as the X11 gdk backend aggressively set+unsets background None around things like moves, resizes, maps and reparents in order to not render anything to the screen until we do the expose. -- =-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-= Alexander LarssonRed Hat, Inc al...@redhat.comalexander.lars...@gmail.com He's a leather-clad ninja jungle king looking for 'the Big One.' She's a radical thirtysomething nun living homeless in New York's sewers. They fight crime! ___ 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 18:57 +0200, Benjamin Otte wrote: 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? Thanks for poking me on irc as I really have not been following this thread (and it did not jump out at me with the explicit CC, I'll see if I can configure something for that in my mail reader... or try to at least glance at all or most of the gtk-devel traffic that rolls in) As we briefly discussed on irc; I don't really care so much about the name of the API so long as it make's sense. I suppose that if we desire to change the name; the most natural name will be to match clutter's get_preferred_width(). While on the topic of gtk_size_request_get[_preferred]_width(), I think we really should be keeping both minimum and natural values as 2 return values of the same api... because: - For any container code that needs to make a request, whether the natural or minimum size gets ignored or not; its better to at least force a some awareness about the height-for-width apis. In other words it should be impossible for a container widget author to use widget.get_minimum_width() and completely ignore the fact that there is a natural width (at least when a natural width is ignored, it should be obviously intentional or be code that needs fixing). - While the exported api could be different than the internal one, The signature should IMO be the same signature as the method invoked on the GtkSizeRequestIface - In most implementations of the -get_width() request api there are some padding and border/style values to calculate which apply to both the minimum and natural size... so it makes sense to calculate both minimum/natural in a single pass (it would be a shame to make them be calculated twice in order to separate those calls). For allocation apis, I would still prefer using an api that is clear about the allocation (widget.get_allocation() or widget.get_allocated_width())... as that is somehow less ambiguous and forces the caller to understand at least a little bit about allocations. Cheers, -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 ___ 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 Mon, Sep 13, 2010 at 4:26 AM, Alexander Larsson al...@redhat.com wrote: I'm personally a tiny bit uneasy about dropping bg None, as in some cases its really required to do flicker-free stuff in X. However, with a modern Gtk+ these situations are quite rare, and I don't think any of these changes really cause any flicker, since: 1) In practice almost all non-toplevel GdkWindows in a typical Gtk+ app are client-side (i.e. have no corresponding native window). This means that mapping them does not make X clear the area to the background, and there is no flicker. 2) Even for windows with native windows there is often no flicker, as the X11 gdk backend aggressively set+unsets background None around things like moves, resizes, maps and reparents in order to not render anything to the screen until we do the expose. Fair enough. It looked to me like the None still in GTK were all on scrollable areas (text and tree view, GtkLayout). So if those are just client-side anyway, None background isn't doing anything. 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 Sep 12, 2010, at 9:54 PM, Havoc Pennington wrote: Hi, On Mon, Sep 13, 2010 at 12:05 AM, John Ralls jra...@ceridwen.us wrote: Could also make it a gdk_x11 api. But maybe a hint that is a no-op on other backends is better. I'm in favor of keeping platform-specific stuff in platform-specific files and directories, but that's in large part just because I'm a bit compulsive when it comes to code organization. Making it gdk_x11 doesn't keep the platform-specific in platform-specific files. It would make all files touched by d3802ca8331ab09fe752407577b12d1525b5d89e now include gdkx.h and an #ifdef'd call to the gdk_x11 API. So textview, layout, treeview, viewport would all now have x11-specific code. The API need not be X specific, though the details depend on how Windows and Mac work and I don't know them. /** * gdk_window_set_clears_background: * @window: a #GdkWindow * @value: #TRUE to clear window to its background when damage occurs * Sure, so the declaration goes in gdkwindow.h and the implementation in x11/gtkwindow-x11.c with no-op stubs in quartz/gdkwindow-quartz.c and win32/gdkwindow-win32.c. I don't think the problem you're attacking actually occurs on quartz, so a no-op stub there should be fine. I don't know about Win32. Regards, John Ralls ___ gtk-devel-list mailing list gtk-devel-list@gnome.org http://mail.gnome.org/mailman/listinfo/gtk-devel-list
Re: rendering-cleanup-next
On Sep 13, 2010, at 6:54 AM, Havoc Pennington wrote: On Mon, Sep 13, 2010 at 12:05 AM, John Ralls jra...@ceridwen.us wrote: Could also make it a gdk_x11 api. But maybe a hint that is a no-op on other backends is better. I'm in favor of keeping platform-specific stuff in platform-specific files and directories, but that's in large part just because I'm a bit compulsive when it comes to code organization. Making it gdk_x11 doesn't keep the platform-specific in platform-specific files. It would make all files touched by d3802ca8331ab09fe752407577b12d1525b5d89e now include gdkx.h and an #ifdef'd call to the gdk_x11 API. So textview, layout, treeview, viewport would all now have x11-specific code. The API need not be X specific, though the details depend on how Windows and Mac work and I don't know them. I got a bit lost whether we are still going to add this API or not :) But for what it's worth, as far as I can see now, this API is fine from an OS X point of view. Especially with the two conditionals at the end of the function description there is likely enough room to do what is best suited for a given platform or to emulate the pieces that are required. -kris. ___ 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 4:23 PM, Havoc Pennington h...@pobox.com 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. Emacs can be set up to fix it - you aren't using some suck editor are you :-P The problem with trailing whitespace (and using tabs) is that you get whitespace changes in diffs, or have to be tip-toeing around to avoid them, one or the other. If GTK doesn't have a policy against... it sure ought to. (Surely gnome.org has some kind of git hook that can be enabled to block trailing whitespace in new commits... ) Anyhow, sure, if GTK has no policy that's fine. I assumed it had a sensible policy... We don't have a written-down policy, beyond 'fit in locally'. But I have become increasingly annoyed by trailing whitespace and mixed-in tabs, since they do show up in my editor nowadays. So maybe we should agree on a policy and put it in writing for 3.0. ___ 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 4:23 PM, Havoc Pennington h...@pobox.com wrote: Does GDK have enough information to know when to set background to None? The patch d3802ca Remove calls that try to set GDK_NO_BG basically removes hints to GDK that the background should not be repainted by the OS. I don't know how GDK would know this. I don't know exactly what the API should be. Maybe set_hint_no_clear() or set_autopaint() or whatever it is. The main thing is, I think pre- this patch, things probably did not flicker as much, and post- this patch, they are probably flickery and uglier. So that's a user-visible regression. It just doesn't seem like a good idea to me to regress that. The API for this may be something that's a no-op on Windows and/or Mac, I don't know. Could also make it a gdk_x11 api. But maybe a hint that is a no-op on other backends is better. I would say, expose-event should be defined to be exactly what comes from GdkWindow, no pre-processing or special handling _at all_, and it only happens on window widgets. In the GTK of the future, this expose-event thing would be like the other obscure list of events that only window widgets ever get (destroy-notify, visibility-notify, blah blah). These events should all be lifted up out of GtkWidget somehow and only be present on windowed widgets; maybe via an interface, I don't know. This makes sense to me, as a general direction. Also, the idea to separate the translation and the size in size_allocate is intriguing. ___ 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 Sun, Sep 12, 2010 at 5:05 PM, Matthias Clasen matthias.cla...@gmail.com wrote: Also, the idea to separate the translation and the size in size_allocate is intriguing. A prior art thing I thought of that's relevant, Clutter has the translation transform *and* the allocation origin. The clutter model is that there's a transform (a full matrix, 3D of course not 2D as in Cairo) which only affects painting (first you do layout, then you can transform when painting so the actor need not paint inside its allocation). I guess the allocation.x,y translation is conceptually part of the allocation (or part of layout). The thing is that the x,y is only of interest to the parent layout container, while the width,height is also of interest to the child. With adjust_size_allocation, though, child-interesting allocation doesn't really match what the parent assigned anyhow. So gtk_widget_size_allocate() (assigning allocation) could take raw allocation including x,y and then the virtual method (going to child) gets adjusted allocation with no x,y perhaps. The x,y could be handled with no virtualization i.e. just stored by GtkWidget rather than storing it in the size_allocate default handler. Widget implementations could never look at GtkAllocation, only at get_allocated_width/height (whatever that would be called). Container implementations would look at GtkAllocation of their children 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 Sep 12, 2010, at 2:05 PM, Matthias Clasen wrote: On Sat, Sep 11, 2010 at 4:23 PM, Havoc Pennington h...@pobox.com wrote: Does GDK have enough information to know when to set background to None? The patch d3802ca Remove calls that try to set GDK_NO_BG basically removes hints to GDK that the background should not be repainted by the OS. I don't know how GDK would know this. I don't know exactly what the API should be. Maybe set_hint_no_clear() or set_autopaint() or whatever it is. The main thing is, I think pre- this patch, things probably did not flicker as much, and post- this patch, they are probably flickery and uglier. So that's a user-visible regression. It just doesn't seem like a good idea to me to regress that. The API for this may be something that's a no-op on Windows and/or Mac, I don't know. Could also make it a gdk_x11 api. But maybe a hint that is a no-op on other backends is better. I'm in favor of keeping platform-specific stuff in platform-specific files and directories, but that's in large part just because I'm a bit compulsive when it comes to code organization. If instead you expose it on all platforms, please do include the stubs for win32 and quartz so that they will at least continue to build and mention here the new function name(s) -- or file bugs against win32 and quartz saying that function foo is stubbed out. Aside: Could we have a recommendation in http://live.gnome.org/GTK%2B/BestPractices about adding functions with stubs and how best to alert the platform devs somehow other than breaking the build? (I rather like the bug report approach; it's too easy to miss something in a thread here, especially if it's long and very technical like this one.) Yes, I could just add it, but I'd like a bit of discussion first. Regards, John Ralls ___ gtk-devel-list mailing list gtk-devel-list@gnome.org http://mail.gnome.org/mailman/listinfo/gtk-devel-list
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
