Re: Bug in kwin (kde2)

["Valeriy E. Ushakov" <uwe ptc spbu ru>]

On Thu, Nov 16, 2000 at 18:29:56 +0100, Matthias Ettrich wrote:

> > The problem seems to be in kwin/client.cpp (verified to exist in 1.192):
> >
> >     WinInfo::changeState(unsigned long state, unsigned long mask)
> >
> > checks for
> >
> >     if ( state & NET::Max )
> >
> > instead of mask & NET::Max
> >
> > and if the request is to clear the flag (either with explicit clear by
> > toggling it off) the request is ignored since state do NOT have the
> > bit set (we are clearing it) - the mask has.
> 
> It's not a bug in kwin. The NETWM implementation should handle the
> how flag and pass the proper mask/state on to the application.

I haven't actually debugged that code but consider in kwin/client.cpp

    virtual void changeState( unsigned long state, unsigned long mask) {
        state &= mask;
	// ...
        if ( state & NET::Max ) {
            if ( (state & NET::Max) == NET::Max )
                m_client->maximize( Client::MaximizeFull );
            else if ( state & NET::MaxVert )
                m_client->maximize( Client::MaximizeVertical );
            else if ( state & NET::MaxHoriz )
                m_client->maximize( Client::MaximizeHorizontal );
            else
                m_client->maximize( Client::MaximizeRestore );
        }
	// ...
    }

Say, for demaximization the passed values are

    state = 0;	// we want bits in the mask to be cleared in the state
    mask = NET::MaxVert | NET::MaxHoriz;

So the test if ( state & NET::Max ) is false and nothing happens.

The logic, it seems, should be something like (untested):

        if ( mask & NET::Max ) {
	    if ( (mask & NET::Max) == NET:Max) {
	        // request fully describes new state
	        bool vert = state & NET::MaxVert;
	        bool horiz = state & NET::MaxHoriz;
		if ( vert && horiz )
		    m_client->maximize( Client::MaximizeFull );
		else if ( vert )
		    m_client->maximize( Client::MaximizeVertical );
		else if ( horiz )
		    m_client->maximize( Client::MaximizeHorizontal );
		else
		    m_client->maximize( Client::MaximizeRestore );
            }
	    else if ( (mask & Net::MaxVert) ) {
		// request only specifies state for one direction
		// need to merge with current state
	        if ( state & NET::MaxVert ) {
		    // XXX: I'm making up API here, since I don't know it
		    if ( m_client->isMaxHoriz() )
		        m_client->maximize( Client::MaximizeFull );
		    else
		        m_client->maximize( Client::MaximizeVertical );
		}
		else {
		    if ( m_client->isMaxHoriz() )
		        m_client->maximize( Client::MaximizeVertical );
		    else
		        m_client->maximize( Client::MaximizeRestore );
                }	    
	    }
	    else if ( (mask & Net::MaxHoriz) ) {
	        // same logic for horizontal
	    }

I don't know KDE internals, so this might be wrong, still the
existing code confuses mask and state and need to be fixed.

SY, Uwe
-- 
uwe ptc spbu ru                         |       Zu Grunde kommen
http://www.ptc.spbu.ru/~uwe/            |       Ist zu Grunde gehen