GNOME Bugzilla – Bug 650300
Notebook tabs are incorrectly displayed with the ms-windows engine
Last modified: 2011-12-08 08:57:08 UTC
Created attachment 187898 [details] [review] patch on gtk+-2.24.4 The ms-windows engine part of gtk+-2.24 incorrectly displays notebooks: the tabs are not attached to the body, and look more like regular buttons than actual notebook tabs. Also, the frame around the notebooks is also incorrectly drawn. The attached patch correct those issues: It removes lots of useless code that was used to remove some part of the tab item when the item was aligned to the edge of the notebook, and uses the correct Windows theme element instead (thanks to the use of the new internal function get_notebook_tab_position). The patch also simplifies the handling of tabs that are not on top of the notebook, by just using flip/rotate.
Hi Jerome, Looks like bugzilla's patch review feature is having trouble with the format of you patch. Could you please generate a new one by either: - "diff -Naurp" MSYS has a working version of diff in it's diffutils package, it's only a "mingw-get install msys-diffutils" [1] away. Or otherwise maybe the gnuwin32 version [2]. - "git format-patch -1 shaofyourcommit", available on Windows through msysgit [3]. I think the "git format-patch" route is preferred though. Thanks! [1] http://www.mingw.org/wiki/Getting_Started [2] http://gnuwin32.sourceforge.net/packages/diffutils.htm [3] http://code.google.com/p/msysgit/, I recommend the PortableGit version
Created attachment 197645 [details] [review] Better formatted patch, against 2.24.5 I used git format-patch -l <> --no-prefix I also cleaned up the patch a bit, added comments when needed. Below is a more detailed change log of the patch: * modules/engines/ms-windows/xp_theme.[ch]: Add the new element XP_THEME_ELEMENT_TAB_ITEM_BOTH_EDGE that allows drawing a notebook tab that have both edges in line with the notebook body. * modules/engines/ms-windows/msw_style.c: (get_notebook_tab_position): New helper function telling if the tab being drawn is the first one, the last one, both, or none. (draw_themed_tab_button): Simplify (and fix) the way tabs are drawn, in particular when the tab is not on top of the container. (draw_box_gap): Fix the drawing of the gap for notebooks.
Review of attachment 197645 [details] [review]: Other than the small remark, looks good to me. I'll go test various scenarios now (XP classic and luna theme, horizontal/vertical tabs, etc) and ask around for somebody to test on newer Windows versions too. Might take a while. ::: modules/engines/ms-windows/msw_style.c @@ +2356,3 @@ + + gboolean found_tabs = FALSE; + gint i, n_pages; I think C89 compilers like MSVC are not going to like this (at least, I was pointed out to something comparable only a couple of days ago). Better to define variables at the beginning of the block and only then assign the default values.
Created attachment 197651 [details] Notebook comparison between GTK+ 2.16, 2.24 and a native app Compared GTK+ 2.16, 2.24 and a native app. Fixes some longstanding bugs (ie the "water" tabs in the screenshot). Nice :) Some nitpicking visible in this screenshot: - the "gap" between notebook tabs seems to be gone - tabs rendered at the bottom of the notebook look a bit too tall, there's also the line above the "1" tab that shouldn't be there (looks like tabs are drawn 1 pixel off)
Created attachment 197653 [details] 2nd Notebook comparison between GTK+ 2.16, 2.24 and a native app Some more details visible in this screenshot: - the "indent" of the first non-active tab seems to be gone - zooming in on the "cccccccccc" tab of the 2.24 test window, you can see it is drawn 1 pixel too high Otherwise, getting really close. Thanks!
Created attachment 197736 [details] [review] Correct final glitches observed above This one should correct the remaining issues observed above: The gap is reintroduced for active tabs, the pixel shift should now be corrected. I had to correct also the way I took into account potential differences between drawing rectangle and clipping rectangle.
Created attachment 197738 [details] The final rendering on Windows 7
Created attachment 197746 [details] XP Luna nitpickings Thanks you for the quick update! Tested and here's some nitpicking for XP Luna: - native apps use a 2 pixel indent for the first tab - active tab seems to be off by 1 pixel
Created attachment 197747 [details] XP Classic trouble We seem to have some trouble with vertical notebook tabs on XP Classic, sadly...
(In reply to comment #9) > We seem to have some trouble with vertical notebook tabs on XP Classic, > sadly... The proposed patch only applies when xp_theme_draw works, so should not be involved in the XP Classic case, as far as I can tell.
Comment on attachment 197747 [details] XP Classic trouble (In reply to comment #10) > The proposed patch only applies when xp_theme_draw works, so should not be > involved in the XP Classic case, as far as I can tell. Was just testing both luna and classic themes without the proposed patch. The vertical notebook tab problem with classic is indeed material for another bug report, probably related to bug #552681 which I'm investigating now.
Created attachment 197945 [details] [review] final patch ? OK, I finally corrected many errors that made the rendering not totally consistent: - My algorithm to determine whether the normal tab was first/last was incorrect - I did not take into account transparency when rotating the tabs The new patch should solve all glitches observed previously. I also took the opportunity to replace the use of pixbuf by cairo.
Created attachment 197946 [details] With the luna theme Rendering on XP with the Luna theme
Created attachment 197947 [details] windows 7 rendering And here is the rendering on Windows7. There is still a small glitch when displaying a vertical tab that should be aligned with the bottom of the notebook, but correcting it breaks the Luna theme display ...
*** Bug 552681 has been marked as a duplicate of this bug. ***
Created attachment 199634 [details] Ultimate notebook rendering test
Created attachment 199635 [details] Ultimate notebook rendering test on Windows XP There's very few glitches left on XP, all in situations hopefully unlikely used (pack_end notebook tabs? that looks unnatural on windows to start with). I don't think these can be solved without actually knowing what tab we're rendering in draw_themed_tab_button, which GTK+ 2 is never going to provide... Could you run this on windows 7 and post a screenshot (just out of curiosity). Thanks.
Created attachment 199636 [details] [review] win32: fix themed notebook tab renderering Cleaned up patch: - coding style - whitespace issues - commit message - author This patch has been pushed onto the experimental gtk-2-24-win32 branch[1], which should hopefully be merged onto gtk-2-24 soon. I'll leave this bug report open until that happens. Jerome: Thanks for your work! [1] http://git.gnome.org/browse/gtk+/log/?h=gtk-2-24-win32
This problem has been fixed in our software repository. The fix will go into the next software release. Thank you for your bug report.
I know the bug is closed, but has anybody noticed that especially on Windows 7 it looks like width of the tab is not wide enough for the label, so it gets clipped a bit. I have been "fixing" it by adding a X-pad of 2 to the labels, but still. Haven't had much time for ms-windows engine hacking the last months unfortunately, so I as yet never got to look at this.
Created attachment 202382 [details] [review] ms-windows-notebook-tabs-clipping.patch Tracked it to a hack to center the label/whatever. Also, the I do not think the draw_focus should be done - tried it via style and 'focus-line-width', but had to add the second hunk as well. Let me know if I should add a new bug, or reopen this one.
(In reply to comment #21) > Created an attachment (id=202382) [details] [review] > ms-windows-notebook-tabs-clipping.patch > > Tracked it to a hack to center the label/whatever. Also, the I do not think the > draw_focus should be done - tried it via style and 'focus-line-width', but had > to add the second hunk as well. Special casing notebook tabs to disable FocusRects is wrong imho. While true that FocusRects are no longer rendered by default for native windows controls since at least windows 2000 (no idea about earlier versions, but who cares), they are still being rendered the moment you start using keyboard navigation. Without that, keynav would be extremely hard to use. So completely disabling FocusRects is not going to be a good idea I guess, even if only for notebook tabs. Additionally, I'm not really convinced the decision to show a FocusRect as described above should be done by the ms-windows theme engine. It feels like something that should "just work" for all themes. But that might simply be impossible though... All this FocusRect stuff deserves to become a separate bug report, and maybe a discussion on windows-devel-list too as how to go about all this is the question and the answer to that question will lead to the discovery of this being doable in 2.24 and 3 or 3 only or not possible at all. Who knows? Would be cool to finally fix this after a decade of alien looking GtkButtons and whatnot :) > Let me know if I should add a new bug, or reopen this one. Please create a new report for the label displacement bug. That way we have something to track.
Will do. The fix for the clipping I need to sort out still though, as just extending the area causes overdrawing issues. And then there are clipping problems as well ... But will add a new bug with patches after I am happy.