[PATCH xserver 2/3] present: Factor code for restoring screen pixmap out of present_unflip
michel at daenzer.net
Mon Feb 22 09:18:12 UTC 2016
On 19.02.2016 14:31, Keith Packard wrote:
> Michel Dänzer <michel at daenzer.net> writes:
> Here's another review, now that I've more carefully looked at the other
> two patches.
>> + if (!flip_window || !flip_pixmap)
>> + return;
> In the original code, this didn't abort the operation entirely...
> This operation would have been skipped:
>> + present_set_tree_pixmap(flip_window, flip_pixmap, screen_pixmap);
> But this operation would not:
>> + present_set_tree_pixmap(screen->root, NULL, screen_pixmap);
BTW, it might be safer to make this
present_set_tree_pixmap(screen->root, flip_pixmap, screen_pixmap);
as well, so the root window pixmap is only changed to the screen pixmap
if it's currently the flip pixmap? Anyway, this can be done in a
followup, since the current call with NULL doesn't seem to cause any
problem in practice.
> Reading the rest of the code, I can't find a case which could call this
> with a null flip_window or flip_pixmap;
While I couldn't either just looking at present.c, I can get it called
with flip_window == NULL when pressing Escape while glxgears is flipping
in fullscreen, presumably due to present_clear_window_flip.
I'm not sure the present_copy_region or
present_set_tree_pixmap(screen->root, ...) calls will ever have any
actual effect in this case, but for now let's preserve the logic of the
refactored code wrt this.
Earthling Michel Dänzer | http://www.amd.com
Libre software enthusiast | Mesa and X developer
More information about the xorg-devel