[PATCH xserver] randr: Stop dirty tracking for shared pixmap being destroyed

[Alex Goins]

> > Are you sure about this? rrCreateSharedPixmap() does not actually start pixmap
> > tracking, rather it is done later in rrSetupPixmapSharing(). It's not a given
> > that a 'shared pixmap' is being tracked when it is destroyed.
> 
> That's fine, StopPixmapTracking is just a no-op in that case.

Sure, if you want to depend on edge cases in an external driver. Seems to me
like it would be best to do the right thing inside the modesetting driver
instead of making unnecessary assumptions about the source driver's behavior.

> > The regression was introduced when that patch was taken by itself without
> > subsequent patches, didn't notice the bug since I was mostly testing with all
> > the patches together.
> 
> FWIW, patches should always be as self-contained as possible even as
> part of a series. In particular, introducing a regression like this in
> one patch and fixing it in a subsequent patch of a series is no good.

Yeah, sorry; it was a mistake when refactoring the patches for public
consumption. I'll test each patch individually next time around. Learning as I
go.

> > A better solution would be to re-introduce the master->StopPixmapTracking() call
> > in front of rrDestroySharedPixmap() in RRCrtcDetachScanoutPixmap().
> 
> That would leave the risk of StopPixmapTracking never getting called,
> leaving a dangling reference to the destroyed pixmap.

How so? The only two places it gets called (until the rest of my patches go in)
are in rrCreateSharedPixmap() and RRCrtcDetachScanoutPixmap(). In the former
case, it currently adds an extraneous call to StopPixmapTracking(), and in the
latter case, moving StopPixmapTracking() would cause no functional difference.

Semantically, it doesn't really make sense to do it this way, especially
considering that the whole point of factoring it out was to make way for sharing
multiple pixmaps tracking the same master screen pixmap. In that case, you don't
want to stop pixmap tracking on a pixmap-by-pixmap basis.

Anyway, it should be apparent why it's a problem in the context of my other
patches. I'll just fix it up in the next patch set.

Thanks,
Alex