On Fri, 9 Dec 2016 17:27:13 +0000 Daniel Stone <[email protected]> wrote:
> Hi, > > On 9 December 2016 at 16:35, Daniel Stone <[email protected]> wrote: > > However, this is undesirable for DRM. In a multi-output situation, > > we would see a view only visible on another output, reasonably decide we > > didn't want it in a plane on our output, and move it to the primary > > plane, causing damage, and an output repaint. The plane wouldn't be > > assigned until the other output ran through repaint. > > > > For large SHM buffers (weston_surface->keep_buffer as false), this means > > that the other output would assign it to a plane later, which caused > > weston_surface_damage to be called - in the exact way the comment says > > it shouldn't - which triggered a flush and buffer upload. By this stage, > > the buffer content would be gone and we would upload garbage. > > Er, I can't English this afternoon. Imagine the above two paragraphs > never happened, and mentally replace them with: > > However, this is undesirable for DRM. With multi-output, when > assign_planes() is called, any view which wasn't on a plane for our > putput was moved to the primary plane, thus causing damage, and an Putput! \o/ > output repaint. Fixing this, to ignore views which do not touch our > output at all, means that the view wouldn't have a plane assigned until > the other output eventually ran through repaint. > > For large SHM buffers (weston_surface->keep_buffer == false), this means > that by the time the other output assigned it to a plane, the buffer may > have been discarded on the client side. Oh, upload garbage, right. That's a very interesting side-effect of weston_surface_damage(). To me it makes perfect sense to assign views to the primary plane by default, on its own. "definitely the wrong way to fix" what? The weston_surface_damage() issue? I'd probably not even make the connection there. The DRM output/plane assignment? Is there something that makes a decision based on which plane a view is already on? Oh right, there has to be, because we don't support having the same view on several planes, right? (Hmm, why was that... let me guess: damage tracking?) I just couldn't spot where we actually check the plane assignment. Is that something you are adding with atomic prep? So, reading both versions of the commit message, I think I was able reconstruct enough of the idea to see what's going on, but please have a third try on explaining it. ;-) Anyway, the change itself is: Reviewed-by: Pekka Paalanen <[email protected]> Thanks, pq _______________________________________________ wayland-devel mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/wayland-devel
