GNOME Bugzilla – Bug 598299
XP theming in the ms-windows theme engine does not work
Last modified: 2013-10-08 21:08:00 UTC
The ms-windows theme engine does not work any more. When running gtk-demo, the window is initially blank, and the labels of the different demos appear only after one moves the pointer over them. Etc. Clearly related to csw.
Tor - what happens if you make xp_theme_is_active return "FALSE"? Do you get the "Win98" look?
Dunno for sure if it is the "Win98" look, but some look at least, yes. Should we do that as a stopgap until the problem with the xp theme code is fixed?
I'd recommend that. It should fall back to a Raleigh-ish theme, but using the Windows Color Palette. As if you'd turned off theming on the Windows desktop.
OK, pushed that to master and gtk-2-18. Keeping this bug open.
So would it be appropriate to change the summary of this bug to "The XP theming in the ms-windows theme engine does not work"?
Created attachment 148073 [details] [review] Partial fix to suggest what the issue is The issue seems to be that when gtk-demo is displayed initially, the list contents are drawn and then its parent window is drawn with a region passed to gdk_window_begin_paint_region that excludes the treeview rect. When wimp draws the win32 theme background, its output is not clipped to that region and it overwrites the list contents. The patch gets the clip region and sets it on the DC via setting it in the gdk_gc. There seem to be problems with drawing expanders though, which also look like clipping problems. More importantly, this seems like a kludge -- the dc obtained from gdk should already have this clip set. Is this supposed to happen elsewhere?
Have you news about this bug ?
Do you see any news here?
I would like use GTK+ 2.18.x on MS Windows because 2.16.x is very old.
Look before you leap. GTK 2.18 on Windows is very buggy. The client side windows work hasn't been fully debugged on Windows. GTK 2.16 is easily the best release of GTK on Windows.
2.16 is certainly not "very old", not even in "Internet time units". Anyway, which do you prefer: software that works, or software that is new? Anyway, when you say "use", you mean use from software you write, you are a programmer? You want ot use some new API present only in 2.18? Please then consider helping in actually debugging and fixing the problems in GTK+ 2.18 on Windows. But if you are an end-user, why would it matter to you what version of GTK+ you are using, especially as the actual experts here (me and Dominic) tell you that the newer version is actually buggier?
Hello! I've encountered this problem in the windows port of my PyGTK application (anomos.info) The program runs fine only if I remove the themes directory, otherwise there are a variety of drawing issues. What are some potential workarounds for me? How can I force xp_theme_is_active=FALSE from Python?
> What are some potential workarounds for me? Just "doing hard work and fixing the bug".
This comment might seem out of place or rude, but perhaps those who introduced the bug might take a look at it? In the software project I work on, commits introducing regressions are reverted.. or the developer who introduced the issue encouraged to fix it! As far as I know, when client-side windows were introduced, there wasn't any (non-code) documentation about the architecture changes, and / or notes as to where there is likely to be breakage (or changes required) in other back-ends or themes. The effort required to fix this for someone who doesn't know the code will obviously be higher than for someone who does. This page was the most prominent thing I could find: http://live.gnome.org/GTK+/ClientSideWindows Suggesting to _users_ of the GTK product that they should go and fix the bug seems like a rather futile, and aggressive thing to do. 1 in 1000 times, you might get someone competent or motivated enough to come and do it - correctly, and get a patch merged (and potentially a new contributor to GTK). I venture to guess that in the other cases, you will just wind people up, and reinforce the image that GTK developers don't care about regressions in their product. Gnome bugzilla doesn't have an "I'm affected by this bug" feature (only as implied by 'CC), so chiming in on existing bugs can be peoples way of letting developers know a particular bug is important to them. I think the developers would do well to show a little patience with users, and perhaps some gesture of "we'd like to get it fixed when we have time" rather than the "go away and fix it". If the answer is "no-one is interested in fixing this", say that.
> perhaps those who introduced the bug might take a look at it? Or perhaps you should just use the version of software as it was before the "introduction" of the bug? I mean, nobody promised you there would be any updates of Windows-specific functionality, or otherwise, in GTK+, right? So just use GTK+ as it was before "somebody broke it".
(In reply to comment #15) > > perhaps those who introduced the bug might take a look at it? > > Or perhaps you should just use the version of software as it was before the > "introduction" of the bug? I mean, nobody promised you there would be any > updates of Windows-specific functionality, or otherwise, in GTK+, right? So > just use GTK+ as it was before "somebody broke it". A perfectly sensible comment - and in fact, what we do when we ship our Win32 installer. I'm only CC'd so I know when it gets fixed, and we can ship later versions of GTK. You'll note that I've been quiet here until comment 14 - where I spoke up to suggest that telling people to go and fix it is not necessarily a very productive comment to make.
Well said, Peter. I have encountered this attitude numerous times while dealing with Gnome developers - frankly it completely turns me off the whole GNU/Gnome project. I would be far more interested in helping out on Gnome projects if people were a bit friendlier (or, more importantly, wouldn't waste my time by shipping software without testing for such major regressions.) I don't want to be a hater, I appreciate the work Gnome has done, but they could really take a hint from Ubuntu/Launchpad when it comes to dealing with community feedback. With regards to the bug at hand - yes, the solution was to use an older version. I suggest that you take the broken 2.18 bundles off of the website so other people don't download it and use it like I did.
OK, so Qt won, yay!
mmm, I don't see the point for a discussion here, Tor doesn't have time to fix this bug, maybe some day he can, but till then, someone could fix, he suggested that. I would do it but I don't have time to do it either, so guys, if you feel like you really need this fixed, just try to fix it and ask how to start doing it.
I've spent some time looking at the cause of the bug. I created a test application consisting of a GtkWindow with a GtkEntry contained in it. This application clearly shows the problematic behavior. When using gtk+ 2.16, gtk_entry_expose() is called four times when the widget needs to be fully repainted: 1. draw frame 2. draw text area 3. draw frame 4. draw text area When using gtk+ 2.18, I also see four gtk_entry_expose() calls, but the order is not the same and sometimes inconsistent. On Linux with X, I have seen different results, but the last one was always "draw text area". On Win32 I mostly see: 1. draw text area 2. draw frame 3. draw text area 4. draw frame For most environments, this appears not to be a problem, because the "draw frame" operation does not paint over the text area. When using the XP theme engine though, the "draw frame" operation does the XP theming stuff and that does paint over the text area, thus giving the problem. I can imagine that similar problems happen for other widgets that the XP theme engine touches and it looks to me that csw needs some work on generating expose events in the proper order. FYI, here's the source of my test application: #include <gtk/gtk.h> int main (int argc, char *argv[]) { GtkWidget *window; GtkWidget *entry; gtk_init (&argc, &argv); window = gtk_window_new (GTK_WINDOW_TOPLEVEL); entry = gtk_entry_new (); gtk_entry_set_text (GTK_ENTRY (entry), "entry"); gtk_container_add (GTK_CONTAINER (window), entry); g_signal_connect_swapped (window, "destroy", G_CALLBACK (gtk_main_quit), NULL); gtk_widget_show_all (window); gtk_main (); return 0; }
(In reply to comment #17) > With regards to the bug at hand - yes, the solution was to use an older > version. I suggest that you take the broken 2.18 bundles off of the website so > other people don't download it and use it like I did. (and comment #12) > What are some potential workarounds for me? How can I force > xp_theme_is_active=FALSE from Python? In the more recent Win32 builds of GTK+ 2.18 the whole XP theme has been effectively disabled in libwimp.dll. Are you sure that you use an up-to-date libwimp.dll? Please note that it resides in the lib/gtk-2.0/2.10.0/engines directory and not in the bin directory. When upgrading to a newer GTK+ release, it's easy to forget to upgrade this DLL.
http://git.gnome.org/browse/gnumeric/tree/tools/win32/patches/gtk-gdk-expose-order.patch
(In reply to comment #22) > http://git.gnome.org/browse/gnumeric/tree/tools/win32/patches/gtk-gdk-expose-order.patch Tested here with 2.20.0 and above patch on Windows XP, Vista and 7, and everything seems to be working as expected.
As that patch touches only cross-platform code, it would of course be essential to test it on X11 (and Mac OS X?), too.
Given - with my schedule my Gentoo box is a bit out of date, but I will update and test on there as well. Cannot really help with MacOS X though.
Created attachment 158811 [details] rendering-error.jpg Actually picked up a rendering error on Vista - will have a look on other systems later. See attached screen-shot.
I have seen a similar rendering bug occuring on Linux quite a few times recently with the most recent GTK+. It happened on a GtkTreeView (wrapped inside a GtkScrolledWindow) where the top-visible element gets updated and draws itself partly over the headers of the GtkTreeView. I'll try to catch a screenshot of it, but my guess is that it's a platform independent bug
What worries me about the patch is that it changes the drawing order rather than fixing the clipping. Aren't child windows supposed to be clipped so drawing on the parent won't draw on the child (in the common case)?
Erik, reagarding your comment #27 you may want to look at the screenshot attached to bug #616207.
(In reply to comment #29) > Erik, reagarding your comment #27 you may want to look at the screenshot > attached to bug #616207. cairo-1.9.6 at least do not fix the issue I noted in comment #26.
Alex, is the patch of comment #23 a valid approach?
The approach in comment #22 is just a workaround. It can still run into issues when a single widget repaints a single window (it will overdraw its childen who are not repainting). Comment #6 is more or less the right approach. The problem is that the theming code draws directly using the win32 api. If you draw using the gdk api then the gdk draw function will look at the drawable destination and automatically apply a clip region based on the drawable visible region. However, when you don't use the gdk apis to do the drawing this doesn't happen, so we overdraw other windows. However, the actual patch is wrong in several ways: 1) It overwrites the gc clip region, rather than add to it 2) It doesn't set back the old clip region on the gc when done 3) gdk_drawable_get_clip_region() returns the region which includes its child windows so we'll still overdraw those If you want to fix this for real i think you need to reuse some of the gdkwindow clipping automagic. Look at for instance gdkwindow.c:gdk_window_draw_rectangle() which is a very simple example of something that draws to a gc. All the "magic" is expressed in the BEGIN_DRAW/END_DRAW macro pairs. If you exposed these in functions and called that before and after xp_theme_draw() then i think everything should work.
if i compile my app with gtk 2.20 files, then deploy it with libpixmap.dll, libwimp.dll and libgail.dll from the gtk 2.16 zip, theming mostly works. lists seem to be problematic, as they appear to be blank until i hover with my mouse, then the text suddenly appears. but alex just wrote how it should be fixed. so can we expect this half year old bug to be resolved sometime?
compiling against one gtk version and then using earlier runtime libraries is not supported. (and even less supported is picking and combining dlls from different versions, if that is what you mean.) it might work but that might be just coincidental. it's pointless to discuss such stuff here.
i was *just saying* that it more or less works even with different versions. the main question was that can we expect this to be fixed sometime now that the solution is given, or do we have to wait another half year.
you can't expect anything
The patch from comment 22 actually does more than one thing: First hunk fixes the invariant in gdk_window_add_update_window. I believe this part is correct. I have never seen it make a difference, though. The rest changes the expose order to match what previous gtk+ versions did and what the theme expects. This patch is enough to put make my Gnumeric build behave. I don't believe it would hurt on any platform.
Created attachment 162552 [details] [review] gtk+-2.20.1-xptheme.patch (In reply to comment #36) > you can't expect anything Meaning somebody have to get the time to look at this, and many of us are either freelance, etc. Anyhow, I tried to do something as suggested in comment #32, and mostly it seems to work fine without any of the other patches. I probably did not do the interface exposing the correct way, but thought that it might be a start if somebody else gets more time before I do. It does have some problems though: - firstly Shadow Types/Borders are not always drawn correctly - look at the "Images" demo from Gtk2. - secondly, seems expander's causes a crash, and I don't have the time to look into in currently - I have gtk2_prefs's source if somebody are interested. Lastly though, it seems the problems from the patch in comment #22 is gone, as I could not reproduce the rendering problem in comment #26 again.
Created attachment 162554 [details] [review] gtk+-2.20.1-xptheme.patch
Sorry for the extra traffic - first version had some extra stuff from before cleaning it up a bit.
Created attachment 162628 [details] [review] gtk+-2.20.1-xptheme-v3.patch Did it again sorry, what happens when its getting late and hand editing a patch.
OK, this patch is really a nice progress. Nevertheless, when I tried to start gnumeric with gtk+-2.20.1 and this patch, I get a following crash: Program received signal SIGSEGV, Segmentation fault. 0x7c91dc7b in ntdll!LdrFindResource_U () from C:\WINDOWS\system32\ntdll.dll (gdb) bt
+ Trace 222214
And this is the error on console: Gdk:ERROR:gdkgc-win32.c:751:get_impl_drawable: code should not be reached
> And this is the error on console: > Gdk:ERROR:gdkgc-win32.c:751:get_impl_drawable: code should not be reached Its probably from the gdk_win32_hdc_get() call in xp_theme_draw(), and probably due to me removing the (!GDK_IS_WINDOW (win)) check, but with the drawable from gdk_window_get_internal_paint_info() anything crashed on startup. I am trying to get a gcc/gdb combination that actually works with a debug build of Gtk2 (without gdb crashing), so I could not even debug the problem I mentioned above myself yet - any possibility that you can trace for what type of widget that occurred?
Breakpoint 2, get_impl_drawable (drawable=0x0) at gdkgc-win32.c:751 751 in gdkgc-win32.c (gdb) bt
+ Trace 222217
Continuing. Program exited with code 03. (gdb)
(In reply to comment #45) > Breakpoint 2, get_impl_drawable (drawable=0x0) at gdkgc-win32.c:751 > 751 in gdkgc-win32.c > (gdb) bt > If you have the time, try adding a null check after: drawable = gdk_win32_window_start_draw_libgtk_only(win, gc, &x_offset, &y_offset, &clip_x, &clip_y, &ts_x, &ts_y); in xp_theme_draw(): if(!drawable) return FALSE; I will try to get my debugging up when I get home.
Program received signal SIGSEGV, Segmentation fault. _gdk_window_start_draw_helper (drawable=0x1321fa8, gc=0x132b5a0, x_offset_out=0x22e70c, y_offset_out=0x22e708) at gdkwindow.c:3547 3547 gdkwindow.c: No such file or directory. in gdkwindow.c (gdb) bt
+ Trace 222219
Created attachment 162939 [details] [review] gtk+-2.20.1-xptheme-v4.patch Ok, problem was that its not necessarily a GdkWindow that we get in (GtkScrollBars in particular), so the window magic should only be done for GdkWindows. It should also be done though for the code in msw_style.c that draws directly, which fixes the problem I had with the Images demo. Updated patch attached.
Thanks a lot, it seems that this patch is fixing a load of stuff and suddenly, the expose-order patch is not needed for gnumeric either. At least, I was able to work with evolution and I did not get any crash due to the theming.
Created attachment 163037 [details] GtkProgressBar-Overdraw.jpg (In reply to comment #49) > Thanks a lot, it seems that this patch is fixing a load of stuff and suddenly, > the expose-order patch is not needed for gnumeric either. At least, I was able > to work with evolution and I did not get any crash due to the theming. Seems there is still some overdraw issues - I could only reproduce this with Glade3 and GtkProgressBar, as its one of the widgets not using a GdkWindow for drawing - see second example in image.
Created attachment 163044 [details] [review] gtk+-2.20.1-xptheme-v6.patch (In reply to comment #50) > Created an attachment (id=163037) [details] > GtkProgressBar-Overdraw.jpg > > (In reply to comment #49) > > Thanks a lot, it seems that this patch is fixing a load of stuff and suddenly, > > the expose-order patch is not needed for gnumeric either. At least, I was able > > to work with evolution and I did not get any crash due to the theming. > > Seems there is still some overdraw issues - I could only reproduce this with > Glade3 and GtkProgressBar, as its one of the widgets not using a GdkWindow for > drawing - see second example in image. I initially used the same workaround as in comment #6, but only for non GdkWindows added the clipping region to the GC, but as mentioned in comment #32, it overwrites the GC's clip region - so I used _gdk_gc_add_drawable_clip() and _gdk_gc_remove_drawable_clip(), but then its something else to export. I would appreciate it if somebody with some internal knowledge can have a look. I know there is some cleanups to be done (can use get_window_dc(), etc. in xp_theme_draw() as well, etc.), also some comments on all the extra exports and implementation of them would be appreciated. And of course more testing would be appreciated.
I don't think you're exposing this in the right way, far to much internals are exposed. Instead add a new virtual method pair on GdkDrawable that you call before and after your calls. Like gdk_drawable_begin/end_custom_draw(), then implement the magic stuff in the gdkwindow implementation. Don't expose the DRAW_INFO type, just have begin malloc some internal type and pass out a pointer that you'll get back in end. Also, you should not need any (extra) clipping in the non-window case.
Created attachment 163209 [details] [review] gtk+-2.20.1-xptheme-v10.patch (In reply to comment #52) > I don't think you're exposing this in the right way, far to much internals are > exposed. Instead add a new virtual method pair on GdkDrawable that you call > before and after your calls. Like gdk_drawable_begin/end_custom_draw(), then > implement the magic stuff in the gdkwindow implementation. Don't expose the > DRAW_INFO type, just have begin malloc some internal type and pass out a > pointer that you'll get back in end. > Thanks, I started with cleaning it up, and this helped a lot. > Also, you should not need any (extra) clipping in the non-window case. > I won't say (extra) clipping - its just that start_draw_helper() only handle the window cases - with pixmaps it crashes as it uses privates that only windows seem to have. But then I am not a GDK internals expert. Attached is an updated patch: - Cleaned up - Only expose _gdk_drawable_begin_custom_draw() and _gdk_drawable_end_custom_draw() via gdkinternals.h - Currently only the ms-windows engine uses them, so I only exported them for win32 via gdk_win32_begin_custom_draw_libgtk_only() and gdk_win32_end_custom_draw_libgtk_only() - Moved get_window_dc() and release_window_dc() and removed the code duplication between msw_style.c and xp_theme.c Comments welcome.
(In reply to comment #53) > Created an attachment (id=163209) [details] [review] > gtk+-2.20.1-xptheme-v10.patch > Sorry, small mistake due to never getting there during testing - last bit of _gdk_drawable_begin_custom_draw() should be: + if (priv->drawable) + { + *priv_data = (gpointer) priv; + } + else + { + g_free (priv); + return NULL; + } + + return (priv->drawable); instead of just: + if (priv->drawable) + *priv_data = (gpointer) priv; + else + g_free (priv); + + return (priv->drawable); Also, I did not want to touch the original code for start_draw_helper(), but I can merge the GdkPixmap code there rather than using the macro's? Although I did see some functions uses the macro declared variables ...
>> Also, you should not need any (extra) clipping in the non-window case. >> > > I won't say (extra) clipping - its just that start_draw_helper() only handle > the window cases - with pixmaps it crashes as it uses privates that only > windows seem to have. But then I am not a GDK internals expert. What i mean is that this whole issue, i.e. "win32 theme drawing get clipping wrong for non-native windows" just does not apply when drawing to pixmaps. There is no need to take gdk_drawable_get_clip_region() and apply it to the gc at all. The previous code works fine when drawing to pixmaps, the only problem is when drawing on windows (non-native child windows or native windows with non-native children to be exact). So, for non-windows, just return a NULL CustomDrawInfo and ignore it in gdk_drawable_end_custom_draw().
I will test again, but see comment #50(In reply to comment #55) > >> Also, you should not need any (extra) clipping in the non-window case. > >> > > > > I won't say (extra) clipping - its just that start_draw_helper() only handle > > the window cases - with pixmaps it crashes as it uses privates that only > > windows seem to have. But then I am not a GDK internals expert. > > What i mean is that this whole issue, i.e. "win32 theme drawing get clipping > wrong for non-native windows" just does not apply when drawing to pixmaps. > There is no need to take gdk_drawable_get_clip_region() and apply it to the gc > at all. The previous code works fine when drawing to pixmaps, the only problem > is when drawing on windows (non-native child windows or native windows with > non-native children to be exact). > > So, for non-windows, just return a NULL CustomDrawInfo and ignore it in > gdk_drawable_end_custom_draw(). I will test again, but see comment #50 (and the screenshot). Thus far it seems that only GtkProgressBars fall into this catagory - everything else have a GtkWindow and not GtkPixmap as drawable.
Well, the image in comment #50 is obviously a bug, so we need to figure out why it does that. That doesn't mean we should add random incorrect code that seems to fix it though. Anyway, I find it hard to belive the pixmap part changes affected anything. gdk_drawable_get_clip_region() for a pixmap will return a rectangle the size of the pixmap, and clipping to that should be a no-op, since drawing outside the pixmap is automatically clipped anyway.
Also, while GtkProgressBar does draw in a pixmap directly that doesn't mean pixmaps are not used for other widgets. Since the gtk double buffering machinery uses pixmaps almost all drawings are on pixmaps except the final pixmap to window paint.
Created attachment 163271 [details] [review] gtk+-2.20.1-xptheme-v12.patch (In reply to comment #57) > Well, the image in comment #50 is obviously a bug, so we need to figure out why > it does that. > Ok, I debugged it, and it was because for GdkPixmap's the GC already had a clip region set that was smaller than the pixmap, so it drew the background and then only half of the progressbar - thus not "overdrawing", but rather not drawing everything it should. > That doesn't mean we should add random incorrect code that seems > to fix it though. > Point taken. Went to the pixmap source, and there it explicitly clears the clip for the GC before draw actions - and this is what was missing with the ms-windows engine's code when bypassing pixmap drawing functions. Patch attached. I changed the function names to _gdk_drawable_begin_direct_draw() and _gdk_drawable_end_direct_draw() though, as "direct" might be less confusing than "custom".
Review of attachment 163271 [details] [review]: ::: gtk+-2.20.1/gdk/gdkwindow.c @@ +3684,3 @@ + priv->gc = NULL; + priv->type = DRAW_TARGET_UNSUPPORTED; + Just assume that drawable is either a window or a pixmap, and can't be any other type. This will simplify a bit and will be true forever (or huge parts of gdk breaks). @@ +3744,3 @@ +_gdk_drawable_end_direct_draw (gpointer *priv_data) +{ + g_return_if_fail (priv_data != NULL); This assert fails if the window was destroyed and begin_direct_draw returned NULL. just do nothing in this case
Otherwise this looks good to me though.
(In reply to comment #60) > Review of attachment 163271 [details] [review]: > > ::: gtk+-2.20.1/gdk/gdkwindow.c > @@ +3684,3 @@ > + priv->gc = NULL; > + priv->type = DRAW_TARGET_UNSUPPORTED; > + > > Just assume that drawable is either a window or a pixmap, and can't be any > other type. This will simplify a bit and will be true forever (or huge parts of > gdk breaks). > Just remove DRAW_TARGET_UNSUPPORTED and above setting of priv->type, or rather remove the whole enum type and just use a gboolean to test for window? > @@ +3744,3 @@ > +_gdk_drawable_end_direct_draw (gpointer *priv_data) > +{ > + g_return_if_fail (priv_data != NULL); > > This assert fails if the window was destroyed and begin_direct_draw returned > NULL. just do nothing in this case > I would have to disagree here a bit, as if priv_data is NULL, we have an problem. But good catch - can add a check below it: if (*priv_data == NULL) return;
Actually, nevermind about the first question about DRAW_TARGET_UNSUPPORTED - no need for fluff until its needed.
Created attachment 163300 [details] [review] gtk+-2.20.1-xptheme-v13.patch Removed the type logic and only currently allocate priv_data if its a GdkWindow or do any handling in _gdk_drawable_end_direct_draw() for windows.
Created attachment 163302 [details] [review] gtk+-2.20.1-xptheme-v14.patch Sorry for the yet more traffic, but sometimes I only see possible issues that do not appear in testing in hindsight.
Created attachment 163307 [details] [review] gtk+-2.20.1-xptheme-v15.patch Sorry, cleaned up a bit more.
Review of attachment 163307 [details] [review]: Some final nitpicking, but i think this is overall good to go. ::: gtk+-2.20.1/gdk/gdkwindow.c @@ +3680,3 @@ + *y_offset_out = 0; + } + else if (GDK_IS_WINDOW (drawable)) Just make this else and avoid the assertion at the end, this will avoid an unnecessary runtime typecheck. @@ +3695,3 @@ + *y_offset_out = y_offset; + + DirectDrawInfo *priv = g_malloc (sizeof (DirectDrawInfo)); Use g_new(DirectDrawInfo, 1) here @@ +3720,3 @@ + +void +_gdk_drawable_end_direct_draw (gpointer *priv_data) Its imho kinda weird to pass the address to priv_data here, it just adds extra work on both the caller side and inside the function. Its a pointer to it in the begin case because there its an out parameter, but thats not needed here.
(In reply to comment #68) > Review of attachment 163307 [details] [review]: > > Some final nitpicking, but i think this is overall good to go. > Great, thanks for your time. > @@ +3720,3 @@ > + > +void > +_gdk_drawable_end_direct_draw (gpointer *priv_data) > > Its imho kinda weird to pass the address to priv_data here, it just adds extra > work on both the caller side and inside the function. > > Its a pointer to it in the begin case because there its an out parameter, but > thats not needed here. > I did it to automatically free the pointer, but can change it - the caller will then just have to free it again I guess?
Err, Ok, I can still free it, but just won't be able to set it to NULL - but then code should not call it twice. I am just not sure on protocol here - leave the free, or require caller to do it?
Created attachment 163325 [details] [review] gtk+-2.20.1-xptheme-v16.patch Ok, went for freeing it, as its an private structure. Question though - did some casual testing and getting these with MS-Windows theme, but not Raleigh: (gtk-demo.exe:2836): GLib-GObject-WARNING **: invalid cast from `GdkOffscreenWindow' to `GdkDrawableImplWin32' Everything works fine for the "Offscreen windows->Effects" demo, but would appreciate some imput, as I assume it is some typing/inheritance issue, but not too familiar with it. I can debug it tomorrow, but this build have some issues do to my scripts doing resource hacking to get proper versions for all executables which causes gdb to crash =/
(In reply to comment #71) > Question though - did some casual testing and getting these with MS-Windows > theme, but not Raleigh: > > (gtk-demo.exe:2836): GLib-GObject-WARNING **: invalid cast from > `GdkOffscreenWindow' to `GdkDrawableImplWin32' > > Everything works fine for the "Offscreen windows->Effects" demo, but would > appreciate some imput, as I assume it is some typing/inheritance issue, but not > too familiar with it. > > I can debug it tomorrow, but this build have some issues do to my scripts doing > resource hacking to get proper versions for all executables which causes gdb to > crash =/ > Added some debug printing, and its happening outside the get_window_dc() and release_window_dc() calls ... will investigate further tomorrow.
I'm not sure what happens, but i can give some background. GdkWindow is a public class inheriting from GdkDrawable, and all windows (offscreen or normal) are this type. However, the GdkWindow (and GdkPixmap works this way too) object has a pointer to an "impl" type which is of another type, also inheriting from drawable which is used to implement the backend specific side of the rendering. So, the impl object is a GdkDrawableImplWin32 on win32 and GdkWindowImplX11 on X. Offscreen windows are implemented as a normal GdkWindow of window_type GDK_WINDOW_OFFSCREEN, but its impl is a platform independent type called GdkOffscreenWindow rather than the backend implementation type. This type implements drawing by allocating a pixmap and doing all drawing on that (which in turn will call the backend-native rendering on the pixmap). This is not totally transparent to the backend code, sometimes it needs to handle that the windows it gets are offscreen windows and do something different that it used to do. Apparently the theme code needs to do something like this in at least one place.
Created attachment 163394 [details] [review] gtk+-2.20.1-gdkwin32-offscreen.patch Problem is with the win32 backend - GDK_DRAWABLE_HANDLE() does not handle offscreen windows. I tried to fix it at first with GDK_WINDOW_TYPE() as done in gdkwindow-x11.c, but it did not evaluate to true for all cases. Let me know if there is a better way to fix this, and if it should be part of the main patch, although I'd think its a different issue just exposed here.
First of all, check window_type for GDK_WINDOW_OFFSCREEN rather than dynamic type checks, secondly i think you should be able to get a handle for the offscreen too, using gdk_offscreen_window_get_pixmap(). I didn't look at when/how GDK_DRAWABLE_HANDLE() is used though, so don't know if this is useful.
Also, i would add the offscreen check to GDK_WINDOW_HWND rather than in GDK_DRAWABLE_HANDLE as that fixes more users.
Actually, thinking some more, its probably a bad idea to add offscreen support to these low level primitives. They're expected to be fast, and its not common to have offscreen windows enter into the backend specific code (typically e.g. draws on an offscreen surface will be caught by the common code and will eventually enter the backend as a draw operation on a pixmap instead). The way the X backend handles this is that it catches offscreen windows at an early state in the functions where its needed rather than continuing with them as normal windows. So, rather than making GDK_DRAWABLE_HANDLE handle offscreens, figure out why it was called on an offscreen and trace back to where the best entrypoint is to handle it. How does the backtrace look where its called? The other patch looks good though, so i'm applying that on master. If it seems ok we can cherry pick it to stable later.
Comment on attachment 163325 [details] [review] gtk+-2.20.1-xptheme-v16.patch Commited the xp theme patch to master. I had to move the .def changes to gdk.symbols because that is an autogenerated file not in git. I wasn't able to try this, so please someone verify that it works.
(In reply to comment #75) > First of all, check window_type for GDK_WINDOW_OFFSCREEN rather than dynamic > type checks, secondly i think you should be able to get a handle for the > offscreen too, using gdk_offscreen_window_get_pixmap(). I didn't look at > when/how GDK_DRAWABLE_HANDLE() is used though, so don't know if this is useful. I am not sure if it's a win32 backend problem, or more a core gdkwindow one, but using window_type just do not work: ------------------------ #define GDK_TYPE_OFFSCREEN_WINDOW (gdk_offscreen_window_get_type()) #define GDK_WINDOW_OBJ(win) ((GdkWindowObject *)win) #define GDK_WINDOW_IMPL(win) ((GdkWindowObject *)((GdkWindowObject *)win)->impl) /* Works */ #define GDK_IS_OFFSCREEN_WINDOW1(object) (G_TYPE_CHECK_INSTANCE_TYPE ((((GdkWindowObject *)object)->impl), GDK_TYPE_OFFSCREEN_WINDOW)) /* Do not always detect offscreen */ #define GDK_IS_OFFSCREEN_WINDOW2(object) (GDK_WINDOW_OBJ (object)->window_type == GDK_WINDOW_OFFSCREEN) /* Do not always detect offscreen */ #define GDK_IS_OFFSCREEN_WINDOW3(object) (GDK_WINDOW_IMPL (object)->window_type == GDK_WINDOW_OFFSCREEN) #undef GDK_ROOT_PARENT /* internal access is direct */ #define GDK_ROOT_PARENT() ((GdkWindow *) _gdk_parent_root) #define GDK_PIXMAP_HBITMAP(pixmap) (GDK_DRAWABLE_IMPL_WIN32(((GdkPixmapObject *)pixmap)->impl)->handle) #define GDK_WINDOW_WIN32_HWND(win) (GDK_DRAWABLE_IMPL_WIN32(((GdkWindowObject *)win)->impl)->handle) #define GDK_WINDOW_OFFSCREEN_HWND(win) (GDK_PIXMAP_HBITMAP (gdk_offscreen_window_get_pixmap (win))) #define GDK_WINDOW_HWND(win) (GDK_IS_OFFSCREEN_WINDOW1 (win) ? GDK_WINDOW_OFFSCREEN_HWND (win) : GDK_WINDOW_WIN32_HWND (win)) #define GDK_DRAWABLE_IMPL_WIN32_HANDLE(d) (((GdkDrawableImplWin32 *) d)->handle) #define GDK_DRAWABLE_HANDLE(win) (GDK_IS_WINDOW (win) ? GDK_WINDOW_HWND (win) : (GDK_IS_PIXMAP (win) ? GDK_PIXMAP_HBITMAP (win) : (GDK_IS_DRAWABLE_IMPL_WIN32 (win) ? GDK_DRAWABLE_IMPL_WIN32_HANDLE (win) : 0))) ----------------------------- (In reply to comment #77) > Actually, thinking some more, its probably a bad idea to add offscreen support > to these low level primitives. They're expected to be fast, and its not common > to have offscreen windows enter into the backend specific code (typically e.g. > draws on an offscreen surface will be caught by the common code and will > eventually enter the backend as a draw operation on a pixmap instead). > I do not have a problem to change things - after all I usually only use gtk2-perl =) > The way the X backend handles this is that it catches offscreen windows at an > early state in the functions where its needed rather than continuing with them > as normal windows. So, rather than making GDK_DRAWABLE_HANDLE handle > offscreens, figure out why it was called on an offscreen and trace back to > where the best entrypoint is to handle it. How does the backtrace look where > its called? > This is easy - its when the theming engine applies the theme to the offscreen, the why though is a different matter ... : --------------------------- @@ -871,26 +911,16 @@ xp_theme_draw (GdkWindow *win, XpThemeEl /* FIXME: Recheck its function */ enable_theme_dialog_texture_func (GDK_WINDOW_HWND (win), ETDT_ENABLETAB); - if (!GDK_IS_WINDOW (win)) --------------------------- The function parameters might be a bit dubious, as it can be any drawable, and not only a window. But anyhow, the whole cause is from the call to enable_theme_dialog_texture_func() which according to the comment of the original author is not known if needed or not, or why. Further, I do not know if this could be an issue anywhere else, because I do not know of any other code out there that uses the HWND or even in the backend itself (probably should have a look at who maintains the backend to get more info) ... so it could be easier just to call above if its not an offscreen window, but ... > The other patch looks good though, so i'm applying that on master. If it seems > ok we can cherry pick it to stable later. Thanks, with current job I have not done this for some time, so forgot how intense it can be, so glad it only took a few comments and two reviews =)
If I filter offscreen windows in xp_theme.c, the theming is not applied btw.
(In reply to comment #78) > (From update of attachment 163325 [details] [review]) > Commited the xp theme patch to master. I had to move the .def changes to > gdk.symbols because that is an autogenerated file not in git. > > I wasn't able to try this, so please someone verify that it works. If not tonight, I will do tomorrow, thanks.
(In reply to comment #80) > If I filter offscreen windows in xp_theme.c, the theming is not applied btw. This is globally btw., not just for: /* FIXME: Recheck its function */ enable_theme_dialog_texture_func (GDK_WINDOW_HWND (win), ETDT_ENABLETAB);
The committed patch broke the build, I fixed it with http://git.gnome.org/browse/gtk+/commit/?id=230f47224b379bf383faf684f591926a297025c5 but I did not try it on win32
(In reply to comment #83) > The committed patch broke the build, I fixed it with > > http://git.gnome.org/browse/gtk+/commit/?id=230f47224b379bf383faf684f591926a297025c5 > > but I did not try it on win32 With above fix the .def is generated properly - I am still struggling to get a full build here, as this box is a bit slow, and I hacked out the glib-2.25.8 dep, but need to build it as well to test fully ...
I see. The reason you can't use ->window_type like that is that you're getting a subwindow which is of type child, but getting its impl window which is really the same as getting the impl window of the nearest native window. That one is of type offscreen, but you can't easily see that from the child window. So, what i think is the real solution here is to do: if (GDK_IS_WINDOW(win) && GDK_WINDOW_IS_WIN32(win)) enable_theme_dialog_texture_func (GDK_WINDOW_HWND (win), ETDT_ENABLETAB); Can you try that?
Created attachment 163575 [details] [review] gtk+-2.20.1-gdkwin32-offscreen-v2.patch (In reply to comment #85) > I see. The reason you can't use ->window_type like that is that you're getting > a subwindow which is of type child, but getting its impl window which is really > the same as getting the impl window of the nearest native window. That one is > of type offscreen, but you can't easily see that from the child window. > > So, what i think is the real solution here is to do: > > if (GDK_IS_WINDOW(win) && GDK_WINDOW_IS_WIN32(win)) > enable_theme_dialog_texture_func (GDK_WINDOW_HWND (win), ETDT_ENABLETAB); > > Can you try that? Yes, that works, but its once again an internal =) Followed the GDK_WINDOW_HWND() route - hope that is the correct way.
I don't see the reason to export it as an macro like that. Just make the xp_theme code call the gdk_win32_window_is_win32() function as a normal function and skip the macro part. Also, i'm not sure if its a good idea to merge the two checks like that, they really do two things, the is_win32 check is a new one and the is_window is really a fix for the old code that wouldn't quite work when passed in pixmap drawables.
Created attachment 163597 [details] [review] gtk+-2.20.1-gdkwin32-offscreen-v3.patch (In reply to comment #87) > I don't see the reason to export it as an macro like that. Just make the > xp_theme code call the gdk_win32_window_is_win32() function as a normal > function and skip the macro part. It was to keep the link between the internal macro and the 'exported' one intact, and I guess because I looked at what GDK_WINDOW_HWND() did. Changed. > Also, i'm not sure if its a good idea to > merge the two checks like that, they really do two things, the is_win32 check > is a new one and the is_window is really a fix for the old code that wouldn't > quite work when passed in pixmap drawables. Fixed. Built and tested.
commited
does that mean xp theming will work in gtk 2.22? either way, martin and alex, many thanks for the work you've done on this already.
its commited to master which is the basis for gtk 3.0 However, if it seems to work fine it can be backported to 2.x easily.
(In reply to comment #91) > its commited to master which is the basis for gtk 3.0 > > However, if it seems to work fine it can be backported to 2.x easily. That would be good. I would appreciate it if the patch was backported. Otherwise, for example, apps written in PyGtk will be unable to run with the XP theme.
One solution for you might be to take this cummulated patch against 2.20.1 and rebuild gtk+ https://build.opensuse.org/package/view_file?file=gtk%2B-2.20.1-xptheme.patch&package=mingw32-gtk2&project=windows%3Amingw%3Awin32 My packages in http://download.opensuse.org/repositories/windows:/mingw:/win32/ and http://download.opensuse.org/repositories/windows:/mingw:/win64/ have it applied.
Any updates for backport the patch to 2.x? It seems work on 2.20
Created attachment 166447 [details] MenuSeparator.jpg Hijacking the bug if nobody mind, as I think its a continued enhancement of the previous patches. Can make a new bug if needed. The engine do not handle menu separators properly.
Created attachment 166449 [details] [review] gtk+-2.20.1-xptheme-cleanup-and-fixes.patch - As stated above, menu separators are not drawn, so fixed this. - Also removed the depreciated code for getting the font info from a LOGFONT, as gtk+-2.20 depends on pango-1.20, which already have pango_win32_font_description_from_logfontw(). - Other compiler warning cleanups. This more for general review - I can split the patches if everything seems fine, or if it will be better to split them from the beginning, let me know.
Just to say that ekiga depends on gtk 2.18 now and for windows it uses http://gtk-win.sourceforge.net; however, its author does not package gtk for windows because of this bug...
Martin, just for record: the headers that I use (mingw-w64 and the windows SDK 7.0) don't define THEME_SIZE, but THEMESIZE.
(In reply to comment #98) > Martin, just for record: the headers that I use (mingw-w64 and the windows SDK > 7.0) don't define THEME_SIZE, but THEMESIZE. Yeah, its an inconsistency in the MSDN docs between the function proto and the enum :( Did it like the rest to not need uxtheme.h.
(In reply to comment #97) > Just to say that ekiga depends on gtk 2.18 now and for windows it uses > http://gtk-win.sourceforge.net; however, its author does not package gtk for > windows because of this bug... The owner of gtk-win.sf.net can just add the patch, or if he do not build it himself, he could have asked somebody here if they had a build that included the patch, or use the binaries that includes it already. Not being in the official tarball have nothing to do with anything - that is the beauty of Open Source - you can add or fix features if need be. Please complain there and not here about vendors of binary packages if they are not willing to go the extra mile to appease their user base. Thanks!
Created attachment 166574 [details] [review] gtk+-2.20.1-xptheme-warnings.patch Fix some warnings related to type differences and unused variables.
Created attachment 166575 [details] [review] gtk+-2.20.1-xptheme-use-pango.patch Use pango_win32_font_description_from_logfontw() to get the system font, instead of the code duplication.
Created attachment 166576 [details] [review] gtk+-2.20.1-xptheme-menu-separator.patch Properly draw the menu separator for Windows Vista/7 - see attachment #166447 [details]. Also added the bits for transparency, but cannot find a way to test it for now.
Created attachment 166586 [details] [review] gtk+-2.20.1-xptheme-use-pango-v2.patch Somewhere I switched when I should free the pango font description around - fixed.
Martin, this bunch of patches compiles well with both mingw-w64 headers and with mingw.org headers. Thanks a lot. Let me now test that it actually even works :)
Tested it on Windows XP/Vista/7 (only 32bit), but thanks, more eyes the better :)
so, what patches do we need to apply to gtk 2.20 to get xp theming work? won't these be committed anytime?
Will someone on this bug look at backporting the committed fixes to 2.22 ? that would be great. I'm not in a position to work on win32 fixes, unfortunately...
Backporting is not such a big deal. All the theming patches here https://build.opensuse.org/package/files?package=mingw32-gtk2&project=windows%3Amingw%3Awin32 are de-fuzzed and made apply without problem with the 2.22 branch. I disabled them nevertheless just yesterday because I wanted to isolate a problem I see with patched 2.22 branch and with master where in the icon browser demo icons appear and disappear if you hover with mouse over them (from up to down they appear, from down to up they disappear). But apart this little tiny :) snafus, the patched gtk+ works well.
The biggest xptheme patch is in fact a backported cummulated patch from the xp theme fixes that went into master. It is not creating any regression that would not be in master (at least in the last version of the libwimp.dll that was buildable on windows). If it is ok to commit, I can do that. Just advise.
(In reply to comment #109) > Backporting is not such a big deal. All the theming patches here > https://build.opensuse.org/package/files?package=mingw32-gtk2&project=windows%3Amingw%3Awin32 > are de-fuzzed and made apply without problem with the 2.22 branch. I disabled > them nevertheless just yesterday because I wanted to isolate a problem I see > with patched 2.22 branch and with master where in the icon browser demo icons > appear and disappear if you hover with mouse over them (from up to down they > appear, from down to up they disappear). But apart this little tiny :) snafus, > the patched gtk+ works well. Disabling them makes it work? How about just applying the first one if so? Which Windows version btw.? I will have a look if its definitely caused by the patches. Also, I slipped into gtk+-2.20.1-xptheme-menu-separator.patch: --------- + /* Support transparency */ + if (is_theme_partially_transparent_func (theme, element_part_map[element], part_state)) + draw_theme_parent_background_func (GDK_WINDOW_HWND (win), dc, pClip); + ----------------------- which might have side effects, and probably should be splitted out. PS: Matthias, who can I ask to have a look at the last three patches I added?
Martin, I cannot tell you whether it works or no because I just upgraded to the 2.21.90 gnome and my VMware does suddenly not start. I did not have time to find out why and what to do :( Nevertheless, the icon browser thingie is happening also in master without any patch added. In 2.90.5 I mean, because in 2.90.6 the windows theme does not compile since it needs to be ported to cairo.
Fridrich: applying the patch to the source may not be a big deal, but building for windows isn't too easy. that's why it would be great to have the patches committed to gtk2 head, so that when the next builds come out with gtk 2.22 on gtk.org, we can use them w/o having libwimp disabled.
(In reply to comment #111) > Disabling them makes it work? How about just applying the first one if so? > Which Windows version btw.? Martin: the behaviour is not due to the xp theming patches. It is the same with the patches applied as it is without them. And with the patches, the theming works just ok. If I had the powers, I would declare the patches (at least the main one) safe for 2.22 branch.
Fridrich, feel free to commit the ms-windows theme engine patches to the 2.22 branch. Maybe also 2.20?
(In reply to comment #115) > Fridrich, feel free to commit the ms-windows theme engine patches to the 2.22 > branch. Maybe also 2.20? As you desire, master :), I committed to both gtk-2-20 and gtk-2-22 branches the main theming patch that is included in master. I refrained from committing the other patches here, since the authorization was a bit ambiguous. Please advise whether one should push them too. I can do so for all master, gtk-2-22 and gtk-2-20.
After discussion with Tor, the remaining three patches don't affect anything outside the ms-windows theme engine. We did not see any regression wrt. the current status. So I pushed them to master and to gtk-2-22 branch after 2.21.7 was tagged. We refrained from pushing to gtk-2-20 branch though, although the cherry-pick should be trivial. Should we close this bug?
sure. i'll try it when 2.22 comes out, and if there's a problem with it, i can reopen it (i guess). probably others will do the same. many thanks to everyone who participated.
Martin: Since all the GdkGC stuff went in master and since the cairo rendering is already in gtk-2-22 branch, I almost ported the engine to use cairo which would make it buildable with in master too. Nonetheless, I need some help with porting the two get_window_dc and release_window_dc because they have some complicated part in gdk/gdkwindow.c that is not present in master now. According to experts, though, cairo does the clipping well so maybe one could port only what was in 2-18? But that is too much for my current understanding to decide.
(In reply to comment #119) > Martin: Since all the GdkGC stuff went in master and since the cairo rendering > is already in gtk-2-22 branch, I almost ported the engine to use cairo which > would make it buildable with in master too. Nonetheless, I need some help with > porting the two get_window_dc and release_window_dc because they have some > complicated part in gdk/gdkwindow.c that is not present in master now. > According to experts, though, cairo does the clipping well so maybe one could > port only what was in 2-18? But that is too much for my current understanding > to decide. The main problem was because it did the draws to the backing (DC) directly, thus ignoring clipping that might have been applied implementation specific. I am not sure about how the cairo changes affects things - a quick look seems that they dropped the voodoo for gdkwindow.c. I will need to have a look and get back to you.
(In reply to comment #120) > The main problem was because it did the draws to the backing (DC) directly, > thus ignoring clipping that might have been applied implementation specific. I committed and pushed an utterly broken version that nevertheless allows the engine to build in master. When one tries to use the engine though, nothing crashes but some of the parts are drawn in wrong position and are wrongly clipped. I believe, nevertheless, that one can somehow start there. > I am not sure about how the cairo changes affects things - a quick look seems > that they dropped the voodoo for gdkwindow.c. The commit that I did has the old code in #if 0 block and the new code in #else block so that one can still have in front of one's eyes what did it use look like. > I will need to have a look and get back to you. It will be heavily appreciated. You do whatever your time allows you. I will also try to understand the underlying voodoo (as you call it rightly). But knowing the capacity of my brain, however busy you might be, you will have fixed it before I even start to have a ray of light :)
Created attachment 169441 [details] [review] master_ms-windows.patch Something like attached. I cannot however test it, as master's win32 backend (or something else) seems to be fairly broken, as not even the pixmap theme render correctly. Or at least for me - just for the demo the scrolledwindow do the wrong thing, with the same rendering issues originally where some components are not drawn until you hover over them, or not at all. Not sure if I should use some other branch. I will look at it again when I get the time, but currently that will mean looking at the backend first, and then the theming code.
Actually thinking about it more, its probably not going to work as well because of expose order, etc. - causing overdraws again. Will probably need to let the theming stuff draw to some temp dc and then draw it from there with cairo. Is there anything to read to clarify the new drawable interface and how it and using cairo influences things?
(In reply to comment #122) > Created an attachment (id=169441) [details] [review] > master_ms-windows.patch > Something like attached. I cannot however test it, as master's win32 backend > (or something else) seems to be fairly broken, as not even the pixmap theme > render correctly. I committed this one as a work in progress, because it is drawing the elements in their correct position. The problem with elements that are not drawn until you hover over them is still there. But it is definitely better then the state before.
Review of attachment 166576 [details] [review]: committed
Review of attachment 166574 [details] [review]: committed
Review of attachment 166586 [details] [review]: committed
Review of attachment 169441 [details] [review]: much better then current status and does not influence anything other
(In reply to comment #123) > Actually thinking about it more, its probably not going to work as well because > of expose order, etc. - causing overdraws again. Will probably need to let the > theming stuff draw to some temp dc and then draw it from there with cairo. When running the gtk3-demo.exe I get a zillion of: (gtk3-demo.exe:1616): Gdk-WARNING **: gdkwindow-win32.c:3455: BitBlt failed: The handle is invalid. (gtk3-demo.exe:1616): Gdk-WARNING **: gdkdrawable-win32.c:219: ReleaseDC failed: The handle is invalid. > Is there anything to read to clarify the new drawable interface and how it and > using cairo influences things? I basically found this thing as useful http://blogs.gnome.org/otte/2010/07/27/rendering-cleanup/ The author (nick Company) is normally at #gtk+ irc at about 11am UTC and is more then helpful when one knows what to actually ask.
(In reply to comment #130) > (In reply to comment #123) > > Actually thinking about it more, its probably not going to work as well because > > of expose order, etc. - causing overdraws again. Will probably need to let the > > theming stuff draw to some temp dc and then draw it from there with cairo. > > When running the gtk3-demo.exe I get a zillion of: > (gtk3-demo.exe:1616): Gdk-WARNING **: gdkwindow-win32.c:3455: BitBlt failed: > The handle is invalid. That is _gdk_win32_window_translate() in gdkwindow-win32.c, which as far as I could see on Friday is used for things like scrolling a scrolledwindow - instead of rendering the whole unchanged part that should be drawn up/down, it just copies the needed part up or down - or that is what it should do, but it seems to do things inverse (when I accuire the dc via cairo), and I looked at the X11 code, which is way more complex, using a scratch gc, taking other/child windows into account, etc. Main problem here is that the GdkWindow passed do not seem to be a win32 implemented on when the call is made to _gdk_win32_drawable_acquire_dc(), so the hdc returned is invalid, causing BitBlt to fail. Not sure what type of drawable implementations are passed, and/or if _gdk_win32_drawable_acquire_dc() should still be updated for the changes. > (gtk3-demo.exe:1616): Gdk-WARNING **: gdkdrawable-win32.c:219: ReleaseDC > failed: The handle is invalid. This could be due to me freeing the dc aquired from cairo - there is no documentation if it should be freed or not - should have put a not there about verifying if it should be done ... > > Is there anything to read to clarify the new drawable interface and how it and > > using cairo influences things? > > I basically found this thing as useful > http://blogs.gnome.org/otte/2010/07/27/rendering-cleanup/ > The author (nick Company) is normally at #gtk+ irc at about 11am UTC and is > more then helpful when one knows what to actually ask. Thanks, will have a look, but until the end of the month I am fairly piled under at work, so cannot promise when.
Just one minor nit about the code: The "Xp" prefix is a bit confusing, I initially thought those were some Microsoft types... but I guess once you read the code a bit more you notice they are defined in the code.
Created attachment 171332 [details] progress bar on xp
Created attachment 171333 [details] progress bar on 7
reporting that xp theming seems to work with 2.22. one minor glitch is that progress bar rendering error for xp which also occured with gtk 2.16. it's used in xchat, i'm using it in xchat-wdk. the corresponding code can be found here: http://xchat.svn.sourceforge.net/viewvc/xchat/src/fe-gtk/fe-gtk.c?view=markup search for "lagometer"
(In reply to comment #132) > Just one minor nit about the code: The "Xp" prefix is a bit confusing, I > initially thought those were some Microsoft types... but I guess once you read > the code a bit more you notice they are defined in the code. Its inherited from the original code .. guess "Ux" might be more appropriate? (In reply to comment #135) > reporting that xp theming seems to work with 2.22. > > one minor glitch is that progress bar rendering error for xp which also occured > with gtk 2.16. > > it's used in xchat, i'm using it in xchat-wdk. the corresponding code can be > found here: > > http://xchat.svn.sourceforge.net/viewvc/xchat/src/fe-gtk/fe-gtk.c?view=markup > > search for "lagometer" This is a rendering bug with Windows XP, and not GTK. It manifests as soon as the progressbar have a height that is less than 10 (tested with a VC++ project - can attach it if need be, but its very quick and dirty). So just change the height of the progressbar's to be at least 10 (I am guessing you have 8 or something currently).
many thanks Martin. it was indeed set to 8, now it works fine with 10.
Created attachment 172343 [details] [review] gtk+-2.91.0-ms-windows.patch Stab at fixing building for GTK3/master. Note, its a big WIP, and my cairo and/or new window/drawing backend knowledge seems to be lacking a bit. Basically everything is now based on cairo, but it does not seem that the target of the CR we are getting have a real HDC as backing, as cairo_win32_surface_get_dc() fails. So I create a temp HDC based on a BITMAP backing, copy whatever is on the CR's target over, give that HDC back, and then in the end copy whatever was changed back to the CR. There are issues though, and here I am at a bit of a loss at the moment - it seems there is not always anything currently on the target of the CR, or something, even if its the highlight overdraw for a toolbutton or menu. Anyhow, might have a look at it again in a while if I get the chance, but thought somebody else might have some insight, or could use it to start from, as some grunt work has been done.
Review of attachment 172343 [details] [review]: I committed this to the master in a state that resulted from me rebasing it from 2.91.0. Will try to see whether it compiles :)
No new updates to this bug report for quite some time now. Is this bug finally safe to close?
I would say it is safe. There are some cosmetic issues with rendering Tabs that are not Top aligned (or take up all the width to the right side on Win7 at least), Tab label (or "tab child) alignment, incorrect GTK_STATE_* for popups that uses GtkCellRenders, and some artifacts (offscreen in particular), but I think that can be added as new bugs if I or somebody else get time to fix them. The last time I tried to fix the Tab issues it was quite evasive (had to fiddle with new style properties like "tab-label-offset" and "shadow-width/height", size request/allocation, etc. which could cause regressions for non-win32 platforms. The others (like the popups) are easier and I'll submit patches when I get some free time.
Closing bug. Remaining issues to be tracked in fresh bug reports.