#14632: XRC derive font from enclosing control

22 views
Skip to first unread message

wxTrac

unread,
Sep 5, 2012, 2:46:41 PM9/5/12
to wx-...@googlegroups.com
Ticket URL: <http://trac.wxwidgets.org/ticket/14632>

#14632: XRC derive font from enclosing control
------------------------------------+---------------------------------------
Reporter: sodev | Owner:
Type: enhancement | Status: new
Priority: normal | Milestone:
Component: XRC | Version: 2.9-svn
Keywords: xrc wxFont inheritance | Blockedby:
Patch: 1 | Blocking:
------------------------------------+---------------------------------------
Sometimes i need to modify the font that a control has by default, e.g.
make the font of a wxStaticText bold. I do this by getting the font object
of the control right after it was created, modify it and set it back.
Currently this cannot be done with XRC because you have to specify the
font characteristics or try to get the proper system font, but the former
might be different between platforms and even the latter cannot always get
the font that the control has by default.

This patch introduces a new boolean parameter, inherit, to the font
description which allows to use the font object of the enclosing control.
It works exactly like the sysfont parameter but uses the font of the
enclosing control instead of getting a system font. This feature cannot be
used if the font is needed before the control is created or if the control
is not derived from wxWindow.

The implementation was rather simple, however a bigger change was needed
to get this actually to work. The macro XRC_MAKE_INSTANCE() does not what
its name implies, it does not update the m_instance member if it creates
the object from scratch, but i need this member to point to the created
object or i can't get the font from it. After carefully inspecting the xrc
sources i see no reason why it should not update this member if it has
created the object, so i modified this macro to do this as well.

The first patch contains all changes for this feature, including updated
documentation. The second patch fixes the handlers which don't use the
XRC_MAKE_INSTANCE() macro to also update the m_instance member.


--
Ticket URL: <http://trac.wxwidgets.org/ticket/14632>

wxTrac

unread,
Sep 18, 2012, 6:39:11 PM9/18/12
to wx-...@googlegroups.com
Ticket URL: <http://trac.wxwidgets.org/ticket/14632#comment:1>

#14632: XRC derive font from enclosing control
------------------------------------+---------------------------------------
Reporter: sodev | Owner:
Type: enhancement | Status: new
Priority: normal | Milestone: 2.9.5
Component: XRC | Version: 2.9-svn
Keywords: xrc wxFont inheritance | Blockedby:
Patch: 1 | Blocking:
------------------------------------+---------------------------------------
Changes (by vadz):

* cc: vaclavslavik (added)
* milestone: => 2.9.5


Comment:

Thanks, I definitely agree it's a useful feature to have, I often use bold
font for some XRC controls too and it would be better to do using this
syntax rather than hardcoding some system font.

I'm not sure about all this `XRC_MAKE_INSTANCE()` stuff though, Vaclav
could you please take a look at the patch?

Also, sodev[*], if you could please add a demo of the new feature to the
xrc sample, e.g. just use the new attribute in `controls.xrc` for some
control to give it a bold label, it would be great. TIA!

----

[*] I don't know your real name, please let me know (here or in private
email, as you prefer) how would you like to be credited for this patch in
the change log if I should use anything else than "sodev".


--
Ticket URL: <http://trac.wxwidgets.org/ticket/14632#comment:1>

wxTrac

unread,
Sep 19, 2012, 10:46:36 AM9/19/12
to wx-...@googlegroups.com
Ticket URL: <http://trac.wxwidgets.org/ticket/14632#comment:2>

#14632: XRC derive font from enclosing control
------------------------------------+---------------------------------------
Reporter: sodev | Owner:
Type: enhancement | Status: new
Priority: normal | Milestone: 2.9.5
Component: XRC | Version: 2.9-svn
Keywords: xrc wxFont inheritance | Blockedby:
Patch: 1 | Blocking:
------------------------------------+---------------------------------------

Comment(by vaclavslavik):

`XRC_MAKE_INSTANCE` does as it says: it ''creates an instance''.
`m_instance` is a ''pre-created'' instance that overrides this creation
when you load an XRC resource into an existing object. So setting
`m_instance` explicitly is not the right thing to do. The way you did it,
this `<inherit>` attribute would only work with toggle buttons and custom
controls (unless I'm missing something) -- and even if you updated all the
handlers in wx, it would still be broken for custom user handlers.

Instead, I'd add an optional `wxWindow*` argument to `GetFont()` and use
that (reporting an error if it's NULL and `<inherit>` is provided). Then
modify `SetupWindow()` -- which is the function used by all handlers to
set these things up -- to pass the window to `SetFont()`.


--
Ticket URL: <http://trac.wxwidgets.org/ticket/14632#comment:2>

wxTrac

unread,
Sep 21, 2012, 4:30:12 PM9/21/12
to wx-...@googlegroups.com
Ticket URL: <http://trac.wxwidgets.org/ticket/14632#comment:3>

#14632: XRC derive font from enclosing control
------------------------------------+---------------------------------------
Reporter: sodev | Owner:
Type: enhancement | Status: new
Priority: normal | Milestone: 2.9.5
Component: XRC | Version: 2.9-svn
Keywords: xrc wxFont inheritance | Blockedby:
Patch: 1 | Blocking:
------------------------------------+---------------------------------------

Comment(by sodev):

Yes, you are missing something, the toggle button and unknown control
handlers are the only ones that do not use the XRC_MAKE_INSTANCE macro so
i had to patch them additionally, every handler that uses
XRC_MAKE_INSTANCE to create the object works without modification ;).

Compatibility with existing XRC files and handlers were my main goal, i
thought about that extra parameter for {{{GetFont()}}} as well, but if any
handler uses it on its own it needs to be changed to actually use this new
feature. I looked around the sources to understand how the xrc handlers
work and what all the members are for and {{{m_instance}}} catched my
attention. If it is non-NULL XRC_MAKE_INSTANCE simply returns it, however
if its not, it creates a new object and returns that pointer. Now the
problem is that this new object can only be accessed locally at the
location where that macro is used because the pointer is nowhere stored in
the handler object.

So if i supply a pre-created object to the handler the whole handler can
access it and even my GetFont()-modification works out of the box, however
if the object gets created no part of the handler has access to it but the
function that actually created it. I don't see any reason why the handler
shouldn't be able to access the created object, actually it looks like an
error to me that it cannot access that new object from other parts in that
case.

I checked all handlers in the source tree if they do something with
{{{m_instance}}}, but all they do is check if it's non-NULL to create a
new object instead of using it. And actually i found another location that
does modify {{{m_instance}}}:
{{{wxXmlResourceHandler::CreateResource(wxXmlNode*, wxObject*,
wxObject*)}}}. If you use subclassing it stores the created object
directly in it. So if any custom handler relies on the fact that
{{{m_instance}}} is never populated if the object is pre-created they
break if you use subclassing ;). So i think it is a symmetric solution to
let {{{m_instance}}} always point to the object that the handler is
processing, if it was pre-created or if it was created on demand.
----
My name, Steffen Olszewski, is no secret at all, i am signed up with it
here and was even a little surprised that it wasn't used for my
wxThreadEvent patch previously. Also my boss has no problem at all if our
company, Gero Meßsysteme GmbH, gets mentioned as well ;).

I'm going to add another patch for the examples next week.


--
Ticket URL: <http://trac.wxwidgets.org/ticket/14632#comment:3>

wxTrac

unread,
Sep 22, 2012, 3:55:05 AM9/22/12
to wx-...@googlegroups.com
Ticket URL: <http://trac.wxwidgets.org/ticket/14632#comment:4>

#14632: XRC derive font from enclosing control
------------------------------------+---------------------------------------
Reporter: sodev | Owner:
Type: enhancement | Status: new
Priority: normal | Milestone: 2.9.5
Component: XRC | Version: 2.9-svn
Keywords: xrc wxFont inheritance | Blockedby:
Patch: 1 | Blocking:
------------------------------------+---------------------------------------

Comment(by vaclavslavik):

Replying to [comment:3 sodev]:
> Yes, you are missing something, the toggle button and unknown control
handlers are the only ones that do not use the XRC_MAKE_INSTANCE macro so
i had to patch them additionally, every handler that uses
XRC_MAKE_INSTANCE to create the object works without modification ;).

Ah, I overlooked that. My point still stands, though: do not set
`m_instance` in handlers, manually or via `XRC_MAKE_INSTANCE`.

> Now the problem is that this new object can only be accessed locally at
the location where that macro is used because the pointer is nowhere
stored in the handler object.

That's not a "problem", that's ''by design''. The handler is long-lived
object, it's called repeatedly to create possibly many controls.
`DoCreateResource()` should not alter the handler's state and should in
fact be `const`.

> So if i supply a pre-created object to the handler the whole handler can
access it and even my GetFont()-modification works out of the box, however
if the object gets created no part of the handler has access to it but the
function that actually created it. I don't see any reason why the handler
shouldn't be able to access the created object, actually it looks like an
error to me that it cannot access that new object from other parts in that
case.

The only error in the design here was the very existence of `m_instance`,
this pre-created object should have been passed to `DoCreateResource()`
directly as an optional argument. But it's too late to change that and we
are stuck with optional data being side-passed to `DoCreateResource()` as
a temporary modification of the handler's state.

And the reason why `GetFont()` shouldn't look at `m_instance` is
readability and obviousness: it's not at all clear that the code calling
`GetFont()` has to update `m_instance` first. Not doing so is a very easy
mistake to make and overlook. OTOH, a documented (and auto-completed by
IDEs...) optional argument to `GetFont()` is clear. And it would work
(again, unlike relying on `m_instance`) even for handlers that needed to
create more windows (e.g. for some dynamically constructed composite
control).

> I checked all handlers in the source tree if they do something with
{{{m_instance}}}, but all they do is check if it's non-NULL to create a
new object instead of using it. And actually i found another location that
does modify {{{m_instance}}}:
{{{wxXmlResourceHandler::CreateResource(wxXmlNode*, wxObject*,
wxObject*)}}}.

Obviously, ''something'' has to set it. This is the one and only place,
internal to XRC, that should do it.

> So i think it is a symmetric solution to let {{{m_instance}}} always
point to the object that the handler is processing, if it was pre-created
or if it was created on demand.

See above, `m_instance` is an input value.


--
Ticket URL: <http://trac.wxwidgets.org/ticket/14632#comment:4>

wxTrac

unread,
Sep 27, 2012, 4:19:19 PM9/27/12
to wx-...@googlegroups.com
Ticket URL: <http://trac.wxwidgets.org/ticket/14632#comment:5>

#14632: XRC derive font from enclosing control
------------------------------------+---------------------------------------
Reporter: sodev | Owner:
Type: enhancement | Status: new
Priority: normal | Milestone: 2.9.5
Component: XRC | Version: 2.9-svn
Keywords: xrc wxFont inheritance | Blockedby:
Patch: 1 | Blocking:
------------------------------------+---------------------------------------

Comment(by sodev):

As i said, i didnt't want to change the public interface because i wanted
to ensure that existing handlers can use this feature without the need to
be updated, so i tried to modify the existing implementation to just do
the right things to get this done on its own. Because there is no
documentation about the internals i had to figure out how the things work
by looking at existing code. The similarities in the naming and because
some other handlers do modify their internal state to e.g. keep track what
to do when they call themselves recursive (like wxSizerXmlHandler) it
looked like a good solution.

However, you are right, there is no rule to use this macro and most of my
own handlers in fact don't use it and i had to change them to get it
working, so i've modified the patch to actually use a parameter for
{{{GetFont()}}} now. I've checked all handlers in the source tree and only
one needed to be adjusted to use this new feature. Because the xrc
controls sample did not contain a section to show how these generic
features work i modified the wxStaticText part to create multiple controls
which show the possible different results depending on how the font was
created. I'm not that happy about this solution but for now it serves the
purpose.

The v2 patch contains all my changes, the feature itself, xrc format
documentation, updated affected handler and the updated sample, it is made
against trunk instead of root like the previous ones.


--
Ticket URL: <http://trac.wxwidgets.org/ticket/14632#comment:5>

wxTrac

unread,
Sep 28, 2012, 7:02:40 PM9/28/12
to wx-...@googlegroups.com
Ticket URL: <http://trac.wxwidgets.org/ticket/14632#comment:6>

#14632: XRC derive font from enclosing control
------------------------------------+---------------------------------------
Reporter: sodev | Owner:
Type: enhancement | Status: new
Priority: normal | Milestone: 2.9.5
Component: XRC | Version: 2.9-svn
Keywords: xrc wxFont inheritance | Blockedby:
Patch: 1 | Blocking:
------------------------------------+---------------------------------------

Comment(by vadz):

Thanks for the update!

I'm not sure if it's a good idea to silently ignore "inherit" if "sysfont"
is also specified, perhaps it would be better to give an error in this
case?

But other than that the patch looks good to me and I'm going to commit it.
If you agree with the proposed change above, please submit another patch
and I'll apply it on top of it.

Thanks again!


--
Ticket URL: <http://trac.wxwidgets.org/ticket/14632#comment:6>

wxTrac

unread,
Sep 28, 2012, 7:48:36 PM9/28/12
to wx-...@googlegroups.com
Ticket URL: <http://trac.wxwidgets.org/ticket/14632#comment:7>

#14632: XRC derive font from enclosing control
--------------------------+-------------------------------------------------
Reporter: sodev | Owner:
Type: enhancement | Status: closed
Priority: normal | Milestone: 2.9.5
Component: XRC | Version: 2.9-svn
Resolution: fixed | Keywords: xrc wxFont inheritance
Blockedby: | Patch: 1
Blocking: |
--------------------------+-------------------------------------------------
Changes (by VZ):

* status: new => closed
* resolution: => fixed


Comment:

(In [72572]) Add "inherit" to <font> XRC tag.

This allows to construct a font based on the parent window font instead of
either fully specifying all font parameters or basing it on a standard
font.

Closes #14632.


--
Ticket URL: <http://trac.wxwidgets.org/ticket/14632#comment:7>

wxTrac

unread,
Oct 5, 2012, 12:45:24 PM10/5/12
to wx-...@googlegroups.com
Ticket URL: <http://trac.wxwidgets.org/ticket/14632#comment:8>

#14632: XRC derive font from enclosing control
--------------------------+-------------------------------------------------
Reporter: sodev | Owner:
Type: enhancement | Status: closed
Priority: normal | Milestone: 2.9.5
Component: XRC | Version: 2.9-svn
Resolution: fixed | Keywords: xrc wxFont inheritance
Blockedby: | Patch: 1
Blocking: |
--------------------------+-------------------------------------------------

Comment(by sodev):

Thanks for integrating this so fast. Double specification of "inherit" and
"sysfont" are not the only undetected errors, the same applies to "size"
and "relativesize", invalid weight and a few others, so i just followed
the general style to not notify about these errors. Currently i'm busy
with another project, but i might submit additional patches to address
these things among some other small problems i discovered using the XRC
system.


--
Ticket URL: <http://trac.wxwidgets.org/ticket/14632#comment:8>

wxTrac

unread,
Oct 6, 2012, 5:37:45 PM10/6/12
to wx-...@googlegroups.com
Ticket URL: <http://trac.wxwidgets.org/ticket/14632#comment:9>

#14632: XRC derive font from enclosing control
--------------------------+-------------------------------------------------
Reporter: sodev | Owner:
Type: enhancement | Status: closed
Priority: normal | Milestone: 2.9.5
Component: XRC | Version: 2.9-svn
Resolution: fixed | Keywords: xrc wxFont inheritance
Blockedby: | Patch: 1
Blocking: |
--------------------------+-------------------------------------------------

Comment(by vadz):

More robustness is always good so don't hesitate to fix this and related
problems. TIA!


--
Ticket URL: <http://trac.wxwidgets.org/ticket/14632#comment:9>
Reply all
Reply to author
Forward
0 new messages