GNOME Bugzilla – Bug 318807
Offscreen windows and window redirection
Last modified: 2009-11-01 06:33:46 UTC
It would be nice if Gtk+ allowed you to redirect rendering to a pixmap, for instance for printing. Similarly, it would be nice if we could support arbitrary transforming and stacking of widgets, for e.g. a canvas widget. I've written a patch that supports this.
Created attachment 53436 [details] [review] Core offscreen support This patch contains the core Gdk/Gtk+ changes needed to support redirection and offscreen windows. It introduces a new interface GdkWindowImpl that abstracts out GdkWindow implementations a bit, and X11 port is changed to support this. A new type GdkOffscreenWindow is implemented for offscreen windows, and support is added to GdkWindow to redirect output to a drawable (which is used by the offscreen window). A new event type GDK_DAMAGE is added which gets sent when the redirected drawable is getting drawn to.
Created attachment 53437 [details] [review] Some tests/* code for the new stuff.
Created attachment 53438 [details] [review] gtk_widget_get_snapshot() New function gtk_widget_get_snapshot() using the redirection to allow you to get a pixmap snapshot of a widget (+ children), even though it might be obscured.
Created attachment 53439 [details] [review] GtkExpander hack Total hack to GtkExpander to do nice scaling expansion animations. This is not nearly finished, but is an interesting example of what this stuff can do.
The expander example requires the patch from bug 318805, and the test cases don't work unless the patch from bug 318806 is applied. Note, all the patches were made against 2.8.latest. If they seem right I will port them to HEAD, which should be pretty easy.
Created attachment 62306 [details] [review] Just the pixmap redirection, against HEAD Here is a first shot at extracting just the pixmap redirection code and porting it to HEAD. This code is much smaller and less complex, but is still useful for things like printing widgets or moz/OOo theme hacks. There seems to be some issue with it, because when i use the testgtk snapshot test on e.g. an entry part of the pixmap is uninitialized.
Small things found while reading the patch: - remove_redirect_from_children has a comment thats copied from apply_redirect_to_children - the docs for gdk_window_redirect_to_drawable refer to gdk_window_new_offscreen, which is not part of the patch. The docs should mention that width, height can be -1 - from _gdk_window_calculate_full_clip_region: /* clip_region is relative to gc clip origin which is relative to the window */ + /* offset it to window relative: */ + tmpreg = gdk_region_copy (clip_region); + gdk_region_offset (real_clip_region, + gc->clip_x_origin, + gc->clip_y_origin); should you not be offsetting tmpreg there ?
Did not make it into 2.10, unfortunately
Still no progress?
CCing
Increasing version. I'd love to see this in GTK+ 2.12
I would like to see this move into gtk+ too!
Is this required to support widget embedding in canvas implementations?
Yes and no. You can also go the way, Gnome Canvas goes for embedding widgets, but using the offscreen approach, you'll be able to have correct stacking etc.
Created attachment 83122 [details] [review] Updated redirection patch Wanted to play a bit with this and had to update the patch so it applies to trunk. Attaching here to save others a bit of work :)
Created attachment 83123 [details] [review] Updated redirect patch, take 2 Even better if I don't include my silly spinbutton local changes.
*** Bug 164854 has been marked as a duplicate of this bug. ***
Many hungs failed in "Core offscreen support" patch.
Xan: Mayank seens to have problems testing this patch. Can you please tell us which of the patches we need to apply for proper testing?
My latest patch still applies with some warnings about offsets. Anyway, updating it to trunk as of today.
Created attachment 86300 [details] [review] Updated patch.
So your patch includes everything that the other patches provide, but is more up to date? (If so, please mark the obsolete patches as obsolete, thanks.)
My patch is the "Just the pixmap redirection, against HEAD" from comment #6 modified to apply to current trunk. The other patches are either more complex versions of the same functionality or test cases for it, so I think they are ok as they are.
Created attachment 87459 [details] offscreen patch + test case against r17785 this new patch applies against recent trunk, contains the testgtk test case from alex' original patch and a few fixes from me: + fixed coordinates for !NO_WINDOW widgets in gtk_widget_get_snapshot; this fixes garbage snap offsets for gammacurve, tree, drawingarea, text, handlebox, etc. + plugged pixmap leak in testgtk most testgtk cases seem to work now, except: - viewports in scrolled windows (e.g. the main testgtk window when picking just its viewport): obscured regions are not properly snapshot (appear black) - "alpha window", produces gdk-error (drawable depths mismatch) - listitems in "list": obscured regions contain garbage - "spinbutton": arrows are missing from snapshot - "layout": snapshooting scrolled window (lower left window corner) produces garbage - "layout": snapshooting scrolled layout produces pixmap at wrong scroll offset - "big windows": junk data at large scroll offsets
Any decision about when (version and/or date) is this patch expected to be available in GTK+/GDK?
I don't think there is but Tim was planning to work on getting it into 2.14.
Is this becoming obsolete now that with Xcomposite, it's possible to do this already and much simpler? That of course requires a working Xcomposite, but the simplicity is appealing. http://mail.gnome.org/archives/gtk-devel-list/2007-December/msg00282.html http://macslow.thepimp.net/?p=153
After looking at and playing with gtk-offscreen-1 I agree with Behdad, that the patch described here is obsolete.
The Xcomposite version is not portable outside the X11 backend, nor is it supported on Xservers that don't support composite. Anyway, the more interesting part of the offscreen rendering patch is the events and grab handling. Is it possible to implement this using Xcomposite for the redirection?
I was thinking about this at GUADEC, and what I want to see is a way to be able to render widgets to arbitrary cairo contexts. That is, say, to SVG, and get a vector output, not bitmap. Neither this patch nor the Xcomposite approach solve that. But that's something we should aim for. Perhaps a new, extended-render interface ;).
the Xcomposite approach is a huge hack, so it doesn't invalidate the original approach. the really interesting parts of the offscreen redirection are the events and grab handling, and that is not possible to achieve anyway with the Xcomposite approach, as Alex said. I'll be working on fixing the patch Tim kindly ported to trunk and finishing the event handling as well; I'll try and get it in a working/reviewable state by the time of the hackfest, so I can get a much wider bandwidth for the review and discussion.
Thanks Emmanuele; great plan.
Created attachment 104587 [details] [review] [PATCH] Full offscreen redirection patch this is the full offscreen redirection patch from attachment 53436 [details] [review] ported to the current trunk. it incorporates the changes made by Tim inside the attachment 87459 [details] and the test case for testgtk. note: this is just the *original* patch, so it still has the bugs and the TODO items, but it should be enough for people to use as a baseline. I also have isolated a patchset for the GdkWindow implementation cleanups - moving GdkWindow functionality into a GdkWindowImpl interface implemented by the backens. this could be applied even without the offscreen redirection, as it makes GDK a lot saner (and allows for expansion). given the size (~7500 lines of diff) this is probably not the best way to comment on the patch; I used a git-svn repository, so I could probably push it somewhere for people to clone and review the single commits; I tried to reduce the patch to something "digestible", but even so it's fairly intrusive.
okay, after IRC discussion with timj, here's a break down of the patch: patches 0001..0005: add GdkWindowImpl interface and let the X11 backend implement it patch 0006: add support for redirecting drawables patch 0007: add redirection to GdkWindow patch 0008: pointer coordinate transformations patch 0009: allow offscreen event rewrite patches 0010..0013: add GdkOffscreenWindow and X11 implementation patch 0014: add GtkWidget::damage-event and get_snapshot() patches 0015..0016: add test cases what's missing: - the GdkWindow signals for translating coordinates from parent to window and vice versa when implementing a widget using GdkOffscreenWindow. the first five patches are also the bulk of the overall offscreen redirection patch.
Created attachment 104923 [details] [review] [PATCH 01/16] Abstract GdkWindow into an interface GdkWindow is suitable for becoming a GTypeInterface implemented by each backend of GDK, as opposed to the current situation where every backend must provide a public symbol. Every required function is now a virtual function of the GdkWindowImpl interface. All the per-backend public symbols are now added to the GdkWindow API and call the corresponding GdkWindowImpl virtual function. Signed-off-by: Emmanuele Bassi <ebassi@gnome.org> --- gdk/Makefile.am | 12 +- gdk/gdk.symbols | 57 ++-- gdk/gdkinternals.h | 21 +- gdk/gdkwindow.c | 855 ++++++++++++++++++++++++++++++++++++++++++++++++++- gdk/gdkwindowimpl.c | 57 ++++ gdk/gdkwindowimpl.h | 133 ++++++++ 6 files changed, 1073 insertions(+), 62 deletions(-) create mode 100644 gdk/gdkwindowimpl.c create mode 100644 gdk/gdkwindowimpl.h
Created attachment 104924 [details] [review] [PATCH 02/16] Fix return type of _gdk_window_new() The backend-specific function should actually return a value, being a constructor. Signed-off-by: Emmanuele Bassi <ebassi@gnome.org> --- gdk/gdkinternals.h | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)
Created attachment 104925 [details] [review] PATCH 03/16] Constify arguments to match with the current public API The passed GdkRegion in the shape_combine_region() and move_region() virtual functions must be constified to match with the public API and avoid the compiler complaining loudly. Signed-off-by: Emmanuele Bassi <ebassi@gnome.org> --- gdk/gdkwindowimpl.c | 2 +- gdk/gdkwindowimpl.h | 155 +++++++++++++++++++++++++-------------------------- 2 files changed, 78 insertions(+), 79 deletions(-)
Created attachment 104926 [details] [review] [PATCH 04/16] Compilation fixes for GdkWindow A stray chunck of the offscreen redirection patch creeped into the current incremental break down of the offscreen redirection support. Also, gdk_window_raise_internal() should be defined earlier - and possibly inlined, as it is simple enough for that. Finally, constify some parameters to match the current public API and the GdkWindowImpl virtual functions signature. Signed-off-by: Emmanuele Bassi <ebassi@gnome.org> --- gdk/gdkwindow.c | 78 +++++++++++++++++-------------------------------------- 1 files changed, 24 insertions(+), 54 deletions(-)
Created attachment 104927 [details] [review] [PATCH 05/16] Port the X11 backend to the new GdkWindowImpl interface Initial porting of the GDK X11 backend to the GdkWindowImpl interface. The part of the public API that is declared in gdkwindow.h and that is now implemented via virtual functions has been made static (or private to the X11 backend). Signed-off-by: Emmanuele Bassi <ebassi@gnome.org> --- gdk/x11/gdkevents-x11.c | 4 +- gdk/x11/gdkgeometry-x11.c | 67 +---- gdk/x11/gdkprivate-x11.h | 8 + gdk/x11/gdkwindow-x11.c | 686 +++++++++++---------------------------------- gdk/x11/gdkwindow-x11.h | 3 + 5 files changed, 183 insertions(+), 585 deletions(-)
Created attachment 104928 [details] [review] [PATCH 06/16] Allow redirection of drawables Add a virtual function to GdkDrawableClass to allow getting a source drawable - that is, a GdkDrawable that is always guaranteed to be valid and on-screen. The default implementation is to return the same drawable, but drawables that can have offscreen redirection will have to override the default implementation. GdkPixmap and GdkBitmap constructors must be updated to use the source drawable instead of the passed drawable; this means moving them to the public symbols. The backends need to implement an internal symbol. Signed-off-by: Emmanuele Bassi <ebassi@gnome.org> --- gdk/gdk.symbols | 11 +++-------- gdk/gdkdraw.c | 25 +++++++++++++++++++++++++ gdk/gdkdrawable.h | 3 ++- gdk/gdkinternals.h | 35 +++++++++++++++++++++++++++-------- gdk/gdkpixmap.c | 41 +++++++++++++++++++++++++++++++++++++++++ gdk/x11/gdkpixmap-x11.c | 30 +++++++++++++++--------------- 6 files changed, 113 insertions(+), 32 deletions(-)
Created attachment 104929 [details] [review] [PATCH 07/16] Initial support for redirecting GdkWindows to another drawable Append a structure to GdkWindow to contain the redirection data, and another to contain the children data of a redirected window. The redirection is directly applied to a GdkDrawable and can be removed when the GdkWindow has finished painting itself. Signed-off-by: Emmanuele Bassi <ebassi@gnome.org> --- gdk/gdkevents.h | 3 +- gdk/gdkwindow.c | 479 ++++++++++++++++++++++++++++++++++++++++++++++++++++++- gdk/gdkwindow.h | 61 +++++++- 3 files changed, 536 insertions(+), 7 deletions(-)
Created attachment 104930 [details] [review] [PATCH 08/16] Override pointer/window retrieval We need to compensate and transform the coordinates to retrieve the correct window in case the passed one is an offscreen GdkWindow. Signed-off-by: Emmanuele Bassi <ebassi@gnome.org> --- gdk/gdkdisplay.c | 92 ++++++++++++++++++++++++++++++++++++++++++++++++++--- 1 files changed, 86 insertions(+), 6 deletions(-)
Created attachment 104931 [details] [review] [PATCH 09/16] Compensate for offscren when synthesising 2- and 3-button press events If the event window is an onscreen redirection of an offscreen window we need to create a new event directed to the real window as well. For this, we need to call _gdk_event_rewrite_for_offscreen(), which is implemented elsewhere, and add a new event queue manipulation function, called _gdk_event_queue_append_after(), which appends a GdkEvent after another event (or to the tail of the queue, in case no event was found). Signed-off-by: Emmanuele Bassi <ebassi@gnome.org> --- gdk/gdkevents.c | 51 +++++++++++++++++++++++++++++++++++++++++++++++++-- 1 files changed, 49 insertions(+), 2 deletions(-)
Created attachment 104932 [details] [review] [PATCH 10/16] Declare the new internals for offscreen windows All the offscreen handling will be done inside GdkOffscreenWindow, which has no public headers. Before we add the implementation we need a lot of function declarations. Signed-off-by: Emmanuele Bassi <ebassi@gnome.org> --- gdk/gdkinternals.h | 66 ++++++++++++++++++++++++++++++++++++++++++++++----- 1 files changed, 59 insertions(+), 7 deletions(-)
Created attachment 104933 [details] [review] [PATCH 11/18] Implement GdkOffscreenWindow GdkOffscreenWindow is a GdkWindow completely rendered into an offscreen buffer. Its implemenetation is common to all the backends. Every child of a GdkOffscreenWindow will be offscreen as well; every event received on a redirected window will be sent to its offscreen counterpart. Limitations of the GdkOffscreenWindow: * Offscreen windows must be child windows * Offscreen windows can't be the child of a foregin window, nor contain foreign windows * All drawing must be within begin/end_paint * The confine_window argument of pointer grabs doesn't work * GDK_POINTER_MOTION_HINT_MASK isn't effective * You can't reparent between offscreen windows and normal windows The TODO list is still long, at this point. Signed-off-by: Emmanuele Bassi <ebassi@gnome.org> --- --- gdk/Makefile.am | 1 + gdk/gdkoffscreenwindow.c | 2506 ++++++++++++++++++++++++++++++++++++++++++++++ gdk/gdkwindow.h | 27 +- 3 files changed, 2525 insertions(+), 9 deletions(-) create mode 100644 gdk/gdkoffscreenwindow.c
Created attachment 104934 [details] [review] [PATCH 12/16] Finish offscreen redirection inside GdkWindow Compensate for redirection when clearing the background of a GdkWindow, and add checks for completely offscreen windows. Signed-off-by: Emmanuele Bassi <ebassi@gnome.org> --- gdk/gdkwindow.c | 144 ++++++++++++++++++++++++++++++++++++++++++++++++------ 1 files changed, 128 insertions(+), 16 deletions(-)
Created attachment 104935 [details] [review] [PATCH 13/16] Update offscreen redirection support in the X11 backend Provide all the backend-specific implementations of the required offscreen handling functions. Signed-off-by: Emmanuele Bassi <ebassi@gnome.org> --- gdk/x11/gdkdisplay-x11.c | 13 ++++- gdk/x11/gdkdisplay-x11.h | 5 ++ gdk/x11/gdkdnd-x11.c | 8 +++ gdk/x11/gdkdrawable-x11.c | 10 ++++- gdk/x11/gdkevents-x11.c | 13 +++++- gdk/x11/gdkgeometry-x11.c | 6 ++ gdk/x11/gdkinput.c | 7 ++- gdk/x11/gdkmain-x11.c | 108 ++++++++++++++++++++++++++++++++++------- gdk/x11/gdkprivate-x11.h | 1 + gdk/x11/gdkproperty-x11.c | 12 ++++- gdk/x11/gdkselection-x11.c | 8 ++-- gdk/x11/gdkwindow-x11.c | 115 +++++++++++++++++++++++--------------------- 12 files changed, 218 insertions(+), 88 deletions(-)
Created attachment 105065 [details] [review] updated pixmap redirection patch Ebassi, i don't think bugzilla is the right medium to add so many individual patches. if each needs 2 or 3 iterations before going in, we'll definitely loose track very soon. the splitting might be helpful, but please do something like packaging up the patches in a tarball, properly numbered, before attaching. (and, i really don't need diffstat results in bugzilla comments, it's just adding noise to an already overly lengthy bug report.) as for proceeding with this patch, i'd like to get the pixmap redirection in first, and then start over with a fresh offscreen patch based on the redirection commit. Alex separated this in Comment #3 and Comment #6, and i updated it in Comment #24. here is the newest iteration of that patch. relative to Comment #24, it contains these updates: - ChangeLog started, to keep track of the changes in the patch - docu updates - made GdkWindowRedirect private - removed damage idle handler, use event queue open issues with this patch that i think need fixing before it goes into trunk: 1) "alpha window", produces gdk-error (drawable depths mismatch) 2) "spinbutton": arrows are missing from snapshot => the arrows are not children of widget->window (figured by Ebassi), changing gtk_widget_snapshot to snapshoot gtk_widget_get_parent_window() instead of widget->window should fix this. 3) "layout": snapshooting scrolled layout produces pixmap at wrong scroll offset => probably the same as "big windows: junk data at large scroll offsets" => i'd guess this is the 16->32 bt translation code in Gdk, needs investigating 4) the non-X11 backends need to be adapted to support _gdk_window_new, _gdk_window_reparent and possibly _gdk_windowing_window_get_visible_rect. for the later, i'd like figure why simply using the window dimensions isnt good enough. 5) we need a unit test that validates the snapshooting of visible widget regions and a test to verify damage event delivery. issues that might be ok fixing *after* the patch got into trunk: - non-visible widget regions aren't properly snapshot (appear black), this can be tested by snapshooting a partially visible GtkViewport widget in testgtk. => this is most probably the _gdk_window_calculate_full_clip_region logic - listitems in "list": obscured regions contain garbage; => probably the same as above - "layout": snapshooting scrolled window (lower left corner) produces garbage => probably the same as above
(In reply to comment #48) > Created an attachment (id=105065) [edit] > updated pixmap redirection patch > > Ebassi, i don't think bugzilla is the right medium to add so many individual > patches. okay. I have a git-svn repository I can push, but it's over http so it'll be extremely slow at cloning. > as for proceeding with this patch, i'd like to get the pixmap redirection in > first, and then start over with a fresh offscreen patch based on the > redirection commit. sounds like a plan. the real offscreen patch is not big, and can be split further. the huge bit, as I said in comment 34, is the GdkWindowImpl code; that should go in a separate bug and should also be discussed on gtk-devel-list as it implies various changes in all the current backends. > - ChangeLog started, to keep track of the changes in the patch this usually creates a lot of conflicts; could it be moved to a ChangeLog.offscreen? > 2) "spinbutton": arrows are missing from snapshot > => the arrows are not children of widget->window (figured by Ebassi), > changing gtk_widget_snapshot to snapshoot gtk_widget_get_parent_window() > instead of widget->window should fix this. just checked, and applying this: - pixmap = gdk_pixmap_new (widget->window, + pixmap = gdk_pixmap_new (gtk_widget_get_parent_window (widget), did not fix the spin buttons case. also, I've applied the change from Matthias in comment 7 but didn't get any change in the output. > issues that might be ok fixing *after* the patch got into trunk: > - non-visible widget regions aren't properly snapshot (appear black), this > can be tested by snapshooting a partially visible GtkViewport widget in > testgtk. > => this is most probably the _gdk_window_calculate_full_clip_region logic > - listitems in "list": obscured regions contain garbage; > => probably the same as above > - "layout": snapshooting scrolled window (lower left corner) produces garbage > => probably the same as above yep, confirming - I need to look into it, because either the full_clip_region() is not returning the correct area (could also be related to the layout snapshots), or the pixmap is not being correcly drawn for the entire area.
Created attachment 105320 [details] [review] updated pixmap redirection patch (4th update) this new version fixes clipping for the main redirected window as described by Alex on gtk-devel-list: http://mail.gnome.org/archives/gtk-devel-list/2008-February/msg00052.html
Created attachment 106494 [details] [review] [PATCH] GdkOffscreenWindow and tests GdkOffscreenWindow and tests, rebased on Tim's patch in attachment 105320 [details] [review]. This is just a rebased patch: I'm still working on fixing the coordinates and the event redirection.
Created attachment 106553 [details] [review] updated pixmap redirection patch (5th update) after a discussion with Alex, i removed _gdk_windowing_window_get_visible_rect() now, so the only remaining backend specific changes needed by the patch are renaming of gdk_window_reparent() and gdk_window_new().
Created attachment 106628 [details] [review] [PATCH] Full offscreen redirection patch Integrated the changes Tim made, this is again the full patch against trunk. There are artifacts, and not every widget is drawing properly, but pointer, button and key events are properly redirected.
Created attachment 106979 [details] [review] updated pixmap redirection patch (6th update) fixed ABI incompatible editing mistake, (_gtk_reserved5 removal from GtkWidgetClass), spotted by Richard.
Created attachment 107518 [details] [review] updated pixmap redirection patch (7th update) this new patch version fixes drawable depth mismatches when an ARGB window is snapshot and the resulting pixmap is displayed in an RGB window.
Hi I tried to exercise this patch. I obtained some limited results but no redirection. I tried to apply it over 2.13.0, trunk head and trunk as of 2008-03-18. Could you just describe a little bit the environment for using offscreen rendering with redirection ? Thanks
(In reply to comment #55) > updated pixmap redirection patch (7th update) I've committed my final offscreen rendering patch to trunk now. The version in trunk has all snapshot crashers fixed, the cleaned up Gdk backend API as discussed with Alex, Gdk clipping fixes so obscured area snapshooting works, ARGB visuals work, spinbuttons work and it supports specification of a clipping rectangle for snapshooting. What's missing is unit testing code for offscreen rendering. I've something unfinished in the queue for this already. So the next point on my list is finishing a pixel based unit test for the offscreen rendering and then tackle offscreen event processing. Ebassi, it probably makes sense to rebase your patch on recent trunk now. Ideally I'd like to see one patch regarding cleanups/refactorings and a second one regarding event processing changes (if such a split is possible, I don't remember the remaining patch off head).
Created attachment 111409 [details] test for widget snapshots
Comment on attachment 111409 [details] test for widget snapshots I've created this test program to test the new get_snapshot() function. When saved the image looks damaged and has some random pixels. Any ideas why that happens?
(In reply to comment #58) > Created an attachment (id=111409) [edit] > test for widget snapshots Thanks, but what we need are automated unit tests. What you've attached is pretty much covered by the new testgtk test already. (In reply to comment #59) > (From update of attachment 111409 [details] [edit]) > I've created this test program to test the new get_snapshot() function. When > saved the image looks damaged and has some random pixels. Any ideas why that > happens? Try snapshotting a _visible_ widget, e.g. by inserting gtk_widget_show_all (window) in your code.
Created attachment 113390 [details] [review] [PATCH] Offscreen redirection patch rebased on a recent trunk this is the full offscreen redirection patch rebased on a recent trunk. it addresses a couple of points expressed by mitch on IRC as well; needs further review and cleanups before being applied.
I've looked at the part of the patch that introduces the window impl interface, mostly from the quartz perspective. All in all it looks good, and only moves code around i.e. no logic changes, which is good. Some minor details: the new file (gdkwindowimpl.c) has <config.h> instead of "config.h", and it doesn't use G_DEFINE_*. The X11 backend's *_set_back_pixmap function removes the preconditons on what argument combinations are OK, but the new generic function doesn't contain those checks, it should probably be moved to the generic function.
Created attachment 113525 [details] [review] [PATCH] GdkWindow reorganization preparing for GdkOffscreenWindow this is the GdkWindow cleanup and refactoring that's the basis for the GdkOffscreenWindow of attachment 113390 [details] [review]. this version of the patch addresses the points raised by mitch and rhult.
(In reply to comment #63) > Created an attachment (id=113525) [edit] > [PATCH] GdkWindow reorganization preparing for GdkOffscreenWindow > > this is the GdkWindow cleanup and refactoring that's the basis for the > GdkOffscreenWindow of attachment 113390 [details] [review] [edit]. > > this version of the patch addresses the points raised by mitch and rhult. Great, thanks. Remaining comments from me: - IMHO, the code would be simpler with a bit of consolidation of WindowImpl.move, WindowImpl.resize and WindowImpl.move_resize, (similar to clear_area and clear_area_e) e.g. like: + void (* move_resize) (GdkWindow *window, + gboolean withmove, + gint x, + gint y, + gint width, + gint height); so we implement the three ops with one interface method: move: move_resize (w, TRUE, x, y, -1, -1); resize: move_resize (w, FALSE, 0, 0, width, height); move_resize: move_resize (w, TRUE, x, y, width, height); - the code should be using TYPE defines instead of _gdk_window_impl_get_type, fixng bug #320482 would prolly help here... - useless indentaiton change: @@ -3647,9 +4393,9 @@ _gdk_window_calculate_full_clip_region (GdkWindow if (!GDK_WINDOW_IS_MAPPED (child) || child_private->input_only) continue; - - window_get_size_rectangle (child, &visible_rect); - + + window_get_size_rectangle (child, &visible_rect); + /* Convert rect to "window" coords */ visible_rect.x += child_private->x - x_offset; visible_rect.y += child_private->y - y_offset; Please apply.
The last patch doesn't contain gdkwindowimpl.[ch] :(
Created attachment 113532 [details] [review] [PATCH] GdkWindow reorganization preparing for GdkOffscreenWindow (v2) new version of the patch, with GdkWindowImpl::move, ::resize and ::move_resize coalesced into a single ::move_resize(). merging the actual X11 implementation proved to mess up the code, so I kept the move, resize and move_resize functions separated. instead of using a gboolean for the with_move argument, I'd probably use a private enumeration to retain some cleanliness when choosing between the three actions. this time, gdkwindowimpl.[ch] have been added.
committed the GdkWindow cleanup patch to trunk 2008-06-27 Emmanuele Bassi <ebassi@gnome.org> Abstract some GdkWindow API into an interface that the backends must implement. (based on a patch by Alex Larsson) * gdk/Makefile.am: Add gdkwindowimpl.[ch] * gdk/gdk.symbols: Move symbols around. * gdk/gdkinternals.h: * gdk/gdkwindowimpl.[ch]: Move some of the GdkWindow API we require from the backends to a GInterface that the backends should implement instead. * gdk/gdkwindow.c: Provide some of the GdkWindow public API as a wrapper call around the GdkWindowImpl interface vtable. * gdk/x11/gdkevents-x11.c: * gdk/x11/gdkgeometry-x11.c: * gdk/x11/gdkprivate-x11.h: * gdk/x11/gdkwindow-x11.c: * gdk/x11/gdkwindow-x11.h: Update the X11 backend to implement the GdkWindowImpl interface.
Created attachment 113693 [details] [review] GtkOffscreenWindow patch that applies on current trunk New patch on top of current trunk
Created attachment 113713 [details] [review] Patch that exposes windows always in parent -> child order GtkOffscreenWindow has this comment: + * change expose order so that children are exposed before parent, to + avoid double drawing due to damage from children coming after parent expose. + Event with the right order we will expose twice, although both with an + uptodate pixmap. + Not sure how to fix this. Perhaps with a get_and_clear_damage() call? I don't quite know how this is supposed to work (exposing childrem before parents) but i tried to implement it anyway, with the result that the drawing in testoffscreen was even more broken. So something must be fishy with the above rationale about DAMAGE, or simply something in the code broken. I changed my patch to expose children always *after* their parents, which intuitively makes more sense to me, and this magically fixed all redrawing problems in testoffscreen. Don't know if it generated double expose events now. Attached patch applies cleanly on top of the rebased patch above, but also on top of clean trunk.
(In reply to comment #69) > I changed my patch to expose children always *after* their parents, > which intuitively makes more sense to me, and this magically fixed > all redrawing problems in testoffscreen. I've applied the patch of attachment 113693 [details] [review] and the one of attachment 113713 [details] [review] and the result is the same of the old patch: - if I run testoffscreen without arguments (like: "./testoffscreen") I get the full redirection for all the items, a core maxing out at 100% and redraw issues: the TextView is empty and the scrolling layout is empty; - if I run testoffscreen with an argument (like: "./testoffscreen 1") I get the redirection only for the "offscreen in offscreen" button, and a core maxing out at 100%. I think that the core maxing out is due to the scrolling layout - but the redraw issues still remain. the video card is an intel 915/945.
I forgot, you need to fix the recursion yourself by s/TRUE/FALSE/ in gtk_offscreen_box_damage_event()
Created attachment 113762 [details] [video] testoffscreen output with FALSE inside GtkOffscreenBox's ::damage_event handler the CPU usage returns normal, but I still have drawing issues - possibly bugs exposed by the patch in attachment 113713 [details] [review] and by the offscreen redirection code. I'm attaching a screencast showing the errors I get: - the scrolling layout doesn't scroll anymore, and the buttons inside it are not clickable or not redrawn - sometimes widgets just disappear
I've created an "offscreen" branch in a shallow Git repository now to avoid reattaching patch fixes here and also to allow easy rebasing to follow trunk frequently. If others want to hack on the patch as well, feel free to clone and ping me for pulling your patches or send them along via email. http://git.testbit.eu/Gtk-shallow/log/?h=offscreen Beware of frequent rebasing though (allthough generally frowned upon in the Git world, we have to do this because git-svn can't dcommit branch merges).
Just adding this so that its not forgotten: The offscreen windows work could be expanded to create a fully subwindow-less Gtk+. I posted some thoughts about this to the mailing list: http://mail.gnome.org/archives/gtk-devel-list/2008-August/msg00003.html
Not wishing to cause undue traffic but will this feature make it into the next release of Gtk and, by proxy, Gnome 2.24. I ask as this would be a nice feature to have in a modern toolkit. I saw the QT equivalent (QWidget/Graphic View) and was duly impressed and I also played with Macslow's work (compositor only with GLX_EXT_texture_from_pixmap but Wow!) but think that letting this patch in would be a nice step towards Gnome 3.0. It may even go some way to the transition effects that link into timelines etc for animations within GTK itself. Apologies for the divergence - so any news on a yay or nay?
Created attachment 117773 [details] [review] Recent state of the offscreen branch Attaching the most recent version of the offscreen branch work, in case some people want easy access for review or testing. The branch's web interface can be used to read up on the commit log entries (will be converted to ChangeLog entries when this is merged into upstream), progress continues to be tracked in git: http://git.testbit.eu/Gtk-shallow/log/?h=offscreen
A small typo error in gdkoffscreenwindow.c file if (width) *width = offscreen->width; if (height) *height = offscreen->width; it should be *height = offscreen->height
Thanks, good spot :) I have fixed this locally and will have it committed to the git repository as soon as possible.
I've been doing some thinking about how to move the offscreen work into a direction that lets us cleanly do: * Allow a fully subwindowless gtk+ (in most cases) * Let you do clutter-style embedding of gtk widgets in weird ways I think this requires some restructuring and refactoring of the current offscreen code. Here is an approach that I think makes sense: First of all we split the "toplevel" aspect out of the current GdkOffscreenWindow class. This creates a new GdkWindow class that doesn't do any of the pixmap stuff. Instead all it does is handle events and clipping just like X would, but client side, while doing all drawing in its "parent" window (i.e. the closest ancestor that has a real drawable. Lets call this something like GdkVirtualWindow. So, GdkVirtualWindow draws in its parent non-virtual window, GdkWindow correctly clips all drawing as if the GdkVirtualWindows were normal X windows (i.e. non transformed axis-aligned rects), it propagates events from the parent automatically, etc. Now, if we made all child windows GdkVirtualWindow we immediately get a subwindowless Gtk+. I.e. We get no tearing on resize due to asynchronous server-side window resize/move. We can also add a new kind of background setting to such windows like GDK_TRANSPARENT_BG, meaning we don't clip the parent drawing from this window so you can get non-rectangular widget drawing without using RGBA windows, composite, etc. There are some cases where the native X window is required, but I think it is possible to handle this on an as-needed basis. I.E. when the app first requests the X id of a GdkWindow, or when adding a non-virtual child, etc, we can automatically convert the GdkWindow to an X window by replacing GdkWindow->impl (in all virtual parents). This is cool because we can actually take advantage of the weird ->impl system. Now, for the "clutter embedding gtk+ widgets" we want a few extra things. We want the ability to render into a pixmap, we want the ability to do fancy stuff with event coordinates (so we can render the pixmap weirdly), and we'd like to not really have to put the window into a parent GdkWindow (maybe we're just using opengl in our app, who knows). For this I think we want to add a new kind of toplevel window. I.e. you call gdk_window_new() with type = GDK_WINDOW_OFFSCREEN. This creates a window with no parent. I.E. its a toplevel just like the windows for GtkWindow and GtkPlug. However, instead of being an X window we just allocate a pixmap for it to render in. And we don't support adding non-virtual children to it. Then we add a way to feed input events into this window. Typically you'd take your events from clutter or whatever and backwards map them to the window coordinates based on the transformation you use when rendering the pixmap and let the offscreen window handle all the subwindow event stuff. If there is any highlevel code to be shared between apps that want to do this we could have a Gtk widget for the toplevel, similar to GtkWindow/GtkPlug. I think this approach is nice. It gives use subwindowless Gtk+ with no api changes when possible, and automatic fallback to old code when not. This minimizes resize flicker and allows us to do transparent widgets (other than the relative-bg hack). It also lets us do clutter integration in an easy way, while not mixing up the general virtual child window handling with the weird details of pixmap redirection and coordinate transformation. Opinions? I'll start looking at how this would look in code now.
Oh, and the API added is quite minimal. I think all we need is a new GdkWindowType, some way to set transparent bg, and maybe a way to flag that you really want a non-virtual window.
Timj wants to keep support for having offscreen windows in the window hierarchy without breaking it like GtkPlug/Socket. I think this should be doable in my approach by supporting the offscreen window as both toplevel and child of other window (but not rendering in its parent, instead to a pixmap).
A branch starting to implementing this new model is availible at: http://git.testbit.eu/Gtk-shallow/log/?h=offscreen-ng It supports offscreen windows, but also makes all child windows virtual. Its got a bunch of work to do, but a lot of things work already.
With the merge of the csw branch, this is fixed, I'd say.