Re: MenuStyle
On Thu, Oct 20, 2005 at 07:42:36AM +0200, Viktor Griph wrote:
Hi
I am looking into the MenuStyle code trying to understand how some things
are done. I plan to enable negation of all on/off menu styles by prefixing
! to them. However looking at the code raise some questions:
First of all what exactly does FreeColors free up?
In certain modes, X colours are managed in so called colour cells.
There may be a limited number of available cells. They are
reserved with XAllocColor and freed with XFreeColor. Because
colours are used in many places inside fvwm (colour sets!), fvwm
keeps a reference counter for the allocated colours. The
corresponding code may be a bit difficult to understand
(libs/PictureUtils.c).
FreeColors eventually decreases the reference counters and frees
the cells if the reference count drops to 0. Actually the process
is more complicated. I think Olivier has written most of the
code.
Answering this might
answe some of my other questions; at least if it has some special
functionality.
Second: The styles HilightBack and ActiveFore do free the colors if the
style have them. Howver the corresponding off-styles does not free them.
Is there any reason for this?
No, it's a bug. Both must free these colours also. Note that
nobody ever actually tested the code to free colours. I'm sure
fvwm still has many colour leaks.
Last: This can't be right can it?:
-- from menustyle.c: menustyle_copy
/* HilightBack */
if (ST_HAS_ACTIVE_BACK(destms))
{
FreeColors(ST_MENU_ACTIVE_COLORS(destms).back, 1, True);
}
ST_HAS_ACTIVE_BACK(destms) = ST_HAS_ACTIVE_BACK(origms);
memcpy(ST_MENU_ACTIVE_COLORS(destms),
ST_MENU_ACTIVE_COLORS(origms), sizeof(ColorPair));
ST_DO_HILIGHT_BACK(destms) = ST_DO_HILIGHT_BACK(origms);
/* ActiveFore */
if (ST_HAS_ACTIVE_FORE(destms))
{
FreeColors(ST_MENU_ACTIVE_COLORS(destms).fore, 1, True);
}
--
It looks to me as it frees the active fore color after having it copied
over, thus freeing the source's color instead of the overwritten.
Yes, without checking what the ST_MENU_ACTIVE_COLORS actually are,
I think you're right.
Furthermore I think the colour reference counts have to be
increased when a menustyle is copied. Consider this:
* Menustyle A has a colour x with reference count 1.
* Copy menustyle A to some other menustyle B. x still has a
reference count of 1.
* Copy A over b again. The reference count of x is decreased to
0 and never increased again. Ouch.
We should have a CopyColor function somewhere in PictureUtils.c
that takes care of this situation, e.g.
CopyColor(
sometype* dest_color, sometype* src_color,
int do_free_dest_color);
(where do_free_dest_color indicates whether the destination
colour should be freed).
Ciao
Dominik ^_^ ^_^
--
Dominik Vogt, [EMAIL PROTECTED]
signature.asc
Description: Digital signature
Re: MenuStyle
On Thu, Oct 20, 2005 at 11:06:36AM +0200, Dominik Vogt wrote: On Thu, Oct 20, 2005 at 07:42:36AM +0200, Viktor Griph wrote: Hi I am looking into the MenuStyle code trying to understand how some things are done. I plan to enable negation of all on/off menu styles by prefixing ! to them. However looking at the code raise some questions: First of all what exactly does FreeColors free up? In certain modes, X colours are managed in so called colour cells. There may be a limited number of available cells. They are reserved with XAllocColor and freed with XFreeColor. Because colours are used in many places inside fvwm (colour sets!), fvwm keeps a reference counter for the allocated colours. The corresponding code may be a bit difficult to understand (libs/PictureUtils.c). FreeColors eventually decreases the reference counters and frees the cells if the reference count drops to 0. Actually the process is more complicated. I think Olivier has written most of the code. Yes, the code is complex. It allows, in particular, to save in a fast way the allocated pixels for an image or a gradient with minimal memory usage. Before that, colours leak was systematic with gradients and images. Any way, colours management (sharing) at depth 8 is a mess in the X world, I discourage any one to try to understand the problems :o) Note that we do not receive complaint anymore on colours at depth 8 on fvwm-users. Maybe, no body have to use anymore these applications which need depth 8 to work properly. Olivier
Re: MenuStyle
On Thu, 20 Oct 2005, Dominik Vogt wrote:
On Thu, Oct 20, 2005 at 07:42:36AM +0200, Viktor Griph wrote:
Hi
I am looking into the MenuStyle code trying to understand how some things
are done. I plan to enable negation of all on/off menu styles by prefixing
! to them. However looking at the code raise some questions:
First of all what exactly does FreeColors free up?
In certain modes, X colours are managed in so called colour cells.
There may be a limited number of available cells. They are
reserved with XAllocColor and freed with XFreeColor. Because
colours are used in many places inside fvwm (colour sets!), fvwm
keeps a reference counter for the allocated colours. The
corresponding code may be a bit difficult to understand
(libs/PictureUtils.c).
FreeColors eventually decreases the reference counters and frees
the cells if the reference count drops to 0. Actually the process
is more complicated. I think Olivier has written most of the
code.
Answering this might
answe some of my other questions; at least if it has some special
functionality.
Second: The styles HilightBack and ActiveFore do free the colors if the
style have them. Howver the corresponding off-styles does not free them.
Is there any reason for this?
No, it's a bug. Both must free these colours also. Note that
nobody ever actually tested the code to free colours. I'm sure
fvwm still has many colour leaks.
Last: This can't be right can it?:
-- from menustyle.c: menustyle_copy
/* HilightBack */
if (ST_HAS_ACTIVE_BACK(destms))
{
FreeColors(ST_MENU_ACTIVE_COLORS(destms).back, 1, True);
}
ST_HAS_ACTIVE_BACK(destms) = ST_HAS_ACTIVE_BACK(origms);
memcpy(ST_MENU_ACTIVE_COLORS(destms),
ST_MENU_ACTIVE_COLORS(origms), sizeof(ColorPair));
ST_DO_HILIGHT_BACK(destms) = ST_DO_HILIGHT_BACK(origms);
/* ActiveFore */
if (ST_HAS_ACTIVE_FORE(destms))
{
FreeColors(ST_MENU_ACTIVE_COLORS(destms).fore, 1, True);
}
--
It looks to me as it frees the active fore color after having it copied
over, thus freeing the source's color instead of the overwritten.
Yes, without checking what the ST_MENU_ACTIVE_COLORS actually are,
I think you're right.
Furthermore I think the colour reference counts have to be
increased when a menustyle is copied. Consider this:
* Menustyle A has a colour x with reference count 1.
* Copy menustyle A to some other menustyle B. x still has a
reference count of 1.
* Copy A over b again. The reference count of x is decreased to
0 and never increased again. Ouch.
We should have a CopyColor function somewhere in PictureUtils.c
that takes care of this situation, e.g.
CopyColor(
sometype* dest_color, sometype* src_color,
int do_free_dest_color);
(where do_free_dest_color indicates whether the destination
colour should be freed).
I found Pixel fvwmlib_clone_color(Pixel p) in ColorUtils.c which can be
used to reallocate a color. With that creating a CopyColor function is no
problem.
Ciao
Dominik ^_^ ^_^
--
Dominik Vogt, [EMAIL PROTECTED]
