#11420: Support for themes on ownerdrawn menu
wxTrac
#11420: Support for themes on ownerdrawn menu
-------------------------+--------------------------------------------------
Reporter: malcompl | Owner:
Type: enhancement | Status: new
Priority: normal | Milestone:
Component: wxMSW | Version: 2.9-svn
Keywords: wxMenu | Blockedby:
Patch: 1 | Blocking:
-------------------------+--------------------------------------------------
Patch added support for themes on ownerdrawn menu.
The menu looks like a native, but to ensure full compliance apperance with
the native menu still missing a few things:
* misalignment of accel text
* draw correct std check/bullet mark if item is check/radio and bitmap is
not set
I will be work on this problems in next separate patches.
--
Ticket URL: <http://trac.wxwidgets.org/ticket/11420>
wxTrac
#11420: Support for themes on ownerdrawn menu
-------------------------+--------------------------------------------------
Reporter: malcompl | Owner: vadz
Type: enhancement | Status: accepted
Priority: normal | Milestone:
Component: wxMSW | Version: 2.9-svn
Keywords: wxMenu | Blockedby:
Patch: 1 | Blocking:
-------------------------+--------------------------------------------------
Changes (by vadz):
* owner: => vadz
* status: new => accepted
Comment:
Thanks for the patch, before looking at it: do I understand correctly that
it's for the "owerdraw" branch (and not for the trunk)?
--
Ticket URL: <http://trac.wxwidgets.org/ticket/11420#comment:1>
wxTrac
#11420: Support for themes on ownerdrawn menu
-------------------------+--------------------------------------------------
Reporter: malcompl | Owner: vadz
Type: enhancement | Status: accepted
Priority: normal | Milestone:
Component: wxMSW | Version: 2.9-svn
Keywords: wxMenu | Blockedby:
Patch: 1 | Blocking:
-------------------------+--------------------------------------------------
Comment(by malcompl):
Yes, it's for the "ownerdraw-refactor" branch.
--
Ticket URL: <http://trac.wxwidgets.org/ticket/11420#comment:2>
wxTrac
#11420: Support for themes on ownerdrawn menu
-------------------------+--------------------------------------------------
Reporter: malcompl | Owner: vadz
Type: enhancement | Status: accepted
Priority: normal | Milestone:
Component: wxMSW | Version: 2.9-svn
Keywords: wxMenu | Blockedby:
Patch: 1 | Blocking:
-------------------------+--------------------------------------------------
Comment(by malcompl):
Correct small bug and added support for pseudo menu themes on Windows XP.
--
Ticket URL: <http://trac.wxwidgets.org/ticket/11420#comment:3>
wxTrac
#11420: Support for themes on ownerdrawn menu
--------------------------+-------------------------------------------------
Reporter: malcompl | Owner: vadz
Type: enhancement | Status: closed
Priority: normal | Milestone:
Component: wxMSW | Version: 2.9-svn
Resolution: fixed | Keywords: wxMenu
Blockedby: | Patch: 1
Blocking: |
--------------------------+-------------------------------------------------
Changes (by vadz):
* status: accepted => closed
* resolution: => fixed
Comment:
Applied with a couple of very minor changes, thanks (and sorry for the
delay).
Looking forward to the next patches and to merging the ownerdraw-refactor
branch into trunk!
--
Ticket URL: <http://trac.wxwidgets.org/ticket/11420#comment:4>
Marcin 'Malcom' Malich
> Ticket URL: <http://trac.wxwidgets.org/ticket/11420#comment:4>
>
> delay).
>
> Looking forward to the next patches and to merging the ownerdraw-refactor
> branch into trunk!
>
Before merging the branch we should resolve too the patch for
wxCheckListBox (#10286).
Other things, when I wrote this patch, I've added some consts and defs
for uxthemes at the top of the file. I saw a similar phenomenon in
other files, which were using uxthemes.
So I think maybe it would be a better solution to add a file uxdefs.h
or sth for all constants and definitions needed for themes to be fully
supported, the content of this file should be based on MSND docs and
PSDK (vssym32.h, vsstyle.h).
It will be better to keep all of themes related defs in one file than
in different places. This should also improve the readability of the
code.
What do you think about this idea?
--
Pozdrowienia,
Marcin 'Malcom' Malich
m...@malcom.pl
http://malcom.pl
Vadim Zeitlin
MMM> Thanks for approval, I will send next patches for menu soon.
Thanks, I've applied them now and the "Drawn" and "Native" menus look
almost identically now. The two tiny differences I see are:
- The text is one pixel too low in owner-drawn case.
- The check mark is one pixel too high.
This is really minor, of course, but it would be great to correct this if
possible (I didn't look at the code at all but could this be simply due to
using x/2 instead of (x+1)/2?).
MMM> Before merging the branch we should resolve too the patch for
MMM> wxCheckListBox (#10286).
Sorry, I've somehow forgot about this one. I've applied it to the
ownerdraw-refactor branch as well now, thanks.
MMM> Other things, when I wrote this patch, I've added some consts and defs
MMM> for uxthemes at the top of the file. I saw a similar phenomenon in
MMM> other files, which were using uxthemes.
MMM> So I think maybe it would be a better solution to add a file uxdefs.h
MMM> or sth for all constants and definitions needed for themes to be fully
MMM> supported, the content of this file should be based on MSND docs and
MMM> PSDK (vssym32.h, vsstyle.h).
MMM> It will be better to keep all of themes related defs in one file than
MMM> in different places. This should also improve the readability of the
MMM> code.
MMM>
MMM> What do you think about this idea?
On one hand, it would probably indeed make sense to have wx/msw/tmschema.h
just as we already have wx/msw/uxtheme.h. OTOH having all definitions in
one file means that a lot of files will be recompiled even when you add a
constant which is only used in one of them. So creating a central file
would IMO only make sense if there are a lot of constants which are needed
in more than one file. Is this the case?
Regards,
VZ
Marcin 'Malcom' Malich
>
> Thanks, I've applied them now and the "Drawn" and "Native" menus look
> almost identically now. The two tiny differences I see are:
>
> - The text is one pixel too low in owner-drawn case.
> - The check mark is one pixel too high.
>
> This is really minor, of course, but it would be great to correct this if
> possible (I didn't look at the code at all but could this be simply due to
> using x/2 instead of (x+1)/2?).
>
int y = rcText.top + (rcText.bottom - rcText.top - textSize.cy) / 2;
I don't known why on not FullThemed menu the text is 1px too low.
So maybe we should fix it by add sth like that:
// we drawing vertically centered text label, but on non FullTheme
menu
// is 1px too low in comparison to native menus
if ( data->MenuLayout() != MenuDrawData::FullTheme )
rcText.top--;
I found the problem on drawing check mark and other bitmap, I forgot
about margin.
Anyway, If the solution of text problem is ok for you, I will add
patch to this ticket on trac.
> On one hand, it would probably indeed make sense to have wx/msw/tmschema.h
> just as we already have wx/msw/uxtheme.h.
"Starting from the Longhorn thschema.h is obsolete. vssym32.h should
be used instead."
> OTOH having all definitions in
> one file means that a lot of files will be recompiled even when you add a
> constant which is only used in one of them. So creating a central file
> would IMO only make sense if there are a lot of constants which are needed
> in more than one file. Is this the case?
current constants to this file and we will add next if needed. I was
thinking about adding every possible values (based on msdn and sdk
data), then recompilation will occur extremely rare, but file will be
big, 2.5k lines.
Vadim Zeitlin
MMM> I don't known why on not FullThemed menu the text is 1px too low.
MMM> So maybe we should fix it by add sth like that:
MMM>
MMM> // we drawing vertically centered text label, but on non FullTheme
MMM> menu
MMM> // is 1px too low in comparison to native menus
MMM> if ( data->MenuLayout() != MenuDrawData::FullTheme )
MMM> rcText.top--;
MMM>
MMM>
MMM> I found the problem on drawing check mark and other bitmap, I forgot
MMM> about margin.
MMM>
MMM> Anyway, If the solution of text problem is ok for you, I will add
MMM> patch to this ticket on trac.
I don't really like this hack but if there is no other explanation as to
why this happens we probably should do it. OTOH it seems that the native
code does centre the label more correctly, actually: in my (default) theme
I have 6 pixels above it and 6 pixels below in the native menu but 7 above
and 5 below when using owner-drawn one. Clearly the former is more properly
centered than the latter...
MMM> > On one hand, it would probably indeed make sense to have wx/msw/tmschema.h
MMM> > just as we already have wx/msw/uxtheme.h.
MMM>
MMM> from tmschema.h on current skd:
MMM> "Starting from the Longhorn thschema.h is obsolete. vssym32.h should
MMM> be used instead."
I completely missed this, sorry. I don't understand the meaning of "32" in
this file name (doesn't it work under Win64?) and the separation between
it and vsstyle.h seems quite artificial. So maybe we should use our own,
simple and clear name, like wx/msw/themeparts.h.
MMM> > OTOH having all definitions in
MMM> > one file means that a lot of files will be recompiled even when you add a
MMM> > constant which is only used in one of them. So creating a central file
MMM> > would IMO only make sense if there are a lot of constants which are needed
MMM> > in more than one file. Is this the case?
MMM>
MMM> You are right, but this recompilation will occur if we will add only
MMM> current constants to this file and we will add next if needed. I was
MMM> thinking about adding every possible values (based on msdn and sdk
MMM> data), then recompilation will occur extremely rare, but file will be
MMM> big, 2.5k lines.
This is not a problem but I don't see how can we create the entire file
without copying it from Microsoft headers (which would be an obvious
copyright breach). Adding individual constants is easy, I use a simple
Win32 program using DrawThemeBackground() with each possible value
(selectable using 2 comboboxes) and showing its value. But this can't be
really automated.
Regards,
VZ
wxTrac
#11420: Support for themes on ownerdrawn menu
Reporter: malcompl | Owner: vadz
Component: wxMSW | Version: 2.9-svn
Blockedby: | Patch: 1
Blocking: |
--------------------------+-------------------------------------------------
Changes (by malcompl):
* status: closed => reopened
* resolution: fixed =>
Comment:
Small patch for resolve the 2 problems:
- The text is one pixel too low in owner-drawn case.
- The check mark is one pixel too high.
Ticket URL: <http://trac.wxwidgets.org/ticket/11420#comment:5>
Marcin 'Malcom' Malich
>
> MMM> I don't known why on not FullThemed menu the text is 1px too low.
> MMM> So maybe we should fix it by add sth like that:
> MMM>
> MMM> // we drawing vertically centered text label, but on non FullTheme
> MMM> menu
> MMM> // is 1px too low in comparison to native menus
> MMM> if ( data->MenuLayout() != MenuDrawData::FullTheme )
> MMM> rcText.top--;
> MMM>
> MMM>
> MMM> I found the problem on drawing check mark and other bitmap, I forgot
> MMM> about margin.
> MMM>
> MMM> Anyway, If the solution of text problem is ok for you, I will add
> MMM> patch to this ticket on trac.
>
> I don't really like this hack but if there is no other explanation as to
> why this happens we probably should do it. OTOH it seems that the native
> code does centre the label more correctly, actually: in my (default) theme
> I have 6 pixels above it and 6 pixels below in the native menu but 7 above
> and 5 below when using owner-drawn one. Clearly the former is more properly
> centered than the latter...
drawing by DrawText with DT_VCENTER flag, which same result - 1 px
difference. I don't know how native version calculates positions.
I attached the patch on trac.
wxTrac
#11420: Support for themes on ownerdrawn menu
Reporter: malcompl | Owner: vadz
Component: wxMSW | Version: 2.9-svn
Blockedby: | Patch: 1
Blocking: |
--------------------------+-------------------------------------------------
Changes (by VZ):
* status: reopened => closed
Comment:
wxMSW.
The label was offset by 1 pixel vertically and the check marks were 1
pixel
too wide compared to the native ones under XP, correct this.
Closes #11420 (again).
--
Ticket URL: <http://trac.wxwidgets.org/ticket/11420#comment:6>
wxTrac
#11420: Support for themes on ownerdrawn menu
--------------------------+-------------------------------------------------
Reporter: malcompl | Owner: vadz
Type: enhancement | Status: closed
Priority: normal | Milestone:
Component: wxMSW | Version: 2.9-svn
Resolution: fixed | Keywords: wxMenu
Blockedby: | Patch: 1
Blocking: |
--------------------------+-------------------------------------------------
Comment(by VZ):
(In [63223]) Use theme functions for drawing owner-drawn menus.
This makes the menu items with custom attributes or bitmaps look more
native,
especially on post-XP systems.
Closes #11420.
--
Ticket URL: <http://trac.wxwidgets.org/ticket/11420#comment:7>
wxTrac
#11420: Support for themes on ownerdrawn menu
--------------------------+-------------------------------------------------
Reporter: malcompl | Owner: vadz
Type: enhancement | Status: closed
Priority: normal | Milestone:
Component: wxMSW | Version: 2.9-svn
Resolution: fixed | Keywords: wxMenu
Blockedby: | Patch: 1
Blocking: |
--------------------------+-------------------------------------------------
Comment(by VZ):
(In [63227]) Fix off by 1 errors in owner-drawn menu drawing code in
wxMSW.
The label was offset by 1 pixel vertically and the check marks were 1
pixel
too wide compared to the native ones under XP, correct this.
Closes #11420 (again).
--
Ticket URL: <http://trac.wxwidgets.org/ticket/11420#comment:8>