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 650300 - Notebook tabs are incorrectly displayed with the ms-windows engine
Notebook tabs are incorrectly displayed with the ms-windows engine
Status: RESOLVED FIXED
Product: gtk+
Classification: Platform
Component: Backend: Win32
2.24.x
Other Windows
: Normal normal
: ---
Assigned To: gtk-win32 maintainers
gtk-bugs
: 552681 (view as bug list)
Depends on:
Blocks:
 
 
Reported: 2011-05-16 10:48 UTC by Jerome Lambourg
Modified: 2011-12-08 08:57 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
patch on gtk+-2.24.4 (15.42 KB, patch)
2011-05-16 10:48 UTC, Jerome Lambourg
none Details | Review
Better formatted patch, against 2.24.5 (14.37 KB, patch)
2011-09-28 10:22 UTC, Jerome Lambourg
needs-work Details | Review
Notebook comparison between GTK+ 2.16, 2.24 and a native app (49.50 KB, image/png)
2011-09-28 12:00 UTC, Dieter Verfaillie
  Details
2nd Notebook comparison between GTK+ 2.16, 2.24 and a native app (17.22 KB, image/png)
2011-09-28 12:04 UTC, Dieter Verfaillie
  Details
Correct final glitches observed above (15.58 KB, patch)
2011-09-29 09:20 UTC, Jerome Lambourg
none Details | Review
The final rendering on Windows 7 (48.91 KB, image/png)
2011-09-29 09:24 UTC, Jerome Lambourg
  Details
XP Luna nitpickings (58.12 KB, image/png)
2011-09-29 12:05 UTC, Dieter Verfaillie
  Details
XP Classic trouble (30.13 KB, image/png)
2011-09-29 12:07 UTC, Dieter Verfaillie
  Details
final patch ? (16.16 KB, patch)
2011-10-01 08:12 UTC, Jerome Lambourg
none Details | Review
With the luna theme (41.88 KB, image/png)
2011-10-01 08:15 UTC, Jerome Lambourg
  Details
windows 7 rendering (52.20 KB, image/png)
2011-10-01 08:19 UTC, Jerome Lambourg
  Details
Ultimate notebook rendering test (3.05 KB, text/plain)
2011-10-21 12:32 UTC, Dieter Verfaillie
  Details
Ultimate notebook rendering test on Windows XP (115.14 KB, image/png)
2011-10-21 12:38 UTC, Dieter Verfaillie
  Details
win32: fix themed notebook tab renderering (18.72 KB, patch)
2011-10-21 12:45 UTC, Dieter Verfaillie
none Details | Review
ms-windows-notebook-tabs-clipping.patch (1.66 KB, patch)
2011-11-29 16:26 UTC, Martin Schlemmer
none Details | Review

Description Jerome Lambourg 2011-05-16 10:48:54 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.
Comment 1 Dieter Verfaillie 2011-09-28 09:53:40 UTC
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
Comment 2 Jerome Lambourg 2011-09-28 10:22:14 UTC
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.
Comment 3 Dieter Verfaillie 2011-09-28 11:20:23 UTC
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.
Comment 4 Dieter Verfaillie 2011-09-28 12:00:10 UTC
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)
Comment 5 Dieter Verfaillie 2011-09-28 12:04:48 UTC
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!
Comment 6 Jerome Lambourg 2011-09-29 09:20:00 UTC
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.
Comment 7 Jerome Lambourg 2011-09-29 09:24:16 UTC
Created attachment 197738 [details]
The final rendering on Windows 7
Comment 8 Dieter Verfaillie 2011-09-29 12:05:58 UTC
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
Comment 9 Dieter Verfaillie 2011-09-29 12:07:01 UTC
Created attachment 197747 [details]
XP Classic trouble

We seem to have some trouble with vertical notebook tabs on XP Classic, sadly...
Comment 10 Jerome Lambourg 2011-09-29 12:41:42 UTC
(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 11 Dieter Verfaillie 2011-09-29 12:58:26 UTC
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.
Comment 12 Jerome Lambourg 2011-10-01 08:12:44 UTC
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.
Comment 13 Jerome Lambourg 2011-10-01 08:15:19 UTC
Created attachment 197946 [details]
With the luna theme

Rendering on XP with the Luna theme
Comment 14 Jerome Lambourg 2011-10-01 08:19:18 UTC
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 ...
Comment 15 Dieter Verfaillie 2011-10-21 07:15:49 UTC
*** Bug 552681 has been marked as a duplicate of this bug. ***
Comment 16 Dieter Verfaillie 2011-10-21 12:32:34 UTC
Created attachment 199634 [details]
Ultimate notebook rendering test
Comment 17 Dieter Verfaillie 2011-10-21 12:38:07 UTC
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.
Comment 18 Dieter Verfaillie 2011-10-21 12:45:19 UTC
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
Comment 19 Dieter Verfaillie 2011-11-08 14:54:32 UTC
This problem has been fixed in our software repository. The fix will go into the next software release. Thank you for your bug report.
Comment 20 Martin Schlemmer 2011-11-22 14:56:19 UTC
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.
Comment 21 Martin Schlemmer 2011-11-29 16:26:39 UTC
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.
Comment 22 Dieter Verfaillie 2011-12-07 19:29:41 UTC
(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.
Comment 23 Martin Schlemmer 2011-12-08 08:57:08 UTC
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.