[haiku-appserver] Re: [Haiku-commits] r12953 - haiku/trunk/src/servers/app

Hi Stephan,

        Have a few observations. They are not crucial but they are important as 
they are related to overall design.

Stephan 
Aïïïïïïïïïïïïïïïïïïïïïïïïïïïïïïïïïïïïïïïïï
 wrote:
>  {
> -     // TODO: Decorator class should have a method witch
> -     // returns the minimum frame width.
> -     if (!frame.IsValid())
> -             fFrame.Set(0, 0, 5, 5);
> +     if (!frame.IsValid()) {
> +char helper[1024];
> +sprintf(helper, "Layer::Layer(BRect(%.1f, %.1f, %.1f, %.1f), name: %s, 
> token: %ld) - frame is invalid\n",
> +             frame.left, frame.top, frame.right, frame.bottom, name, token);
> +CRITICAL(helper);
> +             fFrame.Set(0, 0, 10, 10);
> +     }

        Here, we really need a method that just does what I wrote in the TODO 
you deleted. Don't think about "be" decorator only.

>  Layer::Show(bool invalidate)
>  {
>       STRACE(("Layer(%s)::Show()\n", GetName()));
> -     if( !IsHidden() )
> +     if(!IsHidden())
>               return;
>       
>       fHidden = false;
> -     
> -     if(invalidate)
> +
> +// NOTE: I added this here and it solves the invalid region problem
> +// for Windows that have been resized before they were shown. -Stephan
> +RebuildFullRegion();
> +SendViewCoordUpdateMsg();
> +
> +     if (invalidate)
>               GetRootLayer()->GoInvalidate(this, fFull);
>  }

        You should, under no circumstances call RebuildFullRegion() from 
another thread than RootLayer's one. I can guarantee this can have very 
ugly consequences. For example try moving a window while you show/hide 
another.
        The current design implies all clipping operations to be done from one 
thread only(they are not protected by a semaphore). In the future, when 
we'll be having double buffering, maybe it won't be the case anymore as 
we can accomplish that in a ServerWindow thread, and RootLayer be only a 
gatherer and present the final scene. (we'll talk about this someday).

        
> -             resize_layer(x, y);
> +             if (IsHidden()) {
> +                     // TODO: See large comment in MoveBy()
> +                     fFrame.right += x;
> +                     fFrame.bottom += y;
>  
> +                     fTopLayer->resize_layer(x, y);
> +             } else {
> +                     resize_layer(x, y);
> +             }
> +
>               if (Window()) {
>                       // send a message to the client informing about the 
> changed size
>                       BRect frame(fTopLayer->Frame());

        Same here.


thanks,
Adi.

Other related posts: