After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 598299 - XP theming in the ms-windows theme engine does not work
XP theming in the ms-windows theme engine does not work
Status: RESOLVED FIXED
Product: gtk+
Classification: Platform
Component: Backend: Win32
2.18.x
Other Windows
: Normal normal
: ---
Assigned To: gtk-win32 maintainers
gtk-bugs
csw
Depends on:
Blocks:
 
 
Reported: 2009-10-13 15:48 UTC by Tor Lillqvist
Modified: 2013-10-08 21:08 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Partial fix to suggest what the issue is (1.54 KB, patch)
2009-11-18 20:16 UTC, John Ehresman
none Details | Review
rendering-error.jpg (29.08 KB, image/jpeg)
2010-04-15 12:40 UTC, Martin Schlemmer
  Details
gtk+-2.20.1-xptheme.patch (14.59 KB, patch)
2010-06-02 16:49 UTC, Martin Schlemmer
none Details | Review
gtk+-2.20.1-xptheme.patch (14.02 KB, patch)
2010-06-02 16:54 UTC, Martin Schlemmer
none Details | Review
gtk+-2.20.1-xptheme-v3.patch (14.25 KB, patch)
2010-06-03 10:12 UTC, Martin Schlemmer
none Details | Review
gtk+-2.20.1-xptheme-v4.patch (28.91 KB, patch)
2010-06-07 14:53 UTC, Martin Schlemmer
none Details | Review
GtkProgressBar-Overdraw.jpg (35.88 KB, image/jpeg)
2010-06-08 10:19 UTC, Martin Schlemmer
  Details
gtk+-2.20.1-xptheme-v6.patch (31.46 KB, patch)
2010-06-08 11:31 UTC, Martin Schlemmer
none Details | Review
gtk+-2.20.1-xptheme-v10.patch (24.21 KB, patch)
2010-06-09 16:26 UTC, Martin Schlemmer
none Details | Review
gtk+-2.20.1-xptheme-v12.patch (23.88 KB, patch)
2010-06-10 10:58 UTC, Martin Schlemmer
none Details | Review
gtk+-2.20.1-xptheme-v13.patch (23.54 KB, patch)
2010-06-10 15:57 UTC, Martin Schlemmer
none Details | Review
gtk+-2.20.1-xptheme-v14.patch (23.59 KB, patch)
2010-06-10 16:03 UTC, Martin Schlemmer
none Details | Review
gtk+-2.20.1-xptheme-v15.patch (23.58 KB, patch)
2010-06-10 16:55 UTC, Martin Schlemmer
none Details | Review
gtk+-2.20.1-xptheme-v16.patch (23.41 KB, patch)
2010-06-10 20:32 UTC, Martin Schlemmer
committed Details | Review
gtk+-2.20.1-gdkwin32-offscreen.patch (1.52 KB, patch)
2010-06-11 15:01 UTC, Martin Schlemmer
none Details | Review
gtk+-2.20.1-gdkwin32-offscreen-v2.patch (2.60 KB, patch)
2010-06-14 11:49 UTC, Martin Schlemmer
none Details | Review
gtk+-2.20.1-gdkwin32-offscreen-v3.patch (2.17 KB, patch)
2010-06-14 15:07 UTC, Martin Schlemmer
none Details | Review
MenuSeparator.jpg (20.01 KB, image/jpeg)
2010-07-23 14:11 UTC, Martin Schlemmer
  Details
gtk+-2.20.1-xptheme-cleanup-and-fixes.patch (19.61 KB, patch)
2010-07-23 14:15 UTC, Martin Schlemmer
none Details | Review
gtk+-2.20.1-xptheme-warnings.patch (4.65 KB, patch)
2010-07-26 12:00 UTC, Martin Schlemmer
committed Details | Review
gtk+-2.20.1-xptheme-use-pango.patch (9.92 KB, patch)
2010-07-26 12:01 UTC, Martin Schlemmer
none Details | Review
gtk+-2.20.1-xptheme-menu-separator.patch (6.52 KB, patch)
2010-07-26 12:03 UTC, Martin Schlemmer
committed Details | Review
gtk+-2.20.1-xptheme-use-pango-v2.patch (9.93 KB, patch)
2010-07-26 14:16 UTC, Martin Schlemmer
committed Details | Review
master_ms-windows.patch (2.95 KB, patch)
2010-09-03 16:31 UTC, Martin Schlemmer
committed Details | Review
progress bar on xp (2.57 KB, image/png)
2010-09-29 12:23 UTC, viktor
  Details
progress bar on 7 (2.67 KB, image/png)
2010-09-29 12:24 UTC, viktor
  Details
gtk+-2.91.0-ms-windows.patch (55.87 KB, patch)
2010-10-14 11:40 UTC, Martin Schlemmer
committed Details | Review

Description Tor Lillqvist 2009-10-13 15:48:06 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.
Comment 1 Dominic Lachowicz 2009-10-13 15:55:28 UTC
Tor - what happens if you make xp_theme_is_active return "FALSE"? Do you get the "Win98" look?
Comment 2 Tor Lillqvist 2009-10-13 17:06:13 UTC
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?
Comment 3 Dominic Lachowicz 2009-10-13 17:29:19 UTC
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.
Comment 4 Tor Lillqvist 2009-10-13 18:36:14 UTC
OK, pushed that to master and gtk-2-18. Keeping this bug open.
Comment 5 Tor Lillqvist 2009-10-14 09:52:55 UTC
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"?
Comment 6 John Ehresman 2009-11-18 20:16:12 UTC
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?
Comment 7 Neustradamus 2010-01-17 19:48:23 UTC
Have you news about this bug ?
Comment 8 Tor Lillqvist 2010-01-18 09:40:53 UTC
Do you see any news here?
Comment 9 Neustradamus 2010-01-18 16:55:12 UTC
I would like use GTK+ 2.18.x on MS Windows because 2.16.x is very old.
Comment 10 Dominic Lachowicz 2010-01-18 16:58:27 UTC
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.
Comment 11 Tor Lillqvist 2010-01-19 01:09:17 UTC
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?
Comment 12 Rich 2010-02-19 19:37:52 UTC
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?
Comment 13 Tor Lillqvist 2010-02-19 20:23:45 UTC
> What are some potential workarounds for me?

Just "doing hard work and fixing the bug".
Comment 14 Peter Clifton 2010-02-19 20:48:51 UTC
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.
Comment 15 Tor Lillqvist 2010-02-19 21:01:37 UTC
> 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".
Comment 16 Peter Clifton 2010-02-19 21:07:14 UTC
(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.
Comment 17 Rich 2010-02-19 21:08:45 UTC
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.
Comment 18 Tor Lillqvist 2010-02-19 21:25:04 UTC
OK, so Qt won, yay!
Comment 19 Ignacio Casal Quinteiro (nacho) 2010-02-19 21:31:18 UTC
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.
Comment 20 Bert Broekhuizen 2010-02-22 14:37:18 UTC
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;
}
Comment 21 Bert Broekhuizen 2010-02-23 06:58:35 UTC
(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.
Comment 23 Martin Schlemmer 2010-04-14 09:39:26 UTC
(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.
Comment 24 Tor Lillqvist 2010-04-14 09:44:21 UTC
As that patch touches only cross-platform code, it would of course be essential to test it on X11 (and Mac OS X?), too.
Comment 25 Martin Schlemmer 2010-04-14 09:47:29 UTC
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.
Comment 26 Martin Schlemmer 2010-04-15 12:40:25 UTC
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.
Comment 27 Erik van Pienbroek 2010-04-15 12:50:10 UTC
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
Comment 28 John Ehresman 2010-04-15 15:08:03 UTC
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)?
Comment 29 Andreas J. Guelzow 2010-04-23 16:55:24 UTC
Erik, reagarding your comment #27 you may want to look at the screenshot attached to bug #616207.
Comment 30 Martin Schlemmer 2010-04-29 15:10:50 UTC
(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.
Comment 31 Javier Jardón (IRC: jjardon) 2010-05-25 19:38:10 UTC
Alex, is the patch of comment #23 a valid approach?
Comment 32 Alexander Larsson 2010-05-26 08:53:18 UTC
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.
Comment 33 viktor 2010-05-29 16:08:19 UTC
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?
Comment 34 Tor Lillqvist 2010-06-01 09:00:52 UTC
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.
Comment 35 viktor 2010-06-01 12:52:33 UTC
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.
Comment 36 Tor Lillqvist 2010-06-01 13:05:48 UTC
you can't expect anything
Comment 37 Morten Welinder 2010-06-02 00:46:58 UTC
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.
Comment 38 Martin Schlemmer 2010-06-02 16:49:51 UTC
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.
Comment 39 Martin Schlemmer 2010-06-02 16:54:01 UTC
Created attachment 162554 [details] [review]
gtk+-2.20.1-xptheme.patch
Comment 40 Martin Schlemmer 2010-06-02 16:55:37 UTC
Sorry for the extra traffic - first version had some extra stuff from before cleaning it up a bit.
Comment 41 Martin Schlemmer 2010-06-03 10:12:22 UTC
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.
Comment 42 Fridrich Strba 2010-06-03 12:21:53 UTC
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
  • #0 ntdll!LdrFindResource_U
    from C:\WINDOWS\system32\ntdll.dll
  • #1 ??
  • #2 ??
  • #3 ??
  • #4 ntdll!LdrFindResource_U
    from C:\WINDOWS\system32\ntdll.dll
  • #5 ??
  • #6 ntdll!LdrFindResource_U
    from C:\WINDOWS\system32\ntdll.dll
  • #7 ??
  • #8 ntdll!LdrFindResource_U
    from C:\WINDOWS\system32\ntdll.dll
  • #9 ??
  • #10 ntdll!RtlValidateUnicodeString
    from C:\WINDOWS\system32\ntdll.dll
  • #11 ntdll!LdrShutdownProcess
    from C:\WINDOWS\system32\ntdll.dll
  • #12 ??
  • #0 ntdll!LdrAccessResource
    from C:\WINDOWS\system32\ntdll.dll
  • #1 ntdll!ZwWaitForMultipleObjects
    from C:\WINDOWS\system32\ntdll.dll
  • #2 KERNEL32!CreateFileMappingA
    from C:\WINDOWS\system32\kernel32.dll
  • #3 USER32!GetLastInputInfo
    from C:\WINDOWS\system32\user32.dll
  • #4 USER32!MsgWaitForMultipleObjects
    from C:\WINDOWS\system32\user32.dll
  • #5 gdiplus!GdipTranslateWorldTransform
    from C:\WINDOWS\WinSxS\x86_Microsoft.Windows.GdiPlus_6595b64144ccf1df_1.0.6001.22319_x-ww_f0b4c2df\GdiPlus.dll
  • #6 KERNEL32!GetModuleFileNameA
    from C:\WINDOWS\system32\kernel32.dll
  • #7 ??
  • #0 ntdll!LdrAccessResource
    from C:\WINDOWS\system32\ntdll.dll
  • #1 ntdll!ZwWaitForSingleObject
    from C:\WINDOWS\system32\ntdll.dll
  • #2 WaitForSingleObjectEx
    from C:\WINDOWS\system32\kernel32.dll
  • #3 ??
  • #4 ??

Comment 43 Fridrich Strba 2010-06-03 12:30:05 UTC
And this is the error on console:
Gdk:ERROR:gdkgc-win32.c:751:get_impl_drawable: code should not be reached
Comment 44 Martin Schlemmer 2010-06-03 12:39:18 UTC
> 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?
Comment 45 Fridrich Strba 2010-06-03 12:57:33 UTC
Breakpoint 2, get_impl_drawable (drawable=0x0) at gdkgc-win32.c:751
751     in gdkgc-win32.c
(gdb) bt
  • #0 get_impl_drawable
    at gdkgc-win32.c line 751
  • #1 gdk_win32_hdc_get
    at gdkgc-win32.c line 820
  • #2 xp_theme_draw
    at xp_theme.c line 902
  • #3 draw_box
    at msw_style.c line 2184
  • #4 gtk_paint_box
    at gtkstyle.c line 6204
  • #5 gtk_progress_bar_paint
    at gtkprogressbar.c line 991
  • #6 gtk_progress_create_pixmap
    at gtkprogress.c line 330
  • #7 gtk_progress_realize
    at gtkprogress.c line 237
  • #8 g_cclosure_marshal_VOID__VOID
    at gmarshal.c line 77
  • #9 g_type_class_meta_marshal
    at gclosure.c line 878
  • #10 g_closure_invoke
    at gclosure.c line 767
  • #11 signal_emit_unlocked_R
    at gsignal.c line 3178
  • #12 g_signal_emit_valist
    at gsignal.c line 2981
  • #13 g_signal_emit
    at gsignal.c line 3038
  • #14 gtk_widget_realize
    at gtkwidget.c line 3501
  • #15 gtk_widget_set_parent
    at gtkwidget.c line 6351
  • #16 gtk_box_pack
    at gtkbox.c line 755
  • #17 gtk_box_pack_end
    at gtkbox.c line 850
  • #18 splash_create
    at splash.c line 195
  • #19 gui_init
    at gui.c line 224
  • #20 app_run
    at app.c line 193
  • #21 main
    at main.c line 397
Continuing.

Program exited with code 03.
(gdb)
Comment 46 Martin Schlemmer 2010-06-03 13:36:17 UTC
(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.
Comment 47 Fridrich Strba 2010-06-03 14:18:12 UTC
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
  • #0 _gdk_window_start_draw_helper
    at gdkwindow.c line 3547
  • #1 gdk_win32_window_start_draw_libgtk_only
    at gdkwindow-win32.c line 3567
  • #2 xp_theme_draw
    at xp_theme.c line 877
  • #3 draw_box
    at msw_style.c line 2184
  • #4 gtk_paint_box
    at gtkstyle.c line 6204
  • #5 gtk_progress_bar_paint
    at gtkprogressbar.c line 991
  • #6 gtk_progress_create_pixmap
    at gtkprogress.c line 330
  • #7 gtk_progress_size_allocate
    at gtkprogress.c line 301
  • #8 g_cclosure_marshal_VOID__BOXED
    at gmarshal.c line 566
  • #9 g_type_class_meta_marshal
    at gclosure.c line 878
  • #10 g_closure_invoke
    at gclosure.c line 767
  • #11 signal_emit_unlocked_R
    at gsignal.c line 3178
  • #12 g_signal_emit_valist
    at gsignal.c line 2981
  • #13 g_signal_emit
    at gsignal.c line 3038
  • #14 gtk_widget_size_allocate
    at gtkwidget.c line 4061
  • #15 gtk_box_size_allocate
    at gtkbox.c line 596
  • #16 g_cclosure_marshal_VOID__BOXED
    at gmarshal.c line 566
  • #17 g_type_class_meta_marshal
    at gclosure.c line 878
  • #18 g_closure_invoke
    at gclosure.c line 767
  • #19 signal_emit_unlocked_R
    at gsignal.c line 3178
  • #20 g_signal_emit_valist
    at gsignal.c line 2981
  • #21 g_signal_emit
    at gsignal.c line 3038
  • #22 gtk_widget_size_allocate
    at gtkwidget.c line 4061
  • #23 gtk_frame_size_allocate
    at gtkframe.c line 663
  • #24 g_cclosure_marshal_VOID__BOXED
    at gmarshal.c line 566
  • #25 g_type_class_meta_marshal
    at gclosure.c line 878
  • #26 g_closure_invoke
    at gclosure.c line 767
  • #27 signal_emit_unlocked_R
    at gsignal.c line 3178
  • #28 g_signal_emit_valist
    at gsignal.c line 2981
  • #29 g_signal_emit
    at gsignal.c line 3038
  • #30 gtk_widget_size_allocate
    at gtkwidget.c line 4061
  • #31 gtk_window_size_allocate
    at gtkwindow.c line 4979
  • #32 g_cclosure_marshal_VOID__BOXED
    at gmarshal.c line 566
  • #33 g_type_class_meta_marshal
    at gclosure.c line 878
  • #34 g_closure_invoke
    at gclosure.c line 767
  • #35 signal_emit_unlocked_R
    at gsignal.c line 3178
  • #36 g_signal_emit_valist
    at gsignal.c line 2981
  • #37 g_signal_emit
    at gsignal.c line 3038
  • #38 gtk_widget_size_allocate
    at gtkwidget.c line 4061
  • #39 gtk_window_show
    at gtkwindow.c line 4511
  • #40 g_cclosure_marshal_VOID__VOID
    at gmarshal.c line 77
  • #41 g_type_class_meta_marshal
    at gclosure.c line 878
  • #42 g_closure_invoke
    at gclosure.c line 767
  • #43 signal_emit_unlocked_R
    at gsignal.c line 3178
  • #44 g_signal_emit_valist
    at gsignal.c line 2981
  • #45 g_signal_emit
    at gsignal.c line 3038
  • #46 gtk_widget_show
    at gtkwidget.c line 3185
  • #47 gtk_widget_show_now
    at gtkwidget.c line 3236
  • #48 splash_create
    at splash.c line 198
  • #49 gui_init
    at gui.c line 224
  • #50 app_run
    at app.c line 193
  • #51 main
    at main.c line 397

Comment 48 Martin Schlemmer 2010-06-07 14:53:04 UTC
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.
Comment 49 Fridrich Strba 2010-06-07 23:58:05 UTC
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.
Comment 50 Martin Schlemmer 2010-06-08 10:19:10 UTC
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.
Comment 51 Martin Schlemmer 2010-06-08 11:31:42 UTC
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.
Comment 52 Alexander Larsson 2010-06-09 08:37:50 UTC
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.
Comment 53 Martin Schlemmer 2010-06-09 16:26:24 UTC
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.
Comment 54 Martin Schlemmer 2010-06-09 17:54:00 UTC
(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 ...
Comment 55 Alexander Larsson 2010-06-09 20:03:31 UTC
>> 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().
Comment 56 Martin Schlemmer 2010-06-09 20:28:16 UTC
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.
Comment 57 Alexander Larsson 2010-06-10 07:03:44 UTC
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.
Comment 58 Alexander Larsson 2010-06-10 07:30:35 UTC
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.
Comment 59 Martin Schlemmer 2010-06-10 10:58:48 UTC
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".
Comment 60 Alexander Larsson 2010-06-10 15:01:03 UTC
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
Comment 61 Alexander Larsson 2010-06-10 15:03:02 UTC
Otherwise this looks good to me though.
Comment 62 Martin Schlemmer 2010-06-10 15:20:11 UTC
(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;
Comment 63 Martin Schlemmer 2010-06-10 15:32:05 UTC
Actually, nevermind about the first question about DRAW_TARGET_UNSUPPORTED - no need for fluff until its needed.
Comment 64 Martin Schlemmer 2010-06-10 15:57:25 UTC
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.
Comment 65 Martin Schlemmer 2010-06-10 16:03:08 UTC
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.
Comment 66 Martin Schlemmer 2010-06-10 16:55:49 UTC
Created attachment 163307 [details] [review]
gtk+-2.20.1-xptheme-v15.patch

Sorry, cleaned up a bit more.
Comment 67 Alexander Larsson 2010-06-10 18:02:43 UTC
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.
Comment 68 Alexander Larsson 2010-06-10 18:02:46 UTC
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.
Comment 69 Martin Schlemmer 2010-06-10 18:57:01 UTC
(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?
Comment 70 Martin Schlemmer 2010-06-10 19:00:39 UTC
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?
Comment 71 Martin Schlemmer 2010-06-10 20:32:37 UTC
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 =/
Comment 72 Martin Schlemmer 2010-06-10 21:42:28 UTC
(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.
Comment 73 Alexander Larsson 2010-06-11 08:24:22 UTC
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.
Comment 74 Martin Schlemmer 2010-06-11 15:01:25 UTC
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.
Comment 75 Alexander Larsson 2010-06-11 15:29:08 UTC
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.
Comment 76 Alexander Larsson 2010-06-11 15:30:20 UTC
Also, i would add the offscreen check to GDK_WINDOW_HWND rather than in GDK_DRAWABLE_HANDLE as that fixes more users.
Comment 77 Alexander Larsson 2010-06-11 18:05:48 UTC
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 78 Alexander Larsson 2010-06-11 18:22:13 UTC
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.
Comment 79 Martin Schlemmer 2010-06-11 18:35:42 UTC
(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 =)
Comment 80 Martin Schlemmer 2010-06-11 18:49:31 UTC
If I filter offscreen windows in xp_theme.c, the theming is not applied btw.
Comment 81 Martin Schlemmer 2010-06-11 18:50:27 UTC
(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.
Comment 82 Martin Schlemmer 2010-06-11 18:53:46 UTC
(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);
Comment 83 Paolo Borelli 2010-06-12 11:00:10 UTC
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
Comment 84 Martin Schlemmer 2010-06-12 13:48:36 UTC
(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 ...
Comment 85 Alexander Larsson 2010-06-14 08:24:31 UTC
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?
Comment 86 Martin Schlemmer 2010-06-14 11:49:28 UTC
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.
Comment 87 Alexander Larsson 2010-06-14 14:23:30 UTC
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.
Comment 88 Martin Schlemmer 2010-06-14 15:07:17 UTC
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.
Comment 89 Alexander Larsson 2010-06-14 19:37:36 UTC
commited
Comment 90 viktor 2010-06-26 14:07:49 UTC
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.
Comment 91 Alexander Larsson 2010-06-28 09:20:29 UTC
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.
Comment 92 John Stowers 2010-06-28 10:19:29 UTC
(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.
Comment 94 Tao Wang 2010-07-13 02:36:52 UTC
Any updates for backport the patch to 2.x? It seems work on 2.20
Comment 95 Martin Schlemmer 2010-07-23 14:11:41 UTC
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.
Comment 96 Martin Schlemmer 2010-07-23 14:15:20 UTC
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.
Comment 97 Eugen Dedu 2010-07-25 16:59:37 UTC
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...
Comment 98 Fridrich Strba 2010-07-26 09:01:31 UTC
Martin, just for record: the headers that I use (mingw-w64 and the windows SDK 7.0) don't define THEME_SIZE, but THEMESIZE.
Comment 99 Martin Schlemmer 2010-07-26 11:55:22 UTC
(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.
Comment 100 Martin Schlemmer 2010-07-26 11:59:34 UTC
(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!
Comment 101 Martin Schlemmer 2010-07-26 12:00:35 UTC
Created attachment 166574 [details] [review]
gtk+-2.20.1-xptheme-warnings.patch

Fix some warnings related to type differences and unused variables.
Comment 102 Martin Schlemmer 2010-07-26 12:01:51 UTC
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.
Comment 103 Martin Schlemmer 2010-07-26 12:03:58 UTC
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.
Comment 104 Martin Schlemmer 2010-07-26 14:16:25 UTC
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.
Comment 105 Fridrich Strba 2010-07-27 13:01:01 UTC
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 :)
Comment 106 Martin Schlemmer 2010-07-27 13:15:32 UTC
Tested it on Windows XP/Vista/7 (only 32bit), but thanks, more eyes the better :)
Comment 107 viktor 2010-08-23 14:02:50 UTC
so, what patches do we need to apply to gtk 2.20 to get xp theming work? won't these be committed anytime?
Comment 108 Matthias Clasen 2010-08-26 16:58:07 UTC
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...
Comment 109 Fridrich Strba 2010-08-27 11:12:29 UTC
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.
Comment 110 Fridrich Strba 2010-08-27 11:16:32 UTC
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.
Comment 111 Martin Schlemmer 2010-08-27 11:23:37 UTC
(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?
Comment 112 Fridrich Strba 2010-08-27 12:26:17 UTC
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.
Comment 113 viktor 2010-08-27 13:05:14 UTC
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.
Comment 114 Fridrich Strba 2010-08-27 20:45:23 UTC
(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.
Comment 115 Tor Lillqvist 2010-08-28 06:32:16 UTC
Fridrich, feel free to commit the ms-windows theme engine patches to the 2.22 branch. Maybe also 2.20?
Comment 116 Fridrich Strba 2010-08-28 13:26:12 UTC
(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.
Comment 117 Fridrich Strba 2010-08-30 19:08:22 UTC
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?
Comment 118 viktor 2010-08-31 18:11:43 UTC
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.
Comment 119 Fridrich Strba 2010-09-02 04:55:39 UTC
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.
Comment 120 Martin Schlemmer 2010-09-02 10:18:15 UTC
(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.
Comment 121 Fridrich Strba 2010-09-02 20:27:38 UTC
(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 :)
Comment 122 Martin Schlemmer 2010-09-03 16:31:25 UTC
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.
Comment 123 Martin Schlemmer 2010-09-03 16:58:37 UTC
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?
Comment 124 Fridrich Strba 2010-09-03 20:25:42 UTC
(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.
Comment 125 Fridrich Strba 2010-09-03 20:27:58 UTC
Review of attachment 166576 [details] [review]:

committed
Comment 126 Fridrich Strba 2010-09-03 20:28:54 UTC
Review of attachment 166574 [details] [review]:

committed
Comment 127 Fridrich Strba 2010-09-03 20:29:36 UTC
Review of attachment 166586 [details] [review]:

committed
Comment 128 Fridrich Strba 2010-09-03 20:30:15 UTC
Review of attachment 169441 [details] [review]:

much better then current status and does not influence anything other
Comment 129 Fridrich Strba 2010-09-03 20:30:38 UTC
Review of attachment 166576 [details] [review]:

committed
Comment 130 Fridrich Strba 2010-09-03 20:34:34 UTC
(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.
Comment 131 Martin Schlemmer 2010-09-06 19:24:00 UTC
(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.
Comment 132 Tor Lillqvist 2010-09-12 03:03:16 UTC
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.
Comment 133 viktor 2010-09-29 12:23:31 UTC
Created attachment 171332 [details]
progress bar on xp
Comment 134 viktor 2010-09-29 12:24:09 UTC
Created attachment 171333 [details]
progress bar on 7
Comment 135 viktor 2010-09-29 12:24:33 UTC
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"
Comment 136 Martin Schlemmer 2010-09-29 13:33:50 UTC
(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).
Comment 137 viktor 2010-09-29 13:53:35 UTC
many thanks Martin. it was indeed set to 8, now it works fine with 10.
Comment 138 Martin Schlemmer 2010-10-14 11:40:25 UTC
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.
Comment 139 Fridrich Strba 2010-10-14 12:22:53 UTC
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 :)
Comment 140 Timothy Arceri 2013-10-08 06:21:30 UTC
No new updates to this bug report for quite some time now. Is this bug finally safe to close?
Comment 141 Martin Schlemmer 2013-10-08 10:55:53 UTC
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.
Comment 142 Timothy Arceri 2013-10-08 21:08:00 UTC
Closing bug. Remaining issues to be tracked in fresh bug reports.