Merge lp://qastaging/~alan-griffiths/compiz-core/refactor-Display-from-PrivateScreen into lp://qastaging/compiz-core

I suggest we defer this one for a while. Especially since the last privatescreen tidy up caused a few regressions.

At the very least, we'll need to ensure that compiz-plugins-* and libcompizconfig still work before approving this.

Also, three Screen classes in core is a bit troublesome. We should be looking at how to get back to only two. Maybe merge PrivateScreen+CompScreenImpl.

> I suggest we defer this one for a while. Especially since the last
> privatescreen tidy up caused a few regressions.
>
> At the very least, we'll need to ensure that compiz-plugins-* and
> libcompizconfig still work before approving this.

I'm conscious of problems caused by the previous tidy up and this time have contented myself with segregating code into more coherent areas.

> Also, three Screen classes in core is a bit troublesome. We should be looking
> at how to get back to only two. Maybe merge PrivateScreen+CompScreenImpl.

I think the problem is quite otherwise.

The code spread across CompScreen/Impl/private lacks coherency (it performs a number of unrelated functions). If these were split out into smaller, coherent, classes these would be easier to validate and interactions easier to predict.

Where I'm aiming for is a CompScreen interface and an implementation that contains classes to manage individual functional areas. This commit is a step in that direction.

The reason for keeping PrivateScreen at present is for API compatibility - for existing code that does access ::screen->priv->XXXX - I restrained myself from dumping it all into CompScreenImpl.

> > Also, three Screen classes in core is a bit troublesome. We should be
> looking
> > at how to get back to only two. Maybe merge PrivateScreen+CompScreenImpl.
>
> I think the problem is quite otherwise.
>
> The code spread across CompScreen/Impl/private lacks coherency (it performs a
> number of unrelated functions). If these were split out into smaller,
> coherent, classes these would be easier to validate and interactions easier to
> predict.
>
> Where I'm aiming for is a CompScreen interface and an implementation that
> contains classes to manage individual functional areas. This commit is a step
> in that direction.

+1

I'd like for us all to keep this design goal in mind. CompWindow and CompScreen are monstrosities - even if we can specify a number of different interfaces they provide, this is far better for testing and refactoring then coding to CompScreen and friends.

3746 + // >>>>>>>>>> WEIRD STUFF >>>>>>>>>>
3747 + // TODO - unimplemented and unused?
3748 + void updatePassiveGrabs ();

It has been replaced by updatePassiveKeyGrabs and can be removed

3749 + // TODO - unimplemented and unused?
3750 + bool createFailed ();

We should replace this with exceptions.

3751 + // TODO - unused?
3752 + std::map<CompString, CompPrivate> valueMap;

99% sure that ValueHolder replaced this one (although we need to convert ValueHolder from being a singleton to being an interface)(

3753 + // TODO - unused?
3754 + xcb_connection_t *connection;

Yep, it can go too (although I really would love to use xcb at some point in the future)

3755 + // TODO - unused?
3756 + XRectangle lastViewport;

Can also go

3757 + // <<<<<<<<<< WEIRD STUFF <<<<<<<<<<
3758 +

> 3746 + // >>>>>>>>>> WEIRD STUFF >>>>>>>>>>
> 3747 + // TODO - unimplemented and unused?
> 3748 + void updatePassiveGrabs ();
>
> It has been replaced by updatePassiveKeyGrabs and can be removed
>
> 3749 + // TODO - unimplemented and unused?
> 3750 + bool createFailed ();
>
> We should replace this with exceptions.
>
> 3751 + // TODO - unused?
> 3752 + std::map<CompString, CompPrivate> valueMap;
>
> 99% sure that ValueHolder replaced this one (although we need to convert
> ValueHolder from being a singleton to being an interface)(
>
> 3753 + // TODO - unused?
> 3754 + xcb_connection_t *connection;
>
> Yep, it can go too (although I really would love to use xcb at some point in
> the future)
>
> 3755 + // TODO - unused?
> 3756 + XRectangle lastViewport;
>
> Can also go
>
> 3757 + // <<<<<<<<<< WEIRD STUFF <<<<<<<<<<
> 3758 +

Thanks Sam.

> > Where I'm aiming for is a CompScreen interface and an implementation that
> > contains classes to manage individual functional areas. This commit is a
> step
> > in that direction.
>
> +1
>
> I'd like for us all to keep this design goal in mind. CompWindow and
> CompScreen are monstrosities - even if we can specify a number of different
> interfaces they provide, this is far better for testing and refactoring then
> coding to CompScreen and friends.

Having done this exercise once I think I can get to the end result with less scary diffs. I'll create another branch and submit that.