Re: gtk viewport unrealize



hi owen,

I apologize for the late reply but I was away on vacation.

On Mon, 2004-07-05 at 18:39, Owen Taylor wrote: 
> On Fri, 2004-06-18 at 14:47, Mathieu Lacage wrote:
> > hi,
> > 
> > While debugging some open gl problems of mine, I stumbled upon
> > gtk_viewport_unrealize which destroys its windows before chaining up to
> > its parent. The chain up ends in gtk_widget_real_unrealize where a big
> > comment states that the unrealize handler of the children must be
> > invoked before that of the parent. To enforce this, it does an explicit
> > test whether or not it is a container and then calls each of the
> > container's children unrealize handler.
> > 
> > I wonder if it would not make sense to change the way the
> > gtk_viewport_unrealize handler works to make sure it chains up before
> > destroying its internal windows. 
> > 
> > This seems to fix a lot of problems in my opengl drawing area which is
> > stored in a viewport...
> 
> Hmm,
> 
>  - The comment is pretty old; we tend to be more robust these days;
>    but it's not unlikely that the OpenGL wrapper code doesn't have the
>    necessary checks to prevent bad things when operating on destroyed
>    windows. (They should be still referenced, just that the X 
>    counterpart is gone. GDK is full of GDK_WINDOW_DESTROYED() checks.)

Well, the lack of checks is more often in the DRI drivers: some of them
are particularly robust to such issues. Others don't do much checking
and happily crash the whole server/machine when feeding invalid window
ids.

>  - I wouldn't want to change this on the stable branch; I think there
>    is considerable potential for unexpected breakage; I can't think
>    of a way off-hand that people would be depending on the current
>    order, but that doesn't mean there isn't one.

agreed.

>  - It would need to be done as an audit across all !NO_WINDOW containers
>    (there aren't very many - GtkViewport, GtkTreeView, GtkTextView,
>    GtkLayout, probably a few more.) Changing it in just one place 
>    strikes me as a poor.

yes.

>  - The issue should be put in bugzilla.

will do.


After a bit of thinking about the issue, I just can't really convince
myself that the proposed solution (the switch of the chainup in each
!NO_WINDOW container) is the right solution. It seems to me that it is
fundamentally wrong to try to chainup before cleaning up in the
container. And the only reason for this is because we want to inherit
the implementation of the parent container which unrealizes the
container children in a loop.

It strikes me that it might make much more sense to move the child
unrealize loop from the gtk_widget_unrealize function to each
container's unrealize handler and then to chain up. A nice side-effect
of such a change would be to remove the ugly comment/code from
gtk_widget_unrealize. A not-so-nice side effect would be to duplicate
the child unrealize loop but I fail to see how that would be really bad.
I could commit to provide such a patch within a week or so.

What do you think ?

regards,
Mathieu
-- 
Mathieu Lacage <mathieu gnu org>




[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]