On 08/07/2012 02:03 PM, Scott Moreau wrote:
Hi Ander,
On Tue, Aug 7, 2012 at 2:57 AM, Ander Conselvan de Oliveira
<conselv...@gmail.com <mailto:conselv...@gmail.com>> wrote:
What exactly does this fix? Your commit message doesn't make it that
obvious.
I assumed the problem was obvious. The definition of the word opaque
makes it clear. In frame_button_redraw_handler() the code returns (i.e.
does not draw the buttons) if widget->opaque is set. It should be the
other way around.
Weston commit 010f98b0 added the opaque field to struct widget to
"track and report [...] opaque regions", i.e., the region of a
surface whose alpha channel is 1.
I don't think this commit is relevant here. What is relevant is the
meaning of opaque.
My point exactly. The reason I pointed this commit out is because it
introduced the opaque field with the purpose of tracking the opaque
region of a window.
The protocol defines the opaque region as "the region of the surface
that contain opaque content. The opaque region is an optimization hint
for the compositor that lets it optimize out redrawing of content behind
opaque regions. Setting an opaque region is not required for correct
behaviour, but marking transparent content as opaque will result in
repaint artifacts."
So if the opaque field is 0 for a given widget it does *not* mean that
this widget is hidden or invisible. It only means that the contents
might contain pixels whose alpha component is different than one. In
other words, if a surface is opaque its contents obscure whatever is
below them while otherwise alpha blending is needed.
Later the frame_button code started to use opaque as a visibility field.
(since cgit.fd.o is down at the moment I will post
some snippets) For widget_set_transparent() you have
widget_set_transparent(struct widget *widget, int transparent)
{
widget->opaque = !transparent;
}
It seems to me the frame button code is abusing this field to hide
the buttons in fullscreen mode.
This may be the case but see window.c~:1300 frame_resize_handler()
if (child->opaque) {
widget->window->opaque_region =
wl_compositor_create_region(display->compositor);
wl_region_add(widget->window->opaque_region,
opaque_margin, opaque_margin,
widget->allocation.width - 2 * opaque_margin,
widget->allocation.height - 2 * opaque_margin);
}
it adds the region if opaque is set.
IMO, either way you define a visibility value based on the opacity
value results in improper usage.
I did not write this code, I'm just building on top of it while trying
to improve it.
My point was that since opacity and visibility are not the same thing,
defining visibility as opaque == 0 or opaque == 1 is equally wrong. Your
patch doesn't change any behavior, it only changes if we go with the
former or the latter.
If you have a better idea, I would be interested to know
what you think.
One simple solution is to add a visibility field to struct frame_button.
Or have a general visibility switch on every widget.
Best regards,
Ander
_______________________________________________
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel
