Re: [PATCH] fullscreen-shell: Add missing license tag
Acked-by: Jason Ekstrand On Thu, Jun 28, 2018 at 6:26 AM, Johan Klokkhammer Helsing < johan.hels...@qt.io> wrote: > Although it would probably default to the license at the root of the > repository anyway, it's best to be explicit about it, and also be > consistent with the other extensions. > > The copyright holders have been assembled from git history and the > README. > > Signed-off-by: Johan Klokkhammer Helsing > --- > .../fullscreen-shell-unstable-v1.xml | 25 +++ > 1 file changed, 25 insertions(+) > > diff --git a/unstable/fullscreen-shell/fullscreen-shell-unstable-v1.xml > b/unstable/fullscreen-shell/fullscreen-shell-unstable-v1.xml > index 7d141ee..1bca7b7 100644 > --- a/unstable/fullscreen-shell/fullscreen-shell-unstable-v1.xml > +++ b/unstable/fullscreen-shell/fullscreen-shell-unstable-v1.xml > @@ -1,6 +1,31 @@ > > > > + > +Copyright © 2016 Yong Bakos > +Copyright © 2015 Jason Ekstrand > +Copyright © 2015 Jonas Ådahl > + > +Permission is hereby granted, free of charge, to any person obtaining > a > +copy of this software and associated documentation files (the > "Software"), > +to deal in the Software without restriction, including without > limitation > +the rights to use, copy, modify, merge, publish, distribute, > sublicense, > +and/or sell copies of the Software, and to permit persons to whom the > +Software is furnished to do so, subject to the following conditions: > + > +The above copyright notice and this permission notice (including the > next > +paragraph) shall be included in all copies or substantial portions of > the > +Software. > + > +THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, > EXPRESS OR > +IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF > MERCHANTABILITY, > +FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT > SHALL > +THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR > OTHER > +LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, > ARISING > +FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER > +DEALINGS IN THE SOFTWARE. > + > + > > >Displays a single surface per output. > -- > 2.18.0 > > ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH wayland-protocols v7] Add zwp_linux_explicit_synchronization_v1
Sorry to drag up ancient threads, but what's the status of this? I see rumors that it's in Weston. Is it stable? Is it implemented anywhere else? It'd be great, for the sake of Vulkan, if we could get this stable and everywhere. --Jason On Mon, Dec 17, 2018 at 11:25 AM Tomek Bury wrote: > Thanks! That looks better than my patch. > > On Mon, 17 Dec 2018 at 15:37, Alexandros Frantzis < > alexandros.frant...@collabora.com> wrote: > >> On Monday, December 17, 2018 12:44 GMT, Tomek Bury >> wrote: >> > On Wed, 28 Nov 2018 at 14:35, Tomek Bury wrote: >> > > Hi Pekka, >> > > >> > > > I suppose the compositor-side copy of buffers you mentioned is for >> the >> > > > lack of release fences, not acquire fences? >> > > Yes, the lack of release fences is the very reason for the copy. I >> could >> > > cook something up for the acquire fence, but that wouldn't >> synchronise the >> > > buffer access anyway. The only way I can be sure the client doesn't >> > > overwrite a buffer still in use by the GPU was to texture from a copy >> and >> > > let the compositor release the original without waiting for the GPU >> and >> > > without a fence. >> > > >> > > Cheers, >> > > Tomek >> > > >> > Hi Pekka, Alexandros, >> > >> > Here's a patch containing all I had to do in order to test the explicit >> > sync support Alexandros has implemented in Weston. >> > >> > Thanks, >> > Tomek >> >> Hi Tomek, >> >> I have now updated the weston explicit-sync gitlab MR [1] to support, >> among other things, minor version 2 of the protocol. Instead of >> completely removing the checks, I have updated them to check for and >> accept all non-SHM buffers, which is adequate for current needs. There >> are other ways to deal with these checks if we think we need a more >> versatile approach (e.g., asking the renderer to tell us if it support >> fences >> for a particular buffer). Please see the MR comments for more info and >> discussion. >> >> Thanks, >> Alexandros >> >> [1] https://gitlab.freedesktop.org/wayland/weston/merge_requests/32 >> >> ___ > wayland-devel mailing list > wayland-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/wayland-devel > ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [Intel-gfx] [Mesa-dev] gitlab.fd.o financial situation and impact on services
On Fri, Feb 28, 2020 at 11:00 AM Rob Clark wrote: > > On Fri, Feb 28, 2020 at 3:43 AM Michel Dänzer wrote: > > > > On 2020-02-28 10:28 a.m., Erik Faye-Lund wrote: > > > > > > We could also do stuff like reducing the amount of tests we run on each > > > commit, and punt some testing to a per-weekend test-run or someting > > > like that. We don't *need* to know about every problem up front, just > > > the stuff that's about to be released, really. The other stuff is just > > > nice to have. If it's too expensive, I would say drop it. > > > > I don't agree that pre-merge testing is just nice to have. A problem > > which is only caught after it lands in mainline has a much bigger impact > > than one which is already caught earlier. > > > > one thought.. since with mesa+margebot we effectively get at least > two(ish) CI runs per MR, ie. one when it is initially pushed, and one > when margebot rebases and tries to merge, could we leverage this to > have trimmed down pre-margebot CI which tries to just target affected > drivers, with margebot doing a full CI run (when it is potentially > batching together multiple MRs)? > > Seems like a way to reduce our CI runs with a good safety net to > prevent things from slipping through the cracks. Here are a couple more hopefully constructive but possibly bogus ideas: 1. Suggest people put their CI farms behind a squid transparent caching proxy. There seem to be many HowTo's on the internet for doing this and it shouldn't be terribly hard. Maybe GitLab uses too much HTTPS and that messes things up? If not, this would cut downloads to one-per-farm rather than one-per-machine 2. Add -Dstrip=true to the meson config. We want asserts but do we really need those debug symbols? Quick testing on my machine, it seems to reduce the size of build artifacts by about 60% Feel free to tell the peanut gallery (me) why I'm wrong. :-) --Jason ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [Mesa-dev] [Intel-gfx] gitlab.fd.o financial situation and impact on services
On Sat, Feb 29, 2020 at 3:47 PM Timur Kristóf wrote: > > On Sat, 2020-02-29 at 14:46 -0500, Nicolas Dufresne wrote: > > > > > > 1. I think we should completely disable running the CI on MRs which > > > are > > > marked WIP. Speaking from personal experience, I usually make a lot > > > of > > > changes to my MRs before they are merged, so it is a waste of CI > > > resources. > > > > In the mean time, you can help by taking the habit to use: > > > > git push -o ci.skip > > Thanks for the advice, I wasn't aware such an option exists. Does this > also work on the mesa gitlab or is this a GStreamer only thing? Mesa is already set up so that it only runs on MRs and branches named ci-* (or maybe it's ci/*; I can't remember). > How hard would it be to make this the default? I strongly suggest looking at how Mesa does it and doing that in GStreamer if you can. It seems to work pretty well in Mesa. --Jason > > That's a much more difficult goal then it looks like. Let each > > projects > > manage their CI graph and content, as each case is unique. Running > > more > > tests, or building more code isn't the main issue as the CPU time is > > mostly sponsored. The data transfers between the cloud of gitlab and > > the runners (which are external), along to sending OS image to Lava > > labs is what is likely the most expensive. > > > > As it was already mention in the thread, what we are missing now, and > > being worked on, is per group/project statistics that give us the > > hotspot so we can better target the optimization work. > > Yes, would be nice to know what the hotspot is, indeed. > > As far as I understand, the problem is not CI itself, but the bandwidth > needed by the build artifacts, right? Would it be possible to not host > the build artifacts on the gitlab, but rather only the place where the > build actually happened? Or at least, only transfer the build artifacts > on-demand? > > I'm not exactly familiar with how the system works, so sorry if this is > a silly question. > > ___ > mesa-dev mailing list > mesa-...@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/mesa-dev ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [Intel-gfx] [Mesa-dev] gitlab.fd.o financial situation and impact on services
I don't think we need to worry so much about the cost of CI that we need to micro-optimize to to get the minimal number of CI runs. We especially shouldn't if it begins to impact coffee quality, people's ability to merge patches in a timely manner, or visibility into what went wrong when CI fails. I've seen a number of suggestions which will do one or both of those things including: - Batching merge requests - Not running CI on the master branch - Shutting off CI - Preventing CI on other non-MR branches - Disabling CI on WIP MRs - I'm sure there are more... I think there are things we can do to make CI runs more efficient with some sort of end-point caching and we can probably find some truly wasteful CI to remove. Most of the things in the list above, I've seen presented by people who are only lightly involved the project to my knowledge (no offense to anyone intended). Developers depend on the CI system for their day-to-day work and hampering it will only show down development, reduce code quality, and ultimately hurt our customers and community. If we're so desperate as to be considering painful solutions which will have a negative impact on development, we're better off trying to find more money. --Jason On March 1, 2020 13:51:32 Jacob Lifshay wrote: One idea for Marge-bot (don't know if you already do this): Rust-lang has their bot (bors) automatically group together a few merge requests into a single merge commit, which it then tests, then, then the tests pass, it merges. This could help reduce CI runs to once a day (or some other rate). If the tests fail, then it could automatically deduce which one failed, by recursive subdivision or similar. There's also a mechanism to adjust priority and grouping behavior when the defaults aren't sufficient. Jacob ___ Intel-gfx mailing list intel-...@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [Intel-gfx] [Mesa-dev] gitlab.fd.o financial situation and impact on services
On Sun, Mar 1, 2020 at 2:49 PM Nicolas Dufresne wrote: > > Hi Jason, > > I personally think the suggestion are still a relatively good > brainstorm data for those implicated. Of course, those not implicated > in the CI scripting itself, I'd say just keep in mind that nothing is > black and white and every changes end-up being time consuming. Sorry. I didn't intend to stop a useful brainstorming session. I'm just trying to say that CI is useful and we shouldn't hurt our development flows just to save a little money unless we're truly desperate. From what I understand, I don't think we're that desperate yet. So I was mostly trying to re-focus the discussion towards straightforward things we can do to get rid of pointless waste (there probably is some pretty low-hanging fruit) and away from "OMG X.org is running out of money; CI as little as possible". I don't think you're saying those things; but I've sensed a good bit of fear in this thread. (I could just be totally misreading people, but I don't think so.) One of the things that someone pointed out on this thread is that we need data. Some has been provided here but it's still a bit unclear exactly what the break-down is so it's hard for people to come up with good solutions beyond "just do less CI". We do know that the biggest cost is egress web traffic and that's something we didn't know before. My understanding is that people on the X.org board and/or Daniel are working to get better data. I'm fairly hopeful that, once we understand better what the costs are (or even with just the new data we have), we can bring it down to reasonable and/or come up with money to pay for it in fairly short order. Again, sorry I was so terse. I was just trying to slow the panic. > Le dimanche 01 mars 2020 à 14:18 -0600, Jason Ekstrand a écrit : > > I've seen a number of suggestions which will do one or both of those things > > including: > > > > - Batching merge requests > > Agreed. Or at least I foresee quite complicated code to handle the case > of one batched merge failing the tests, or worst, with flicky tests. > > > - Not running CI on the master branch > > A small clarification, this depends on the chosen work-flow. In > GStreamer, we use a rebase flow, so "merge" button isn't really > merging. It means that to merge you need your branch to be rebased on > top of the latest. As it is multi-repo, there is always a tiny chance > of breakage due to mid-air collision in changes in other repos. What we > see is that the post "merge" cannot even catch them all (as we already > observed once). In fact, it usually does not catch anything. Or each > time it cached something, we only notice on the next MR.0 So we are > really considering doing this as for this specific workflow/project, we > found very little gain of having it. > > With real merge, the code being tested before/after the merge is > different, and for that I agree with you. Even with a rebase model, it's still potentially different; though marge re-runs CI before merging. I agree the risk is low, however, and if you have GitLab set up to block MRs that don't pass CI, then you may be able to drop the master branch to a daily run or something like that. Again, should be project-by-project. > > - Shutting off CI > > Of course :-), specially that we had CI before gitlab in GStreamer > (just not pre-commit), we don't want a regress that far in the past. > > > - Preventing CI on other non-MR branches > > Another small nuance, mesa does not prevent CI, it only makes it manual > on non-MR. Users can go click run to get CI results. We could also have > option to trigger the ci (the opposite of ci.skip) from git command > line. Hence my use of "prevent". :-) It's very useful but, IMO, it should be opt-in and not opt-out. I think we agree here. :-) --Jason ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
Plumbing explicit synchronization through the Linux ecosystem
her How to solve all these chicken-and-egg problems is something I've been giving quite a bit of thought (and talking with many others about) in the last couple of years. One motivation for this is that we have to deal with a mismatch in Vulkan. Another motivation is that I'm becoming increasingly unhappy with the way that synchronization, memory residency, and command submission are inherently intertwined in i915 and would like to break things apart. Towards that end, I have an actual proposal. A couple weeks ago, I sent a series of patches to the dri-devel mailing list which adds a pair of new ioctls to dma-buf which allow userspace to manually import or export a sync_file from a dma-buf. The idea is that something like a Wayland compositor can switch to 100% explicit sync internally once the ioctl is available. If it gets buffers in from a client that doesn't use the explicit sync extension, it can pull a sync_file from the dma-buf and use that exactly as it would a sync_file passed via the explicit sync extension. When it goes to scan out a user buffer and discovers that KMS doesn't accept sync_files (or if it tries to use that pesky media encoder no one has converted), it can take it's sync_file for display and stuff it into the dma-buf before handing it to KMS. Along with the kernel patches, I've also implemented support for this in the Vulkan WSI code used by ANV and RADV. With those patches, the only requirement on the Vulkan drivers is that you be able to export any VkSemaphore as a sync_file and temporarily import a sync_file into any VkFence or VkSemaphore. As long as that works, the core Vulkan driver only ever sees explicit synchronization via sync_file. The WSI code uses these new ioctls to translate the implicit sync of X11 and Wayland to the explicit sync the Vulkan driver wants. I'm hoping (and here's where I want a sanity check) that a simple API like this will allow us to finally start moving the Linux ecosystem over to explicit synchronization one piece at a time in a way that's actually correct. (No Wayland explicit sync with compositors hoping KMS magically works even though it doesn't have a sync_file API.) Once some pieces in the ecosystem start moving, there will be motivation to start moving others and maybe we can actually build the momentum to get most everything converted. For reference, you can find the kernel RFC patches and mesa MR here: https://lists.freedesktop.org/archives/dri-devel/2020-March/258833.html https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/4037 At this point, I welcome your thoughts, comments, objections, and maybe even help/review. :-) --Jason Ekstrand ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: Plumbing explicit synchronization through the Linux ecosystem
On Wed, Mar 11, 2020 at 12:31 PM Jason Ekstrand wrote: > > All, > > Sorry for casting such a broad net with this one. I'm sure most people > who reply will get at least one mailing list rejection. However, this > is an issue that affects a LOT of components and that's why it's > thorny to begin with. Please pardon the length of this e-mail as > well; I promise there's a concrete point/proposal at the end. > > > Explicit synchronization is the future of graphics and media. At > least, that seems to be the consensus among all the graphics people > I've talked to. I had a chat with one of the lead Android graphics > engineers recently who told me that doing explicit sync from the start > was one of the best engineering decisions Android ever made. It's > also the direction being taken by more modern APIs such as Vulkan. > > > ## What are implicit and explicit synchronization? > > For those that aren't familiar with this space, GPUs, media encoders, > etc. are massively parallel and synchronization of some form is > required to ensure that everything happens in the right order and > avoid data races. Implicit synchronization is when bits of work (3D, > compute, video encode, etc.) are implicitly based on the absolute > CPU-time order in which API calls occur. Explicit synchronization is > when the client (whatever that means in any given context) provides > the dependency graph explicitly via some sort of synchronization > primitives. If you're still confused, consider the following > examples: > > With OpenGL and EGL, almost everything is implicit sync. Say you have > two OpenGL contexts sharing an image where one writes to it and the > other textures from it. The way the OpenGL spec works, the client has > to make the API calls to render to the image before (in CPU time) it > makes the API calls which texture from the image. As long as it does > this (and maybe inserts a glFlush?), the driver will ensure that the > rendering completes before the texturing happens and you get correct > contents. > > Implicit synchronization can also happen across processes. Wayland, > for instance, is currently built on implicit sync where the client > does their rendering and then does a hand-off (via wl_surface::commit) > to tell the compositor it's done at which point the compositor can now > texture from the surface. The hand-off ensures that the client's > OpenGL API calls happen before the server's OpenGL API calls. > > A good example of explicit synchronization is the Vulkan API. There, > a client (or multiple clients) can simultaneously build command > buffers in different threads where one of those command buffers > renders to an image and the other textures from it and then submit > both of them at the same time with instructions to the driver for > which order to execute them in. The execution order is described via > the VkSemaphore primitive. With the new VK_KHR_timeline_semaphore > extension, you can even submit the work which does the texturing > BEFORE the work which does the rendering and the driver will sort it > out. > > The #1 problem with implicit synchronization (which explicit solves) > is that it leads to a lot of over-synchronization both in client space > and in driver/device space. The client has to synchronize a lot more > because it has to ensure that the API calls happen in a particular > order. The driver/device have to synchronize a lot more because they > never know what is going to end up being a synchronization point as an > API call on another thread/process may occur at any time. As we move > to more and more multi-threaded programming this synchronization (on > the client-side especially) becomes more and more painful. > > > ## Current status in Linux > > Implicit synchronization in Linux works via a the kernel's internal > dma_buf and dma_fence data structures. A dma_fence is a tiny object > which represents the "done" status for some bit of work. Typically, > dma_fences are created as a by-product of someone submitting some bit > of work (say, 3D rendering) to the kernel. The dma_buf object has a > set of dma_fences on it representing shared (read) and exclusive > (write) access to the object. When work is submitted which, for > instance renders to the dma_buf, it's queued waiting on all the fences > on the dma_buf and and a dma_fence is created representing the end of > said rendering work and it's installed as the dma_buf's exclusive > fence. This way, the kernel can manage all its internal queues (3D > rendering, display, video encode, etc.) and know which things to > submit in what order. > > For the last few years,
Re: Plumbing explicit synchronization through the Linux ecosystem
It seems I may have not set the tone I intended with this e-mail... My intention was never to stomp on anyone's favorite window system (Adam, isn't the only one who's seemed a bit miffed). My intention was to try and solve some very real problems that we have with Vulkan and I had the hope that a solution there could be helpful for others. The problem we have in Vulkan is that we have an inherently explicit sync graphics API and we're trying to strap it onto some inherently implicit sync window systems and kernel interfaces. Our mechanisms for doing so have evolved over the course of the last 4-5 years and it's way better now than it was when we started but it's still pretty bad and very invasive to the driver. My objective is to completely remove the concept of implicit sync from the Vulkan driver eventually. Also (and this is going further down the rabbit hole), I would like to begin cleaning up our i915 UAPI to better separate memory residency handling, command submission, and synchronization. Eventually (and this may sound crazy to some), I'd like to get to the point where i915 doesn't own any of the synchronization primitives except what it needs to handle memory management internally. Linux graphics UAPI is about 10 years behind Windows in terms of design (roughly equivalent to Win7) and I think it's costing us in terms of latency and CPU overhead. Some of that may just be implementation problems in i915; some of it may be core API design. It's a bit unclear. Why am I bringing up kernel APIs? Because one of the biggest problems in evolving things is the fact that our kernel APIs are tied to implicit sync on dma-buf. We can't detangle that until we can remove implicit dma-buf signaling from the command execution APIs. This means that we either need to get rid of ALL implicit synchronization from window-system APIs far enough back in time that we don't run the risk of "breaking userspace" or else we need a plan which lets the kernel driver not support implicit sync but make implicit sync work anyway. What I'm proposing with dma-buf sync_file import/export is one such plan. So, while this may not solve any problems for Wayland compositors as I previously thought (KMS/atomic supports sync_file. Yay!), we still have a very real problem in Vulkan. It's great that Wayland has an explicit sync API but until all compositors have supported it for at least 2 years, I can't assume it's existence and start deleting my old code paths. Currently, it's only implemented in Weston and the ChromeOS compositor; gnome-shell, kwin, and sway are all still 100% implicit sync AFAIK. We also have to deal with X11. For those who are asking the question in the back of their minds: Yes, I'm trying to solve a userspace problem with kernel code and, no, I don't think that's necessarily the wrong way around. Don't get me wrong; I very much want to solve the problem "properly" but unless we're very sure we can get it solved properly everywhere quickly, a solution which lets us improve our driver kernel APIs independently of misc. Wayland compositors seems advantageous. On Wed, Mar 11, 2020 at 6:02 PM Adam Jackson wrote: > > On Wed, 2020-03-11 at 12:31 -0500, Jason Ekstrand wrote: > > > - X11: With present, it has these "explicit" fence objects but > > they're always a shmfence which lets the X server and client do a > > userspace CPU-side hand-off without going over the socket (and > > round-tripping through the kernel). However, the only thing that > > fence does is order the OpenGL API calls in the client and server and > > the real synchronization is still implicit. > > I'm pretty sure "the only thing that fence does" is an implementation > detail. So I've been told, many times. > PresentPixmap blocks until the wait-fence signals, but when and > how it signals are properties of the fence itself. You could have drm > give the client back a fence fd, pass that to xserver to create a fence > object, and name that in the PresentPixmap request, and then drm can do > whatever it wants to signal the fence. Poking around at things, X11 may not be quite as bad as I thought here. It's not really set up for sync_file for a couple reasons: 1. It only passes the file descriptor in once at xcb_dri3_fence_from_fd rather than re-creating every frame from a new sync_file 2. It only takes a fence on present and doesn't return one in the PRESENT_COMPLETE event That said, plumbing syncobj in as an extension looks like a real possibility. A syncobj is just a container that holds a pointer to a dma_fence and it has roughly the same CPU signal/reset behavior that's exposed by the SyncFenceFuncsRec struct. There's a few things I'm not sure how to handle: 1. The
Re: [Mesa-dev] Plumbing explicit synchronization through the Linux ecosystem
Could you elaborate. If there's something missing from my mental model of how implicit sync works, I'd like to have it corrected. People continue claiming that AMD is somehow special but I have yet to grasp what makes it so. (Not that anyone has bothered to try all that hard to explain it.) --Jason On March 13, 2020 21:03:21 Marek Olšák wrote: There is no synchronization between processes (e.g. 3D app and compositor) within X on AMD hw. It works because of some hacks in Mesa. Marek On Wed, Mar 11, 2020 at 1:31 PM Jason Ekstrand wrote: All, Sorry for casting such a broad net with this one. I'm sure most people who reply will get at least one mailing list rejection. However, this is an issue that affects a LOT of components and that's why it's thorny to begin with. Please pardon the length of this e-mail as well; I promise there's a concrete point/proposal at the end. Explicit synchronization is the future of graphics and media. At least, that seems to be the consensus among all the graphics people I've talked to. I had a chat with one of the lead Android graphics engineers recently who told me that doing explicit sync from the start was one of the best engineering decisions Android ever made. It's also the direction being taken by more modern APIs such as Vulkan. ## What are implicit and explicit synchronization? For those that aren't familiar with this space, GPUs, media encoders, etc. are massively parallel and synchronization of some form is required to ensure that everything happens in the right order and avoid data races. Implicit synchronization is when bits of work (3D, compute, video encode, etc.) are implicitly based on the absolute CPU-time order in which API calls occur. Explicit synchronization is when the client (whatever that means in any given context) provides the dependency graph explicitly via some sort of synchronization primitives. If you're still confused, consider the following examples: With OpenGL and EGL, almost everything is implicit sync. Say you have two OpenGL contexts sharing an image where one writes to it and the other textures from it. The way the OpenGL spec works, the client has to make the API calls to render to the image before (in CPU time) it makes the API calls which texture from the image. As long as it does this (and maybe inserts a glFlush?), the driver will ensure that the rendering completes before the texturing happens and you get correct contents. Implicit synchronization can also happen across processes. Wayland, for instance, is currently built on implicit sync where the client does their rendering and then does a hand-off (via wl_surface::commit) to tell the compositor it's done at which point the compositor can now texture from the surface. The hand-off ensures that the client's OpenGL API calls happen before the server's OpenGL API calls. A good example of explicit synchronization is the Vulkan API. There, a client (or multiple clients) can simultaneously build command buffers in different threads where one of those command buffers renders to an image and the other textures from it and then submit both of them at the same time with instructions to the driver for which order to execute them in. The execution order is described via the VkSemaphore primitive. With the new VK_KHR_timeline_semaphore extension, you can even submit the work which does the texturing BEFORE the work which does the rendering and the driver will sort it out. The #1 problem with implicit synchronization (which explicit solves) is that it leads to a lot of over-synchronization both in client space and in driver/device space. The client has to synchronize a lot more because it has to ensure that the API calls happen in a particular order. The driver/device have to synchronize a lot more because they never know what is going to end up being a synchronization point as an API call on another thread/process may occur at any time. As we move to more and more multi-threaded programming this synchronization (on the client-side especially) becomes more and more painful. ## Current status in Linux Implicit synchronization in Linux works via a the kernel's internal dma_buf and dma_fence data structures. A dma_fence is a tiny object which represents the "done" status for some bit of work. Typically, dma_fences are created as a by-product of someone submitting some bit of work (say, 3D rendering) to the kernel. The dma_buf object has a set of dma_fences on it representing shared (read) and exclusive (write) access to the object. When work is submitted which, for instance renders to the dma_buf, it's queued waiting on all the fences on the dma_buf and and a dma_fence is created representing the end of said rendering work and it's installed as the dma_buf's exclusive fence. This way, the kernel can manage all its internal queues (3D rendering, display, vi
Re: Plumbing explicit synchronization through the Linux ecosystem
On Mon, Mar 16, 2020 at 5:20 AM Laurent Pinchart wrote: > > On Wed, Mar 11, 2020 at 04:18:55PM -0400, Nicolas Dufresne wrote: > > (I know I'm going to be spammed by so many mailing list ...) > > > > Le mercredi 11 mars 2020 à 14:21 -0500, Jason Ekstrand a écrit : > > > On Wed, Mar 11, 2020 at 12:31 PM Jason Ekstrand > > > wrote: > > > > All, > > > > > > > > Sorry for casting such a broad net with this one. I'm sure most people > > > > who reply will get at least one mailing list rejection. However, this > > > > is an issue that affects a LOT of components and that's why it's > > > > thorny to begin with. Please pardon the length of this e-mail as > > > > well; I promise there's a concrete point/proposal at the end. > > > > > > > > > > > > Explicit synchronization is the future of graphics and media. At > > > > least, that seems to be the consensus among all the graphics people > > > > I've talked to. I had a chat with one of the lead Android graphics > > > > engineers recently who told me that doing explicit sync from the start > > > > was one of the best engineering decisions Android ever made. It's > > > > also the direction being taken by more modern APIs such as Vulkan. > > > > > > > > > > > > ## What are implicit and explicit synchronization? > > > > > > > > For those that aren't familiar with this space, GPUs, media encoders, > > > > etc. are massively parallel and synchronization of some form is > > > > required to ensure that everything happens in the right order and > > > > avoid data races. Implicit synchronization is when bits of work (3D, > > > > compute, video encode, etc.) are implicitly based on the absolute > > > > CPU-time order in which API calls occur. Explicit synchronization is > > > > when the client (whatever that means in any given context) provides > > > > the dependency graph explicitly via some sort of synchronization > > > > primitives. If you're still confused, consider the following > > > > examples: > > > > > > > > With OpenGL and EGL, almost everything is implicit sync. Say you have > > > > two OpenGL contexts sharing an image where one writes to it and the > > > > other textures from it. The way the OpenGL spec works, the client has > > > > to make the API calls to render to the image before (in CPU time) it > > > > makes the API calls which texture from the image. As long as it does > > > > this (and maybe inserts a glFlush?), the driver will ensure that the > > > > rendering completes before the texturing happens and you get correct > > > > contents. > > > > > > > > Implicit synchronization can also happen across processes. Wayland, > > > > for instance, is currently built on implicit sync where the client > > > > does their rendering and then does a hand-off (via wl_surface::commit) > > > > to tell the compositor it's done at which point the compositor can now > > > > texture from the surface. The hand-off ensures that the client's > > > > OpenGL API calls happen before the server's OpenGL API calls. > > > > > > > > A good example of explicit synchronization is the Vulkan API. There, > > > > a client (or multiple clients) can simultaneously build command > > > > buffers in different threads where one of those command buffers > > > > renders to an image and the other textures from it and then submit > > > > both of them at the same time with instructions to the driver for > > > > which order to execute them in. The execution order is described via > > > > the VkSemaphore primitive. With the new VK_KHR_timeline_semaphore > > > > extension, you can even submit the work which does the texturing > > > > BEFORE the work which does the rendering and the driver will sort it > > > > out. > > > > > > > > The #1 problem with implicit synchronization (which explicit solves) > > > > is that it leads to a lot of over-synchronization both in client space > > > > and in driver/device space. The client has to synchronize a lot more > > > > because it has to ensure that the API calls happen in a particular > > > > order. The driver/device have to synchronize a lot more because they > > > > never know what is going
Re: Plumbing explicit synchronization through the Linux ecosystem
On Mon, Mar 16, 2020 at 10:33 AM Tomek Bury wrote: > > > GL and GLES are not relevant. What is relevant is EGL, which defines > > interfaces to make things work on the native platform. > Yes and no. This is what EGL spec says about sharing a texture between > contexts: > > "OpenGL and OpenGL ES makes no attempt to synchronize access to > texture objects. If a texture object is bound to more than one > context, then it is up to the programmer to ensure that the contents > of the object are not being changed via one context while another > context is using the texture object for rendering. The results of > changing a texture object while another context is using it are > undefined." > > There are similar statements with regards to the lack of > synchronisation guarantees for EGL images or between GL and native > rendering, etc. But the main thing here is that EGL and Vulkan differ > significantly. The eglSwapBuffers() is expected to post an unspecified > "back buffer" to the display system using some internal driver magic. > EGL driver is then expected to obtain another back buffer at some > unspecified point in the future. Vulkan on the other hand is very > specific and explicit. The vkQueuePresentKHR() is expected to post a > specific vkImage with an explicit set of set of semaphores. Another > image is obtained through vkAcquireNextImageKHR() and it's the > application's decision whether it wants a fence, a semaphore, both or > none with the acquired buffer. The implicit synchronisation doesn't > mix well with Vulkan drivers and requires a lot of extra plumbing in > the WSI code. Yes, and that (the Vulkan issues in particular) is what I'm trying to fix. :-) (among other things...) Assuming the kernel patch I linked to, your usermode driver could stuff fences in the dma-buf without having that be part of your kernel driver. This assumes, of course, that your kernel driver supports sync_file. > > If you are using EGL_WL_bind_wayland_display, then one of the things > > it is explicitly allowed/expected to do is to create a Wayland > > protocol interface between client and compositor, which can be used to > > pass buffer handles and metadata in a platform-specific way. Adding > > synchronisation is also possible. > Only one-way synchronisation is possible with this mechanism. There's > a standard protocol for recycling buffers - wl_buffer_release() so > buffer hand-over from the compositor to client remains unsynchronised > > - see below. > > > > The most troublesome part was Wayland buffer release mechanism, as it > > > only involves a CPU signalling over Wayland IPC, without any 3D driver > > > involvement. The choices were: explicit synchronisation extension or a > > > buffer copy in the compositor (i.e. compositor textures from the copy, so > > > the client can re-write the original), or some implicit synchronisation > > > in kernel space (but that wasn't an option in Broadcom driver). > > > > You can add your own explicit synchronisation extension. > I could but that requires implementing in in the driver and in a > number of compositors, therefore a standard extension > zwp_linux_explicit_synchronization_v1 is much better choice here than > a custom one. I think you may be missing what Daniel is saying. Wayland allows you to do basically anything you want within your client and server-side EGL implementations. That could include the server-side EGL sending an event with a fence every single time a flush operation happens in the server-side GL/GLES implementation. (Could be glFlush, glFinish, eglSwapBuffers, or other things). Since wayland protocol events are ordered, the client-side EGL implementation would get the most recent flush event before it got the wl_buffer::release. I fully agree that it's rather cumbersome though. > > In every cross-process and cross-subsystem usecase, synchronisation is > > obviously required. The two options for this are to implement kernel > > support for implicit synchronisation (as everyone else has done), > That would require major changes in driver architecture or a 2nd > mechanisms doing the same thing but in kernel space - both are > non-starters. > > > or implement generic support for explicit synchronisation (as we have > > been working on with implementations inside Weston and Exosphere at > > least), > The zwp_linux_explicit_synchronization_v1 is a good step forward. I'm > using this extension as a main synchronisation mechanism in EGL and > Vulkan driver whenever available. I remember that Gustavo Padovan was > working on explicit sync support in the display system some time ago. > I hope it got merged into kernel by now, but I don't know to what > extend it's actually being used. It is supported by KMS/atomic. Legacy KMS, however, does not support it. > > or implement private support for explicit synchronisation, > If everything else fails, that would be the last resort scenario, but > far from ideal and very costly in terms of implementat
Re: Plumbing explicit synchronization through the Linux ecosystem
On Mon, Mar 16, 2020 at 4:15 PM Laurent Pinchart wrote: > > Hi Jason, > > On Mon, Mar 16, 2020 at 10:06:07AM -0500, Jason Ekstrand wrote: > > On Mon, Mar 16, 2020 at 5:20 AM Laurent Pinchart wrote: > > > On Wed, Mar 11, 2020 at 04:18:55PM -0400, Nicolas Dufresne wrote: > > >> (I know I'm going to be spammed by so many mailing list ...) > > >> > > >> Le mercredi 11 mars 2020 à 14:21 -0500, Jason Ekstrand a écrit : > > >>> On Wed, Mar 11, 2020 at 12:31 PM Jason Ekstrand > > >>> wrote: > > >>>> All, > > >>>> > > >>>> Sorry for casting such a broad net with this one. I'm sure most people > > >>>> who reply will get at least one mailing list rejection. However, this > > >>>> is an issue that affects a LOT of components and that's why it's > > >>>> thorny to begin with. Please pardon the length of this e-mail as > > >>>> well; I promise there's a concrete point/proposal at the end. > > >>>> > > >>>> > > >>>> Explicit synchronization is the future of graphics and media. At > > >>>> least, that seems to be the consensus among all the graphics people > > >>>> I've talked to. I had a chat with one of the lead Android graphics > > >>>> engineers recently who told me that doing explicit sync from the start > > >>>> was one of the best engineering decisions Android ever made. It's > > >>>> also the direction being taken by more modern APIs such as Vulkan. > > >>>> > > >>>> > > >>>> ## What are implicit and explicit synchronization? > > >>>> > > >>>> For those that aren't familiar with this space, GPUs, media encoders, > > >>>> etc. are massively parallel and synchronization of some form is > > >>>> required to ensure that everything happens in the right order and > > >>>> avoid data races. Implicit synchronization is when bits of work (3D, > > >>>> compute, video encode, etc.) are implicitly based on the absolute > > >>>> CPU-time order in which API calls occur. Explicit synchronization is > > >>>> when the client (whatever that means in any given context) provides > > >>>> the dependency graph explicitly via some sort of synchronization > > >>>> primitives. If you're still confused, consider the following > > >>>> examples: > > >>>> > > >>>> With OpenGL and EGL, almost everything is implicit sync. Say you have > > >>>> two OpenGL contexts sharing an image where one writes to it and the > > >>>> other textures from it. The way the OpenGL spec works, the client has > > >>>> to make the API calls to render to the image before (in CPU time) it > > >>>> makes the API calls which texture from the image. As long as it does > > >>>> this (and maybe inserts a glFlush?), the driver will ensure that the > > >>>> rendering completes before the texturing happens and you get correct > > >>>> contents. > > >>>> > > >>>> Implicit synchronization can also happen across processes. Wayland, > > >>>> for instance, is currently built on implicit sync where the client > > >>>> does their rendering and then does a hand-off (via wl_surface::commit) > > >>>> to tell the compositor it's done at which point the compositor can now > > >>>> texture from the surface. The hand-off ensures that the client's > > >>>> OpenGL API calls happen before the server's OpenGL API calls. > > >>>> > > >>>> A good example of explicit synchronization is the Vulkan API. There, > > >>>> a client (or multiple clients) can simultaneously build command > > >>>> buffers in different threads where one of those command buffers > > >>>> renders to an image and the other textures from it and then submit > > >>>> both of them at the same time with instructions to the driver for > > >>>> which order to execute them in. The execution order is described via > > >>>> the VkSemaphore primitive. With the new VK_KHR_timeline_semaphore > > >>>> extension, you can even submit the work which does the texturing > > >>>> BEFORE the work whic
Re: Plumbing explicit synchronization through the Linux ecosystem
On Mon, Mar 16, 2020 at 6:39 PM Roman Gilg wrote: > > On Wed, Mar 11, 2020 at 8:21 PM Jason Ekstrand wrote: > > > > On Wed, Mar 11, 2020 at 12:31 PM Jason Ekstrand > > wrote: > > > > > > All, > > > > > > Sorry for casting such a broad net with this one. I'm sure most people > > > who reply will get at least one mailing list rejection. However, this > > > is an issue that affects a LOT of components and that's why it's > > > thorny to begin with. Please pardon the length of this e-mail as > > > well; I promise there's a concrete point/proposal at the end. > > > > > > > > > Explicit synchronization is the future of graphics and media. At > > > least, that seems to be the consensus among all the graphics people > > > I've talked to. I had a chat with one of the lead Android graphics > > > engineers recently who told me that doing explicit sync from the start > > > was one of the best engineering decisions Android ever made. It's > > > also the direction being taken by more modern APIs such as Vulkan. > > > > > > > > > ## What are implicit and explicit synchronization? > > > > > > For those that aren't familiar with this space, GPUs, media encoders, > > > etc. are massively parallel and synchronization of some form is > > > required to ensure that everything happens in the right order and > > > avoid data races. Implicit synchronization is when bits of work (3D, > > > compute, video encode, etc.) are implicitly based on the absolute > > > CPU-time order in which API calls occur. Explicit synchronization is > > > when the client (whatever that means in any given context) provides > > > the dependency graph explicitly via some sort of synchronization > > > primitives. If you're still confused, consider the following > > > examples: > > > > > > With OpenGL and EGL, almost everything is implicit sync. Say you have > > > two OpenGL contexts sharing an image where one writes to it and the > > > other textures from it. The way the OpenGL spec works, the client has > > > to make the API calls to render to the image before (in CPU time) it > > > makes the API calls which texture from the image. As long as it does > > > this (and maybe inserts a glFlush?), the driver will ensure that the > > > rendering completes before the texturing happens and you get correct > > > contents. > > > > > > Implicit synchronization can also happen across processes. Wayland, > > > for instance, is currently built on implicit sync where the client > > > does their rendering and then does a hand-off (via wl_surface::commit) > > > to tell the compositor it's done at which point the compositor can now > > > texture from the surface. The hand-off ensures that the client's > > > OpenGL API calls happen before the server's OpenGL API calls. > > > > > > A good example of explicit synchronization is the Vulkan API. There, > > > a client (or multiple clients) can simultaneously build command > > > buffers in different threads where one of those command buffers > > > renders to an image and the other textures from it and then submit > > > both of them at the same time with instructions to the driver for > > > which order to execute them in. The execution order is described via > > > the VkSemaphore primitive. With the new VK_KHR_timeline_semaphore > > > extension, you can even submit the work which does the texturing > > > BEFORE the work which does the rendering and the driver will sort it > > > out. > > > > > > The #1 problem with implicit synchronization (which explicit solves) > > > is that it leads to a lot of over-synchronization both in client space > > > and in driver/device space. The client has to synchronize a lot more > > > because it has to ensure that the API calls happen in a particular > > > order. The driver/device have to synchronize a lot more because they > > > never know what is going to end up being a synchronization point as an > > > API call on another thread/process may occur at any time. As we move > > > to more and more multi-threaded programming this synchronization (on > > > the client-side especially) becomes more and more painful. > > > > > > > > > ## Current status in Linux > > > > > > Implicit synchronization in Linux works via a the kernel's internal > > > dma_buf an
Re: Plumbing explicit synchronization through the Linux ecosystem
On Tue, Mar 17, 2020 at 3:01 AM Simon Ser wrote: > > On Monday, March 16, 2020 5:04 PM, Jason Ekstrand > wrote: > > > Hopefully, that will provide some motivation for other compositors > > (kwin, gnome-shell, etc.) because they now have a real user of it in > > an upstream driver for a major desktop platform and not just a few > > weston examples. However, someone is going to have to drive the > > actual development in those compositors. I'd be very happy if more > > people got involved, :-) > > FWIW, a wlroots pull request is in progress [0]. The plan is first to > accept fence FDs from clients, then send them our fences as a second > step. What exactly are the semantics there? Are you going to somehow wait inside wlroots for the buffer to be 100% idle or are you expecting the client to somehow use explicit for sending buffers implicit to wait for idle? If it's the latter, that's not going to work. --Jason ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: Plumbing explicit synchronization through the Linux ecosystem
On Tue, Mar 17, 2020 at 10:33 AM Nicolas Dufresne wrote: > > Le lundi 16 mars 2020 à 23:15 +0200, Laurent Pinchart a écrit : > > Hi Jason, > > > > On Mon, Mar 16, 2020 at 10:06:07AM -0500, Jason Ekstrand wrote: > > > On Mon, Mar 16, 2020 at 5:20 AM Laurent Pinchart wrote: > > > > On Wed, Mar 11, 2020 at 04:18:55PM -0400, Nicolas Dufresne wrote: > > > > > (I know I'm going to be spammed by so many mailing list ...) > > > > > > > > > > Le mercredi 11 mars 2020 à 14:21 -0500, Jason Ekstrand a écrit : > > > > > > On Wed, Mar 11, 2020 at 12:31 PM Jason Ekstrand > > > > > > wrote: > > > > > > > All, > > > > > > > > > > > > > > Sorry for casting such a broad net with this one. I'm sure most > > > > > > > people > > > > > > > who reply will get at least one mailing list rejection. However, > > > > > > > this > > > > > > > is an issue that affects a LOT of components and that's why it's > > > > > > > thorny to begin with. Please pardon the length of this e-mail as > > > > > > > well; I promise there's a concrete point/proposal at the end. > > > > > > > > > > > > > > > > > > > > > Explicit synchronization is the future of graphics and media. At > > > > > > > least, that seems to be the consensus among all the graphics > > > > > > > people > > > > > > > I've talked to. I had a chat with one of the lead Android > > > > > > > graphics > > > > > > > engineers recently who told me that doing explicit sync from the > > > > > > > start > > > > > > > was one of the best engineering decisions Android ever made. It's > > > > > > > also the direction being taken by more modern APIs such as Vulkan. > > > > > > > > > > > > > > > > > > > > > ## What are implicit and explicit synchronization? > > > > > > > > > > > > > > For those that aren't familiar with this space, GPUs, media > > > > > > > encoders, > > > > > > > etc. are massively parallel and synchronization of some form is > > > > > > > required to ensure that everything happens in the right order and > > > > > > > avoid data races. Implicit synchronization is when bits of work > > > > > > > (3D, > > > > > > > compute, video encode, etc.) are implicitly based on the absolute > > > > > > > CPU-time order in which API calls occur. Explicit > > > > > > > synchronization is > > > > > > > when the client (whatever that means in any given context) > > > > > > > provides > > > > > > > the dependency graph explicitly via some sort of synchronization > > > > > > > primitives. If you're still confused, consider the following > > > > > > > examples: > > > > > > > > > > > > > > With OpenGL and EGL, almost everything is implicit sync. Say you > > > > > > > have > > > > > > > two OpenGL contexts sharing an image where one writes to it and > > > > > > > the > > > > > > > other textures from it. The way the OpenGL spec works, the > > > > > > > client has > > > > > > > to make the API calls to render to the image before (in CPU time) > > > > > > > it > > > > > > > makes the API calls which texture from the image. As long as it > > > > > > > does > > > > > > > this (and maybe inserts a glFlush?), the driver will ensure that > > > > > > > the > > > > > > > rendering completes before the texturing happens and you get > > > > > > > correct > > > > > > > contents. > > > > > > > > > > > > > > Implicit synchronization can also happen across processes. > > > > > > > Wayland, > > > > > > > for instance, is currently built on implicit sync where the client > > > > > > > does their rendering and then does a hand-off (via > > > > > > > wl_surface::commit) > > &
Re: [Mesa-dev] Plumbing explicit synchronization through the Linux ecosystem
On Tue, Mar 17, 2020 at 12:13 PM Jacob Lifshay wrote: > > One related issue with explicit sync using sync_file is that combined > CPUs/GPUs (the CPU cores *are* the GPU cores) that do all the > rendering in userspace (like llvmpipe but for Vulkan and with extra > instructions for GPU tasks) but need to synchronize with other > drivers/processes is that there should be some way to create an > explicit fence/semaphore from userspace and later signal it. This > seems to conflict with the requirement for a sync_file to complete in > finite time, since the user process could be stopped or killed. Yeah... That's going to be a problem. The only way I could see that working is if you created a sync_file that had a timeout associated with it. However, then you run into the issue where you may have corruption if stuff doesn't complete on time. Then again, you're not really dealing with an external unit and so the latency cost of going across the window system protocol probably isn't massively different from the latency cost of triggering the sync_file. Maybe the answer there is to just do everything in-order and not worry about synchronization? ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [Mesa-dev] Plumbing explicit synchronization through the Linux ecosystem
On Tue, Mar 17, 2020 at 7:16 PM Jacob Lifshay wrote: > > On Tue, Mar 17, 2020 at 11:14 AM Lucas Stach wrote: > > > > Am Dienstag, den 17.03.2020, 10:59 -0700 schrieb Jacob Lifshay: > > > I think I found a userspace-accessible way to create sync_files and > > > dma_fences that would fulfill the requirements: > > > https://github.com/torvalds/linux/blob/master/drivers/dma-buf/sw_sync.c > > > > > > I'm just not sure if that's a good interface to use, since it appears > > > to be designed only for debugging. Will have to check for additional > > > requirements of signalling an error when the process that created the > > > fence is killed. It is expressly only for debugging and testing. Exposing such an API to userspace would break the finite time guarantees that are relied upon to keep sync_file a secure API. > > Something like that can certainly be lifted for general use if it makes > > sense. But then with a software renderer I don't really see how fences > > help you at all. With a software renderer you know exactly when the > > frame is finished and you can just defer pushing it over to the next > > pipeline element until that time. You won't gain any parallelism by > > using fences as the CPU is busy doing the rendering and will not run > > other stuff concurrently, right? > > There definitely may be other hardware and/or processes that can > process some stuff concurrently with the main application, such as the > compositor and or video encoding processes (for video capture). > Additionally, from what I understand, sync_file is the standard way to > export and import explicit synchronization between processes and > between drivers on Linux, so it seems like a good idea to support it > from an interoperability standpoint even if it turns out that there > aren't any scheduling/timing benefits. There are different ways that one can handle interoperability, however. One way is to try and make the software rasterizer look as much like a GPU as possible: lots of threads to make things as asynchronous as possible, "real" implementations of semaphores and fences, etc. Another is to let a SW rasterizer be a SW rasterizer: do everything immediately, thread only so you can exercise all the CPU cores, and minimally implement semaphores and fences well enough to maintain compatibility. If you take the first approach, then we have to solve all these problems with letting userspace create unsignaled sync_files which it will signal later and figure out how to make it safe. If you take the second approach, you'll only ever have to return already signaled sync_files and there's no problem with the sync_file finite time guarantees. --Jason ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [Mesa-dev] Plumbing explicit synchronization through the Linux ecosystem
On Wed, Mar 18, 2020 at 12:20 AM Jacob Lifshay wrote: > > On Tue, Mar 17, 2020 at 7:08 PM Jason Ekstrand wrote: > > > > On Tue, Mar 17, 2020 at 7:16 PM Jacob Lifshay > > wrote: > > > > > > On Tue, Mar 17, 2020 at 11:14 AM Lucas Stach wrote: > > > > > > > > Am Dienstag, den 17.03.2020, 10:59 -0700 schrieb Jacob Lifshay: > > > > > I think I found a userspace-accessible way to create sync_files and > > > > > dma_fences that would fulfill the requirements: > > > > > https://github.com/torvalds/linux/blob/master/drivers/dma-buf/sw_sync.c > > > > > > > > > > I'm just not sure if that's a good interface to use, since it appears > > > > > to be designed only for debugging. Will have to check for additional > > > > > requirements of signalling an error when the process that created the > > > > > fence is killed. > > > > It is expressly only for debugging and testing. Exposing such an API > > to userspace would break the finite time guarantees that are relied > > upon to keep sync_file a secure API. > > Ok, I was figuring that was probably the case. > > > > > Something like that can certainly be lifted for general use if it makes > > > > sense. But then with a software renderer I don't really see how fences > > > > help you at all. With a software renderer you know exactly when the > > > > frame is finished and you can just defer pushing it over to the next > > > > pipeline element until that time. You won't gain any parallelism by > > > > using fences as the CPU is busy doing the rendering and will not run > > > > other stuff concurrently, right? > > > > > > There definitely may be other hardware and/or processes that can > > > process some stuff concurrently with the main application, such as the > > > compositor and or video encoding processes (for video capture). > > > Additionally, from what I understand, sync_file is the standard way to > > > export and import explicit synchronization between processes and > > > between drivers on Linux, so it seems like a good idea to support it > > > from an interoperability standpoint even if it turns out that there > > > aren't any scheduling/timing benefits. > > > > There are different ways that one can handle interoperability, > > however. One way is to try and make the software rasterizer look as > > much like a GPU as possible: lots of threads to make things as > > asynchronous as possible, "real" implementations of semaphores and > > fences, etc. > > This is basically the route I've picked, though rather than making > lots of native threads, I'm planning on having just one thread per > core and have a work-stealing scheduler (inspired by Rust's rayon > crate) schedule all the individual render/compute jobs, because that > allows making a lot more jobs to allow finer load balancing. > > > Another is to let a SW rasterizer be a SW rasterizer: do > > everything immediately, thread only so you can exercise all the CPU > > cores, and minimally implement semaphores and fences well enough to > > maintain compatibility. If you take the first approach, then we have > > to solve all these problems with letting userspace create unsignaled > > sync_files which it will signal later and figure out how to make it > > safe. If you take the second approach, you'll only ever have to > > return already signaled sync_files and there's no problem with the > > sync_file finite time guarantees. > > The main issue with doing everything immediately is that a lot of the > function calls that games expect to take a very short time (e.g. > vkQueueSubmit) would instead take a much longer time, potentially > causing problems. Do you have any evidence that it will cause problems? What I said above is what switfshader is doing and they're running real apps and I've not heard of it causing any problems. It's also worth noting that you would only really have to stall at sync_file export. You can async as much as you want internally. > One idea for a safe userspace-backed sync_file is to have a step > counter that counts down until the sync_file is ready, where if > userspace doesn't tell it to count any steps in a certain amount of > time, then the sync_file switches to the error state. This way, it > will error shortly after a process deadlocks for some reason, while > still having the finite-time guarantee. > > When the sync_file is created, the step counter would be set to th
Re: [Mesa-dev] XDC 2020 feedback and comments
First off, I think you all did a fantastic job. I felt that things ran very smoothly and, as far as the talks themselves go, I think it went almost as smoothly as an in-person XDC. I'm really quite impressed. I do have a couple pieces of more nuanced feedback: 1. I think we were maybe a bit too scared of overloading jitsi. Having more people in the instance for questions might have made that portion go better. As it was, there was only one or two talks that had any live questions. That said, there are a few advantages to having things funneled through IRC, the most obvious of which being that people can ask their question mid-talk and have it handled at the end instead of having to remember it for 20 minutes. 2. I really miss the hallway track. On Thursday, after the conference, Bas, Connor, and I used jitsi to have a chat about ray-tracing. That was really fun and I wish I'd done something like that every day of XDC. Maybe it's my own fault for not setting up said chats but I think it could have been made more accessible (I had no idea how to fork off a jitsi instance) and/or encouraged somehow. --Jason On Mon, Sep 21, 2020 at 3:07 AM Samuel Iglesias Gonsálvez wrote: > > Hi all, > > Huge thanks again to the entire team from Intel, for their great work > organizing XDC 2020, our first virtual conference! > > As usual we're looking for feedback on both XDC itself, and the CFP > process and program selection. Both about what was great and should be > kept for next year's edition, and where there's room for improvement. > > The board does keep some notes, for those interested in what we have > already: > > - XDC notes for prospective organizers: > https://www.x.org/wiki/Events/RFP/ > > - CFP notes: https://www.x.org/wiki/Events/PapersCommittee/ > > If you want to send in your comments in private, please send them to > the X.org Foundation board: bo...@foundation.x.org > > Cheers, > > Sam > ___ > mesa-dev mailing list > mesa-...@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/mesa-dev ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH 3/3] dma-buf: Add an API for exporting sync files (v8)
Adding mesa-dev and wayland-devel for broader circulation.
On Wed, Mar 17, 2021 at 5:19 PM Jason Ekstrand wrote:
>
> Modern userspace APIs like Vulkan are built on an explicit
> synchronization model. This doesn't always play nicely with the
> implicit synchronization used in the kernel and assumed by X11 and
> Wayland. The client -> compositor half of the synchronization isn't too
> bad, at least on intel, because we can control whether or not i915
> synchronizes on the buffer and whether or not it's considered written.
>
> The harder part is the compositor -> client synchronization when we get
> the buffer back from the compositor. We're required to be able to
> provide the client with a VkSemaphore and VkFence representing the point
> in time where the window system (compositor and/or display) finished
> using the buffer. With current APIs, it's very hard to do this in such
> a way that we don't get confused by the Vulkan driver's access of the
> buffer. In particular, once we tell the kernel that we're rendering to
> the buffer again, any CPU waits on the buffer or GPU dependencies will
> wait on some of the client rendering and not just the compositor.
>
> This new IOCTL solves this problem by allowing us to get a snapshot of
> the implicit synchronization state of a given dma-buf in the form of a
> sync file. It's effectively the same as a poll() or I915_GEM_WAIT only,
> instead of CPU waiting directly, it encapsulates the wait operation, at
> the current moment in time, in a sync_file so we can check/wait on it
> later. As long as the Vulkan driver does the sync_file export from the
> dma-buf before we re-introduce it for rendering, it will only contain
> fences from the compositor or display. This allows to accurately turn
> it into a VkFence or VkSemaphore without any over- synchronization.
>
> v2 (Jason Ekstrand):
> - Use a wrapper dma_fence_array of all fences including the new one
>when importing an exclusive fence.
>
> v3 (Jason Ekstrand):
> - Lock around setting shared fences as well as exclusive
> - Mark SIGNAL_SYNC_FILE as a read-write ioctl.
> - Initialize ret to 0 in dma_buf_wait_sync_file
>
> v4 (Jason Ekstrand):
> - Use the new dma_resv_get_singleton helper
>
> v5 (Jason Ekstrand):
> - Rename the IOCTLs to import/export rather than wait/signal
> - Drop the WRITE flag and always get/set the exclusive fence
>
> v6 (Jason Ekstrand):
> - Drop the sync_file import as it was all-around sketchy and not nearly
>as useful as import.
> - Re-introduce READ/WRITE flag support for export
> - Rework the commit message
>
> v7 (Jason Ekstrand):
> - Require at least one sync flag
> - Fix a refcounting bug: dma_resv_get_excl() doesn't take a reference
> - Use _rcu helpers since we're accessing the dma_resv read-only
>
> v8 (Jason Ekstrand):
> - Return -ENOMEM if the sync_file_create fails
> - Predicate support on IS_ENABLED(CONFIG_SYNC_FILE)
>
> Signed-off-by: Jason Ekstrand
> ---
> drivers/dma-buf/dma-buf.c| 62
> include/uapi/linux/dma-buf.h | 6
> 2 files changed, 68 insertions(+)
>
> diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c
> index f264b70c383eb..a5e4b0b6a049c 100644
> --- a/drivers/dma-buf/dma-buf.c
> +++ b/drivers/dma-buf/dma-buf.c
> @@ -20,6 +20,7 @@
> #include
> #include
> #include
> +#include
> #include
> #include
> #include
> @@ -362,6 +363,62 @@ static long dma_buf_set_name(struct dma_buf *dmabuf,
> const char __user *buf)
> return ret;
> }
>
> +#if IS_ENABLED(CONFIG_SYNC_FILE)
> +static long dma_buf_export_sync_file(struct dma_buf *dmabuf,
> +void __user *user_data)
> +{
> + struct dma_buf_sync_file arg;
> + struct dma_fence *fence = NULL;
> + struct sync_file *sync_file;
> + int fd, ret;
> +
> + if (copy_from_user(&arg, user_data, sizeof(arg)))
> + return -EFAULT;
> +
> + if (arg.flags & ~DMA_BUF_SYNC_RW)
> + return -EINVAL;
> +
> + if ((arg.flags & DMA_BUF_SYNC_RW) == 0)
> + return -EINVAL;
> +
> + fd = get_unused_fd_flags(O_CLOEXEC);
> + if (fd < 0)
> + return fd;
> +
> + if (arg.flags & DMA_BUF_SYNC_WRITE) {
> + ret = dma_resv_get_singleton_rcu(dmabuf->resv, &fence);
> + if (ret)
> + goto err_put_fd;
> + } else if (arg.flags & DMA_BUF_SYNC_READ) {
> + fence = dma_resv_get_excl_rcu(dmabuf->resv);
> + }
> +
>
Re: [PATCH 3/3] dma-buf: Add an API for exporting sync files (v8)
On Tue, Mar 23, 2021 at 2:06 PM Simon Ser wrote: > > From a user-space point-of-view, this looks super useful! The uAPI sounds > good to me. > > Acked-by: Simon Ser > > Would be nice to have some short docs as well. Here's an example of a > patch adding some docs for an ioctl [1], if you aren't familiar with > that. I think just some basic stuff (description, which parameters are > in/out, what the flags are) would be great. Good call. v9 will have docs. --Jason ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH 0/6] dma-buf: Add an API for exporting sync files (v10)
Modern userspace APIs like Vulkan are built on an explicit synchronization model. This doesn't always play nicely with the implicit synchronization used in the kernel and assumed by X11 and Wayland. The client -> compositor half of the synchronization isn't too bad, at least on intel, because we can control whether or not i915 synchronizes on the buffer and whether or not it's considered written. The harder part is the compositor -> client synchronization when we get the buffer back from the compositor. We're required to be able to provide the client with a VkSemaphore and VkFence representing the point in time where the window system (compositor and/or display) finished using the buffer. With current APIs, it's very hard to do this in such a way that we don't get confused by the Vulkan driver's access of the buffer. In particular, once we tell the kernel that we're rendering to the buffer again, any CPU waits on the buffer or GPU dependencies will wait on some of the client rendering and not just the compositor. This new IOCTL solves this problem by allowing us to get a snapshot of the implicit synchronization state of a given dma-buf in the form of a sync file. It's effectively the same as a poll() or I915_GEM_WAIT only, instead of CPU waiting directly, it encapsulates the wait operation, at the current moment in time, in a sync_file so we can check/wait on it later. As long as the Vulkan driver does the sync_file export from the dma-buf before we re-introduce it for rendering, it will only contain fences from the compositor or display. This allows to accurately turn it into a VkFence or VkSemaphore without any over- synchronization. This patch series actually contains two new ioctls. There is the export one mentioned above as well as an RFC for an import ioctl which provides the other half. The intention is to land the export ioctl since it seems like there's no real disagreement on that one. The import ioctl, however, has a lot of debate around it so it's intended to be RFC-only for now. Mesa MR: https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/4037 IGT tests: https://patchwork.freedesktop.org/series/90490/ v10 (Jason Ekstrand, Daniel Vetter): - Add reviews/acks - Add a patch to rename _rcu to _unlocked - Split things better so import is clearly RFC status Cc: Christian König Cc: Michel Dänzer Cc: Dave Airlie Cc: Bas Nieuwenhuizen Cc: Daniel Stone Cc: mesa-...@lists.freedesktop.org Cc: wayland-devel@lists.freedesktop.org Test-with: 20210524205225.872316-1-ja...@jlekstrand.net Christian König (1): dma-buf: add dma_fence_array_for_each (v2) Jason Ekstrand (5): dma-buf: Rename dma_resv helpers from _rcu to _unlocked dma-buf: add dma_resv_get_singleton_unlocked (v4) dma-buf: Add an API for exporting sync files (v9) RFC: dma-buf: Add an extra fence to dma_resv_get_singleton_unlocked RFC: dma-buf: Add an API for importing sync files (v6) drivers/dma-buf/dma-buf.c | 100 - drivers/dma-buf/dma-fence-array.c | 27 drivers/dma-buf/dma-resv.c| 140 -- drivers/gpu/drm/amd/amdgpu/amdgpu_display.c | 2 +- drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c | 2 +- drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c | 2 +- drivers/gpu/drm/amd/amdgpu/amdgpu_ids.c | 4 +- drivers/gpu/drm/amd/amdgpu/amdgpu_mn.c| 2 +- drivers/gpu/drm/amd/amdgpu/amdgpu_object.c| 2 +- drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.c | 2 +- drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c| 8 +- .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 2 +- drivers/gpu/drm/drm_gem.c | 6 +- drivers/gpu/drm/drm_gem_atomic_helper.c | 2 +- drivers/gpu/drm/etnaviv/etnaviv_gem.c | 4 +- drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c | 4 +- drivers/gpu/drm/i915/display/intel_display.c | 2 +- drivers/gpu/drm/i915/dma_resv_utils.c | 2 +- drivers/gpu/drm/i915/gem/i915_gem_busy.c | 2 +- .../gpu/drm/i915/gem/i915_gem_execbuffer.c| 2 +- drivers/gpu/drm/i915/gem/i915_gem_object.h| 2 +- drivers/gpu/drm/i915/gem/i915_gem_userptr.c | 2 +- drivers/gpu/drm/i915/gem/i915_gem_wait.c | 10 +- drivers/gpu/drm/i915/i915_request.c | 4 +- drivers/gpu/drm/i915/i915_sw_fence.c | 4 +- drivers/gpu/drm/msm/msm_gem.c | 2 +- drivers/gpu/drm/nouveau/dispnv50/wndw.c | 2 +- drivers/gpu/drm/nouveau/nouveau_gem.c | 2 +- drivers/gpu/drm/panfrost/panfrost_drv.c | 2 +- drivers/gpu/drm/panfrost/panfrost_job.c | 2 +- drivers/gpu/drm/radeon/radeon_gem.c | 6 +- drivers/gpu/drm/radeon/radeon_mn.c| 2 +- drivers/gpu/drm/ttm/ttm_bo.c | 12 +- drivers/gpu/drm/vgem/vgem_fence.c | 2 +- drivers/gpu/drm/virtio/virtgpu_ioctl.c| 4 +- drivers/
[PATCH 0/7] dma-buf: Add an API for exporting sync files (v11)
Modern userspace APIs like Vulkan are built on an explicit synchronization model. This doesn't always play nicely with the implicit synchronization used in the kernel and assumed by X11 and Wayland. The client -> compositor half of the synchronization isn't too bad, at least on intel, because we can control whether or not i915 synchronizes on the buffer and whether or not it's considered written. The harder part is the compositor -> client synchronization when we get the buffer back from the compositor. We're required to be able to provide the client with a VkSemaphore and VkFence representing the point in time where the window system (compositor and/or display) finished using the buffer. With current APIs, it's very hard to do this in such a way that we don't get confused by the Vulkan driver's access of the buffer. In particular, once we tell the kernel that we're rendering to the buffer again, any CPU waits on the buffer or GPU dependencies will wait on some of the client rendering and not just the compositor. This new IOCTL solves this problem by allowing us to get a snapshot of the implicit synchronization state of a given dma-buf in the form of a sync file. It's effectively the same as a poll() or I915_GEM_WAIT only, instead of CPU waiting directly, it encapsulates the wait operation, at the current moment in time, in a sync_file so we can check/wait on it later. As long as the Vulkan driver does the sync_file export from the dma-buf before we re-introduce it for rendering, it will only contain fences from the compositor or display. This allows to accurately turn it into a VkFence or VkSemaphore without any over- synchronization. This patch series actually contains two new ioctls. There is the export one mentioned above as well as an RFC for an import ioctl which provides the other half. The intention is to land the export ioctl since it seems like there's no real disagreement on that one. The import ioctl, however, has a lot of debate around it so it's intended to be RFC-only for now. Mesa MR: https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/4037 IGT tests: https://patchwork.freedesktop.org/series/90490/ v10 (Jason Ekstrand, Daniel Vetter): - Add reviews/acks - Add a patch to rename _rcu to _unlocked - Split things better so import is clearly RFC status v11 (Daniel Vetter): - Add more CCs to try and get maintainers - Add a patch to document DMA_BUF_IOCTL_SYNC - Generally better docs - Use separate structs for import/export (easier to document) - Fix an issue in the import patch Cc: Christian König Cc: Michel Dänzer Cc: Dave Airlie Cc: Bas Nieuwenhuizen Cc: Daniel Stone Cc: mesa-...@lists.freedesktop.org Cc: wayland-devel@lists.freedesktop.org Test-with: 20210524205225.872316-1-ja...@jlekstrand.net Christian König (1): dma-buf: Add dma_fence_array_for_each (v2) Jason Ekstrand (6): dma-buf: Rename dma_resv helpers from _rcu to _unlocked (v2) dma-buf: Add dma_resv_get_singleton_unlocked (v5) dma-buf: Document DMA_BUF_IOCTL_SYNC dma-buf: Add an API for exporting sync files (v11) RFC: dma-buf: Add an extra fence to dma_resv_get_singleton_unlocked RFC: dma-buf: Add an API for importing sync files (v7) Documentation/driver-api/dma-buf.rst | 8 + drivers/dma-buf/dma-buf.c | 107 +- drivers/dma-buf/dma-fence-array.c | 27 drivers/dma-buf/dma-resv.c| 139 -- drivers/gpu/drm/amd/amdgpu/amdgpu_display.c | 6 +- drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c | 2 +- drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c | 4 +- drivers/gpu/drm/amd/amdgpu/amdgpu_ids.c | 6 +- drivers/gpu/drm/amd/amdgpu/amdgpu_mn.c| 4 +- drivers/gpu/drm/amd/amdgpu/amdgpu_object.c| 4 +- drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.c | 6 +- drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c| 14 +- .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 6 +- drivers/gpu/drm/drm_gem.c | 10 +- drivers/gpu/drm/drm_gem_atomic_helper.c | 2 +- drivers/gpu/drm/etnaviv/etnaviv_gem.c | 7 +- drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c | 8 +- drivers/gpu/drm/i915/display/intel_display.c | 2 +- drivers/gpu/drm/i915/dma_resv_utils.c | 2 +- drivers/gpu/drm/i915/gem/i915_gem_busy.c | 2 +- .../gpu/drm/i915/gem/i915_gem_execbuffer.c| 2 +- drivers/gpu/drm/i915/gem/i915_gem_object.h| 2 +- drivers/gpu/drm/i915/gem/i915_gem_userptr.c | 4 +- drivers/gpu/drm/i915/gem/i915_gem_wait.c | 10 +- drivers/gpu/drm/i915/i915_request.c | 6 +- drivers/gpu/drm/i915/i915_sw_fence.c | 4 +- drivers/gpu/drm/msm/msm_gem.c | 3 +- drivers/gpu/drm/nouveau/dispnv50/wndw.c | 2 +- drivers/gpu/drm/nouveau/nouveau_gem.c | 4 +- drivers/gpu/drm/panfrost/panfrost_drv.c | 4 +- drivers/gp
Re: [PATCH 0/7] dma-buf: Add an API for exporting sync files (v11)
On Thu, Jun 10, 2021 at 3:11 PM Chia-I Wu wrote: > > On Tue, May 25, 2021 at 2:18 PM Jason Ekstrand wrote: > > Modern userspace APIs like Vulkan are built on an explicit > > synchronization model. This doesn't always play nicely with the > > implicit synchronization used in the kernel and assumed by X11 and > > Wayland. The client -> compositor half of the synchronization isn't too > > bad, at least on intel, because we can control whether or not i915 > > synchronizes on the buffer and whether or not it's considered written. > We might have an important use case for this half, for virtio-gpu and Chrome > OS. > > When the guest compositor acts as a proxy to connect guest apps to the > host compositor, implicit fencing requires the guest compositor to do > a wait before forwarding the buffer to the host compositor. With this > patch, the guest compositor can extract the dma-fence from the buffer, > and if the fence is a virtio-gpu fence, forward both the fence and the > buffer to the host compositor. It will allow us to convert a > guest-side wait into a host-side wait. Yeah, I think the first half solves a lot of problems. I'm rebasing it now and will send a v12 series shortly. I don't think there's a lot standing between the first few patches and merging. I've got IGT tests and I'm pretty sure the code is good. The last review cycle got distracted with some renaming fun. --Jason ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH 0/6] dma-buf: Add an API for exporting sync files (v12)
Modern userspace APIs like Vulkan are built on an explicit synchronization model. This doesn't always play nicely with the implicit synchronization used in the kernel and assumed by X11 and Wayland. The client -> compositor half of the synchronization isn't too bad, at least on intel, because we can control whether or not i915 synchronizes on the buffer and whether or not it's considered written. The harder part is the compositor -> client synchronization when we get the buffer back from the compositor. We're required to be able to provide the client with a VkSemaphore and VkFence representing the point in time where the window system (compositor and/or display) finished using the buffer. With current APIs, it's very hard to do this in such a way that we don't get confused by the Vulkan driver's access of the buffer. In particular, once we tell the kernel that we're rendering to the buffer again, any CPU waits on the buffer or GPU dependencies will wait on some of the client rendering and not just the compositor. This new IOCTL solves this problem by allowing us to get a snapshot of the implicit synchronization state of a given dma-buf in the form of a sync file. It's effectively the same as a poll() or I915_GEM_WAIT only, instead of CPU waiting directly, it encapsulates the wait operation, at the current moment in time, in a sync_file so we can check/wait on it later. As long as the Vulkan driver does the sync_file export from the dma-buf before we re-introduce it for rendering, it will only contain fences from the compositor or display. This allows to accurately turn it into a VkFence or VkSemaphore without any over- synchronization. This patch series actually contains two new ioctls. There is the export one mentioned above as well as an RFC for an import ioctl which provides the other half. The intention is to land the export ioctl since it seems like there's no real disagreement on that one. The import ioctl, however, has a lot of debate around it so it's intended to be RFC-only for now. Mesa MR: https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/4037 IGT tests: https://patchwork.freedesktop.org/series/90490/ v10 (Jason Ekstrand, Daniel Vetter): - Add reviews/acks - Add a patch to rename _rcu to _unlocked - Split things better so import is clearly RFC status v11 (Daniel Vetter): - Add more CCs to try and get maintainers - Add a patch to document DMA_BUF_IOCTL_SYNC - Generally better docs - Use separate structs for import/export (easier to document) - Fix an issue in the import patch v12 (Daniel Vetter): - Better docs for DMA_BUF_IOCTL_SYNC v12 (Christian König): - Drop the rename patch in favor of Christian's series - Add a comment to the commit message for the dma-buf sync_file export ioctl saying why we made it an ioctl on dma-buf Cc: Christian König Cc: Michel Dänzer Cc: Dave Airlie Cc: Bas Nieuwenhuizen Cc: Daniel Stone Cc: mesa-...@lists.freedesktop.org Cc: wayland-devel@lists.freedesktop.org Test-with: 20210524205225.872316-1-ja...@jlekstrand.net Christian König (1): dma-buf: Add dma_fence_array_for_each (v2) Jason Ekstrand (5): dma-buf: Add dma_resv_get_singleton (v6) dma-buf: Document DMA_BUF_IOCTL_SYNC (v2) dma-buf: Add an API for exporting sync files (v12) RFC: dma-buf: Add an extra fence to dma_resv_get_singleton_unlocked RFC: dma-buf: Add an API for importing sync files (v7) Documentation/driver-api/dma-buf.rst | 8 ++ drivers/dma-buf/dma-buf.c| 103 + drivers/dma-buf/dma-fence-array.c| 27 +++ drivers/dma-buf/dma-resv.c | 110 +++ include/linux/dma-fence-array.h | 17 + include/linux/dma-resv.h | 2 + include/uapi/linux/dma-buf.h | 103 - 7 files changed, 369 insertions(+), 1 deletion(-) -- 2.31.1 ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [Mesa-dev] [PATCH 0/6] dma-buf: Add an API for exporting sync files (v12)
On Tue, Jun 15, 2021 at 3:41 AM Christian König wrote: > > Hi Jason & Daniel, > > maybe I should explain once more where the problem with this approach is > and why I think we need to get that fixed before we can do something > like this here. > > To summarize what this patch here does is that it copies the exclusive > fence and/or the shared fences into a sync_file. This alone is totally > unproblematic. > > The problem is what this implies. When you need to copy the exclusive > fence to a sync_file then this means that the driver is at some point > ignoring the exclusive fence on a buffer object. Not necessarily. Part of the point of this is to allow for CPU waits on a past point in buffers timeline. Today, we have poll() and GEM_WAIT both of which wait for the buffer to be idle from whatever GPU work is currently happening. We want to wait on something in the past and ignore anything happening now. But, to the broader point, maybe? I'm a little fuzzy on exactly where i915 inserts and/or depends on fences. > When you combine that with complex drivers which use TTM and buffer > moves underneath you can construct an information leak using this and > give userspace access to memory which is allocated to the driver, but > not yet initialized. > > This way you can leak things like page tables, passwords, kernel data > etc... in large amounts to userspace and is an absolutely no-go for > security. Ugh... Unfortunately, I'm really out of my depth on the implications going on here but I think I see your point. > That's why I'm said we need to get this fixed before we upstream this > patch set here and especially the driver change which is using that. Well, i915 has had uAPI for a while to ignore fences. Those changes are years in the past. If we have a real problem here (not sure on that yet), then we'll have to figure out how to fix it without nuking uAPI. --Jason > Regards, > Christian. > > Am 10.06.21 um 23:09 schrieb Jason Ekstrand: > > Modern userspace APIs like Vulkan are built on an explicit > > synchronization model. This doesn't always play nicely with the > > implicit synchronization used in the kernel and assumed by X11 and > > Wayland. The client -> compositor half of the synchronization isn't too > > bad, at least on intel, because we can control whether or not i915 > > synchronizes on the buffer and whether or not it's considered written. > > > > The harder part is the compositor -> client synchronization when we get > > the buffer back from the compositor. We're required to be able to > > provide the client with a VkSemaphore and VkFence representing the point > > in time where the window system (compositor and/or display) finished > > using the buffer. With current APIs, it's very hard to do this in such > > a way that we don't get confused by the Vulkan driver's access of the > > buffer. In particular, once we tell the kernel that we're rendering to > > the buffer again, any CPU waits on the buffer or GPU dependencies will > > wait on some of the client rendering and not just the compositor. > > > > This new IOCTL solves this problem by allowing us to get a snapshot of > > the implicit synchronization state of a given dma-buf in the form of a > > sync file. It's effectively the same as a poll() or I915_GEM_WAIT only, > > instead of CPU waiting directly, it encapsulates the wait operation, at > > the current moment in time, in a sync_file so we can check/wait on it > > later. As long as the Vulkan driver does the sync_file export from the > > dma-buf before we re-introduce it for rendering, it will only contain > > fences from the compositor or display. This allows to accurately turn > > it into a VkFence or VkSemaphore without any over- synchronization. > > > > This patch series actually contains two new ioctls. There is the export > > one mentioned above as well as an RFC for an import ioctl which provides > > the other half. The intention is to land the export ioctl since it seems > > like there's no real disagreement on that one. The import ioctl, however, > > has a lot of debate around it so it's intended to be RFC-only for now. > > > > Mesa MR: https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/4037 > > IGT tests: https://patchwork.freedesktop.org/series/90490/ > > > > v10 (Jason Ekstrand, Daniel Vetter): > > - Add reviews/acks > > - Add a patch to rename _rcu to _unlocked > > - Split things better so import is clearly RFC status > > > > v11 (Daniel Vetter): > > - Add more CCs to try and get maintainers > > - Add a
Re: [Mesa-dev] [PATCH 0/6] dma-buf: Add an API for exporting sync files (v12)
ot > > actually a ttm_bo->moving fence we can ignore. > > > > p2p dma-buf aka dynamic dma-buf is a different beast, and i915 (and fwiw > > these other drivers) need to change before they can do dynamic dma-buf. > > > >> Otherwise we have an information leak worth a CVE and that is certainly not > >> something we want. > > Because yes otherwise we get a CVE. But right now I don't think we have > > one. > > Yeah, agree. But this is just because of coincident and not because of > good engineering :) > > > We do have a quite big confusion on what exactly the signaling ordering is > > supposed to be between exclusive and the collective set of shared fences, > > and there's some unifying that needs to happen here. But I think what > > Jason implements here in the import ioctl is the most defensive version > > possible, so really can't break any driver. It really works like you have > > an ad-hoc gpu engine that does nothing itself, but waits for the current > > exclusive fence and then sets the exclusive fence with its "CS" completion > > fence. > > > > That's imo perfectly legit use-case. > > The use case is certainly legit, but I'm not sure if merging this at the > moment is a good idea. > > Your note that drivers are already ignoring the exclusive fence in the > dma_resv object was eye opening to me. And I now have the very strong > feeling that the synchronization and the design of the dma_resv object > is even more messy then I thought it is. > > To summarize we can be really lucky that it didn't blow up into our > faces already. > > > Same for the export one. Waiting for a previous snapshot of implicit > > fences is imo perfectly ok use-case and useful for compositors - client > > might soon start more rendering, and on some drivers that always results > > in the exclusive slot being set, so if you dont take a snapshot you > > oversync real bad for your atomic flip. > > The export use case is unproblematic as far as I can see. Then why are we holding it up? I'm not asking to have import merged. That's still labled RFC and I've said over and over that it's not baked and I'm not convinced it helps all that much. > >>> Those changes are years in the past. If we have a real problem here (not > >>> sure on > >>> that yet), then we'll have to figure out how to fix it without nuking > >>> uAPI. > >> Well, that was the basic idea of attaching flags to the fences in the > >> dma_resv object. > >> > >> In other words you clearly denote when you have to wait for a fence before > >> accessing a buffer or you cause a security issue. > > Replied somewhere else, and I do kinda like the flag idea. But the problem > > is we first need a ton more encapsulation and review of drivers before we > > can change the internals. One thing at a time. I think I'm warming to flags as well. I didn't like them at first but I think they actually make quite a bit of sense. Unlike the explicit fence where ignoring it can lead to a security hole, the worst that happens if someone forgets a flag somewhere is a bit of memory corruption and garbage on-screen. That seems fairly non-dangerous to me. > Ok how should we then proceed? > > The large patch set I've send out to convert all users of the shared > fence list to a for_each API is a step into the right direction I think, > but there is still a bit more todo. > > > And yes for amdgpu this gets triple-hard because you both have the > > ttm_bo->moving fence _and_ the current uapi of using fence ownership _and_ > > you need to figure out how to support vulkan properly with true opt-in > > fencing. > > Well I have been pondering on that for a bit and I came to the > conclusion that it is actually not a problem at all. > > See radeon, nouveau, msm etc... all implement functions that they don't > wait for fences from the same timeline, context, engine. That amdgpu > doesn't wait for fences from the same process can be seen as just a > special case of this. That doesn't totally solve the issue, though. RADV suffers in the WSI arena today from too much cross-process implicit sharing. I do think you want some sort of "ignore implicit sync" API but, in this new world of flags, it would look more like "don't bother looking for shared fences flagged write". You'd still respect the exclusive fence, if there is one, and you'd still add a shared fence. You just wouldn't bother with implicit sync. Should be safe, right? --Jason > > I'm pretty sure it's doable,
[PATCH 0/2] *dma-buf: Add an API for exporting sync files (v13)
Modern userspace APIs like Vulkan are built on an explicit synchronization model. This doesn't always play nicely with the implicit synchronization used in the kernel and assumed by X11 and Wayland. The client -> compositor half of the synchronization isn't too bad, at least on intel, because we can control whether or not i915 synchronizes on the buffer and whether or not it's considered written. The harder part is the compositor -> client synchronization when we get the buffer back from the compositor. We're required to be able to provide the client with a VkSemaphore and VkFence representing the point in time where the window system (compositor and/or display) finished using the buffer. With current APIs, it's very hard to do this in such a way that we don't get confused by the Vulkan driver's access of the buffer. In particular, once we tell the kernel that we're rendering to the buffer again, any CPU waits on the buffer or GPU dependencies will wait on some of the client rendering and not just the compositor. This new IOCTL solves this problem by allowing us to get a snapshot of the implicit synchronization state of a given dma-buf in the form of a sync file. It's effectively the same as a poll() or I915_GEM_WAIT only, instead of CPU waiting directly, it encapsulates the wait operation, at the current moment in time, in a sync_file so we can check/wait on it later. As long as the Vulkan driver does the sync_file export from the dma-buf before we re-introduce it for rendering, it will only contain fences from the compositor or display. This allows to accurately turn it into a VkFence or VkSemaphore without any over-synchronization. This patch series actually contains two new ioctls. There is the export one mentioned above as well as an RFC for an import ioctl which provides the other half. The intention is to land the export ioctl since it seems like there's no real disagreement on that one. The import ioctl, however, has a lot of debate around it so it's intended to be RFC-only for now. Mesa MR: https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/4037 IGT tests: https://patchwork.freedesktop.org/series/90490/ v10 (Jason Ekstrand, Daniel Vetter): - Add reviews/acks - Add a patch to rename _rcu to _unlocked - Split things better so import is clearly RFC status v11 (Daniel Vetter): - Add more CCs to try and get maintainers - Add a patch to document DMA_BUF_IOCTL_SYNC - Generally better docs - Use separate structs for import/export (easier to document) - Fix an issue in the import patch v12 (Daniel Vetter): - Better docs for DMA_BUF_IOCTL_SYNC v12 (Christian König): - Drop the rename patch in favor of Christian's series - Add a comment to the commit message for the dma-buf sync_file export ioctl saying why we made it an ioctl on dma-buf v13 (Jason Ekstrand): - Rebase on Christian König's fence rework Cc: Christian König Cc: Michel Dänzer Cc: Dave Airlie Cc: Bas Nieuwenhuizen Cc: Daniel Stone Cc: mesa-...@lists.freedesktop.org Cc: wayland-devel@lists.freedesktop.org Jason Ekstrand (2): dma-buf: Add an API for exporting sync files (v13) dma-buf: Add an API for importing sync files (v8) drivers/dma-buf/dma-buf.c| 100 +++ include/uapi/linux/dma-buf.h | 57 2 files changed, 157 insertions(+) -- 2.36.0
[PATCH 0/2] dma-buf: Add an API for exporting sync files (v13)
Modern userspace APIs like Vulkan are built on an explicit synchronization model. This doesn't always play nicely with the implicit synchronization used in the kernel and assumed by X11 and Wayland. The client -> compositor half of the synchronization isn't too bad, at least on intel, because we can control whether or not i915 synchronizes on the buffer and whether or not it's considered written. The harder part is the compositor -> client synchronization when we get the buffer back from the compositor. We're required to be able to provide the client with a VkSemaphore and VkFence representing the point in time where the window system (compositor and/or display) finished using the buffer. With current APIs, it's very hard to do this in such a way that we don't get confused by the Vulkan driver's access of the buffer. In particular, once we tell the kernel that we're rendering to the buffer again, any CPU waits on the buffer or GPU dependencies will wait on some of the client rendering and not just the compositor. This new IOCTL solves this problem by allowing us to get a snapshot of the implicit synchronization state of a given dma-buf in the form of a sync file. It's effectively the same as a poll() or I915_GEM_WAIT only, instead of CPU waiting directly, it encapsulates the wait operation, at the current moment in time, in a sync_file so we can check/wait on it later. As long as the Vulkan driver does the sync_file export from the dma-buf before we re-introduce it for rendering, it will only contain fences from the compositor or display. This allows to accurately turn it into a VkFence or VkSemaphore without any over-synchronization. This patch series actually contains two new ioctls. There is the export one mentioned above as well as an RFC for an import ioctl which provides the other half. The intention is to land the export ioctl since it seems like there's no real disagreement on that one. The import ioctl, however, has a lot of debate around it so it's intended to be RFC-only for now. Mesa MR: https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/4037 IGT tests: https://patchwork.freedesktop.org/series/90490/ v10 (Jason Ekstrand, Daniel Vetter): - Add reviews/acks - Add a patch to rename _rcu to _unlocked - Split things better so import is clearly RFC status v11 (Daniel Vetter): - Add more CCs to try and get maintainers - Add a patch to document DMA_BUF_IOCTL_SYNC - Generally better docs - Use separate structs for import/export (easier to document) - Fix an issue in the import patch v12 (Daniel Vetter): - Better docs for DMA_BUF_IOCTL_SYNC v12 (Christian König): - Drop the rename patch in favor of Christian's series - Add a comment to the commit message for the dma-buf sync_file export ioctl saying why we made it an ioctl on dma-buf v13 (Jason Ekstrand): - Rebase on Christian König's fence rework Cc: Christian König Cc: Michel Dänzer Cc: Dave Airlie Cc: Bas Nieuwenhuizen Cc: Daniel Stone Cc: mesa-...@lists.freedesktop.org Cc: wayland-devel@lists.freedesktop.org Jason Ekstrand (2): dma-buf: Add an API for exporting sync files (v13) dma-buf: Add an API for importing sync files (v8) drivers/dma-buf/dma-buf.c| 100 +++ include/uapi/linux/dma-buf.h | 57 2 files changed, 157 insertions(+) -- 2.36.0
Re: [RFC wayland 00/18] Wayland network transparency :)
On Tue, Feb 9, 2016 at 8:55 AM, Derek Foreman wrote: > I saw a presentation once where someone said that even the simplest > implementation of wayland network transparency that just pushed the > existing protocol over tcp/ip would still be better than X. > > Let's test that theory. > Hint - application startup time is shockingly fast due to wayland's > excellent protocol design, but my poor damage tracking and lack of > compression hurts. Damage tracking is especially bad for anything > that doesn't use wl_surface.damage_buffer() - so weston-terminal may > be rough but terminology is much snappier. > Kristian came up with a very nifty little image diffing algorithm based on a rolling hash that's used to provide compression for the screen recorder. (The actual video recorder, not screenshooter.) You might want to check that out. > > To try it out (and open a gaping security hole because I've punted on > kind of authentication), just add: > wl_display_add_remote_socket(display, "foo"); > to weston right before load_modules in main. Use at your own risk. > > That's a literal "foo" because I haven't bothered defining how that > name string changes the port. > > I've written up a short blog post over here: > http://blogs.s-osg.org/wow-wayland-over-wire/ > > (It lists other decisions I haven't bothered to make for this trial > run.) > > For those of you that want to view PDF files over the network, EFL's > etui viewer comes highly recommended. ;) > > Derek Foreman (17): > os-compatability: Allow creation of 0 byte anonymous files > os: make set_cloexec_or_close private instead of static > cursor: use wl_os_set_cloexec_or_close instead of local copy > os-compatability: Remove cursor's private os compat stuff entirely > client: Check remaining connection buffer status after each > queue_event() > protocol: Add fd_static type > protocol: Use new fd_static type for keymaps > os: Add wl_os_resize_file() > scanner: Add the concept of "pre hooks" > os: Add wl_os_read() and wl_os_write() > os: Add a wl_os_socket_reuseaddr > connections: Add remote sockets > shm: properly resize remote buffers > connection: support sending the contents of fds as bulk data > connection: Use bulk transfers for fd_static on remote connections > protocol: Add a wl_buffer.transmit > protocol: Add hooks for network transparency > > Giulio Camuffo (1): > shm: add getters for the fd and offset > > Makefile.am | 5 +- > cursor/os-compatibility.c | 148 - > cursor/os-compatibility.h | 34 -- > cursor/wayland-cursor.c | 15 +-- > protocol/wayland.dtd | 1 + > protocol/wayland.xml | 40 --- > src/connection.c | 150 +- > src/network-client.c | 269 > ++ > src/scanner.c | 117 ++-- > src/wayland-client-core.h | 10 ++ > src/wayland-client.c | 120 + > src/wayland-os.c | 162 ++-- > src/wayland-os.h | 19 > src/wayland-private.h | 13 +++ > src/wayland-server-core.h | 13 +++ > src/wayland-server.c | 146 +++-- > src/wayland-shm.c | 63 ++- > 17 files changed, 1045 insertions(+), 280 deletions(-) > delete mode 100644 cursor/os-compatibility.c > delete mode 100644 cursor/os-compatibility.h > create mode 100644 src/network-client.c > > -- > 2.7.0 > > ___ > wayland-devel mailing list > wayland-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/wayland-devel > ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [RFC wayland] protocol: Add wl_surface.buffer_damage
Thanks for picking this up! Also, thanks for posting this on the bug, I would have missed it otherwise! On Thu, Nov 12, 2015 at 11:16 AM, Derek Foreman wrote: > On 09/11/15 09:06 AM, Pekka Paalanen wrote: >> On Fri, 6 Nov 2015 12:55:19 -0600 >> Derek Foreman wrote: >> >>> wl_surface.damage uses surface local co-ordinates. >>> >>> Buffer scale and buffer transforms came along, and EGL surfaces >>> have no understanding of them. >>> >>> Theoretically, clients pass damage rectangles - in Y-inverted surface >>> co-ordinates) to EGLSwapBuffersWithDamage, and the EGL implementation >>> passed them on to wayland. However, for this to work the EGL >>> implementation must be able to flip those rectangles into the space >>> the compositor is expecting, but it's unable to do so because it >>> doesn't know the height of the transformed buffer. >>> >>> So, currently, EGLSwapBuffersWithDamage is unusable and EGLSwapBuffers >>> has to pass (0,0) - (INT32_MAX, INT32_MAX) damage to function. >>> >>> wl_surface.buffer_damage allows damage to be registered on a surface >>> in buffer co-ordinates, avoiding this problem. >>> >>> Credit where it's due, these ideas are not entirely my own: >>> Over a year ago the idea of changing damage co-ordinates to buffer >>> co-ordinates was suggested (by Jason Ekstrand), and it was at least >>> partially rejected and abandoned. At the time it was also suggested >>> (by Pekka Paalanen) that adding a new wl_surface.buffer_damage request >>> was another option. >>> >> >> Hi Derek, >> >> please mention https://bugs.freedesktop.org/show_bug.cgi?id=78190 in >> this patch. > > Will do. > >>> Signed-off-by: Derek Foreman >>> --- >>> >>> Necro-posting on: >>> http://lists.freedesktop.org/archives/wayland-devel/2014-February/013440.html >>> and >>> http://lists.freedesktop.org/archives/wayland-devel/2014-February/013442.html >>> >>> This came up on IRC again the other day, and it's still an unsolved >>> problem... >>> I'm posting this as RFC to see if anyone's interested in it - I'll do an >>> implementation if we can get an agreement on the protocol text. >> >> Thanks for picking this up! > > Since all the conflict seems to be around how aggressively we should > encourage people to use this instead of the existing surface damage > request, I'll write a weston implementation. > >>> protocol/wayland.xml | 44 ++-- >>> 1 file changed, 42 insertions(+), 2 deletions(-) >>> >>> diff --git a/protocol/wayland.xml b/protocol/wayland.xml >>> index 9c22d45..1cb2f66 100644 >>> --- a/protocol/wayland.xml >>> +++ b/protocol/wayland.xml >>> @@ -176,7 +176,7 @@ >>> >>> >>> >>> - >>> + >>> >>>A compositor. This object is a singleton global. The >>>compositor is in charge of combining the contents of multiple >>> @@ -986,7 +986,7 @@ >>> >>> >>> >>> - >>> + >>> >>>A surface is a rectangular area that is displayed on the screen. >>>It has a location, size and pixel contents. >>> @@ -1327,6 +1327,46 @@ >>> >>> >>> >> >> I know Jasper suggested deprecating wl_surface.damage, but I see no >> reason for that. wl_surface.damage is well-defined - it is only misused. >> >> We could add some wording to have both refer to each other as an >> alternative way to post damage. Especially to wl_surface.damage should >> mention buffer_damage as doing what you'd usually expect. > > Ok, that sounds viable. > >>> + >>> + >>> + >> >> The name "buffer_damage" is slightly unfortunate. See: >> https://www.khronos.org/registry/egl/extensions/KHR/EGL_KHR_swap_buffers_with_damage.txt >> >> What we are doing in Wayland is always "surface damage"[*], while that >> EGL extension reserves "buffer damage" for a different purpose. I feel >> it might be confusing. >> >> Could we come up with a another name for this request? >> - wl_surface.damage_pixels >> - wl_surface.damage_by_buffer >> >> eh... buffer_damage is fine if nothing else sticks. The specification >> language below is clear anyway, I
Re: [RFC wayland] Track protocol object versions inside wl_proxy.
On Fri, Nov 13, 2015 at 3:18 AM, Giulio Camuffo wrote:
> So do i understand correctly that if the app creating the
> wl_compositor was built against a libwayland without this patch the
> version returned in mesa by wl_proxy_get_version() for every proxy
> will be always 1?
Yes, see also the second e-mail link below. I proposed to make
wl_display version 0 so that libraries can detect whether the client
was built against old or new libwayland.
--Jason
> 2015-11-12 22:01 GMT+02:00 Derek Foreman :
>> From: Jason Ekstrand
>>
>> This provides a standardized mechanism for tracking protocol object
>> versions in client code. The wl_display object is created with version 1.
>> Every time an object is created from within wl_registry_bind, it gets the
>> bound version. Every other time an object is created, it simply inherits
>> it's version from the parent object that created it.
>>
>> ---
>> Sooo, seems to finally fix the EGLSwapBuffersWithDamage problem, we also
>> need this patch. However, it's not complete.
>>
>> I've rebased it and updated it. Here's the original posting:
>> http://lists.freedesktop.org/archives/wayland-devel/2014-April/014004.html
>>
>> Of special importance is:
>> http://lists.freedesktop.org/archives/wayland-devel/2014-April/014011.html
>>
>> And the proposed solution to the new backwards compatibility issue. If we
>> get a consensus on a way forward, I can pick this up and finish it off...
>>
>> src/scanner.c | 29
>> src/wayland-client-core.h | 14 ++
>> src/wayland-client.c | 112
>> +++---
>> 3 files changed, 140 insertions(+), 15 deletions(-)
>>
>> diff --git a/src/scanner.c b/src/scanner.c
>> index 8ecdd56..850e72a 100644
>> --- a/src/scanner.c
>> +++ b/src/scanner.c
>> @@ -889,6 +889,14 @@ emit_stubs(struct wl_list *message_list, struct
>> interface *interface)
>>interface->name, interface->name, interface->name,
>>interface->name);
>>
>> + printf("static inline uint32_t\n"
>> + "%s_get_version(struct %s *%s)\n"
>> + "{\n"
>> + "\treturn wl_proxy_get_version((struct wl_proxy *) %s);\n"
>> + "}\n\n",
>> + interface->name, interface->name, interface->name,
>> + interface->name);
>> +
>> has_destructor = 0;
>> has_destroy = 0;
>> wl_list_for_each(m, message_list, link) {
>> @@ -960,20 +968,25 @@ emit_stubs(struct wl_list *message_list, struct
>> interface *interface)
>>
>> printf(")\n"
>>"{\n");
>> - if (ret) {
>> + if (ret && ret->interface_name == NULL) {
>> printf("\tstruct wl_proxy *%s;\n\n"
>> - "\t%s = wl_proxy_marshal_constructor("
>> + "\t%s =
>> wl_proxy_marshal_constructor_versioned("
>>"(struct wl_proxy *) %s,\n"
>> - "\t\t\t %s_%s, ",
>> + "\t\t\t %s_%s, interface, version",
>>ret->name, ret->name,
>>interface->name,
>>interface->uppercase_name,
>>m->uppercase_name);
>> -
>> - if (ret->interface_name == NULL)
>> - printf("interface");
>> - else
>> - printf("&%s_interface", ret->interface_name);
>> + } else if (ret) {
>> + printf("\tstruct wl_proxy *%s;\n\n"
>> + "\t%s = wl_proxy_marshal_constructor("
>> + "(struct wl_proxy *) %s,\n"
>> + "\t\t\t %s_%s, &%s_interface",
>> + ret->name, ret->name,
>> + interface->name,
>> + interface->uppercase_name,
>> + m->uppercase_name,
>> + ret->interface_name
Re: [RFC wayland] protocol: Add wl_surface.buffer_damage
On Nov 13, 2015 10:43 AM, "Derek Foreman" wrote: > > On 13/11/15 02:03 AM, Pekka Paalanen wrote: > > On Thu, 12 Nov 2015 13:48:10 -0600 > > Derek Foreman wrote: > > > >> On 12/11/15 01:27 PM, Jason Ekstrand wrote: > >>> Thanks for picking this up! Also, thanks for posting this on the bug, > >>> I would have missed it otherwise! > >> > >> Thanks to Pekka for prodding me to do so. :) > >> > >>> On Thu, Nov 12, 2015 at 11:16 AM, Derek Foreman < der...@osg.samsung.com> wrote: > >>>> On 09/11/15 09:06 AM, Pekka Paalanen wrote: > >>>>> On Fri, 6 Nov 2015 12:55:19 -0600 > >>>>> Derek Foreman wrote: > >>>>> > > > > >>>>>> +Mixing wl_surface.buffer_damage and wl_surface.damage requests > >>>>>> +on the same surface will result in undefined behaviour. > >>>>> > >>>>> Why undefined? The compositor will always transform between surface and > >>>>> buffer coordinate systems: surface to buffer for texture updates, and > >>>>> buffer to surface for repaint damage. > >>>>> > >>>>> I suppose you can avoid accumulating two different regions depending on > >>>>> the coordinate space until wl_surface.commit by saying only one > >>>>> coordinate space is guaranteed to work at a time. Is that reason > >>>>> enough, or the only reason? > >>>> > >>>> Just lazy. I don't want to care about it or have to test it. Saying > >>>> not to mix them within a commit is just fine too, I think. > >>>> > >>>> Realistically, I can't imagine anyone wanting to do this in the first > >>>> place, so I didn't see much point in spending any time verifying the two > >>>> requests worked well together. > >>>> > >>>> I suppose it'd be possible to slaughter clients trying to mix them - > >>>> would that be preferably to undefined? > >>> > >>> We could say that it's the union of the two but I kind of like "don't > >>> do this" better. > > > > Don't do this indeed, but should it be a fatal error, or just > > undefined? Or defined as whole-surface damage? :-) > > > > I have hard time making my mind. If it's not a fatal error, I am sure > > someone will do it even if by accident. When someone does it, I'd > > expect the undefined behaviour to be practically damaging the whole > > surface, so you wouldn't easily notice it. > > > > So... for the sake of forcing programs to be more correct, make it a > > fatal error? In which case we need a new error code in wl_surface. > > Interesting problem just occurred to me... I don't think just not > mixing damage/buffer_damage within a commit is good enough. > > What if a client commits faster than the screen refresh rate? Then > we're expected to accumulate the damage inside the compositor, right? > > So a client could use damage exclusively in one commit, damage_buffer > exclusively in another, and the compositor still has to mix the result... > > Is it too heavy handed to allow the first damage/damage_buffer to set > what must be used on the surface for its lifetime? I think it's better to just let the client damage however it wants and highly recommend they pick one and stick with it. It's perfectly well-defined to damage in both and trying to come up with a precise condition for a protocol error is, as you pointed out, rather difficult. The only sane condition I can think of is "don't use both in the same commit" but, as you pointed out, that doesn't actually help the compositor. Compositors that don't want to deal with it can just stomp to full damage as soon as things get complicated. --Jason ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [RFC wayland] Track protocol object versions inside wl_proxy.
On Wed, Nov 25, 2015 at 10:15 AM, Derek Foreman wrote: > On 13/11/15 12:21 PM, Jason Ekstrand wrote: >> On Fri, Nov 13, 2015 at 3:18 AM, Giulio Camuffo >> wrote: >>> So do i understand correctly that if the app creating the >>> wl_compositor was built against a libwayland without this patch the >>> version returned in mesa by wl_proxy_get_version() for every proxy >>> will be always 1? >> >> Yes, see also the second e-mail link below. I proposed to make >> wl_display version 0 so that libraries can detect whether the client >> was built against old or new libwayland. >> >> --Jason > > But the generated protocol headers won't have wl_surface_get_version() > in that case, so build would fail? > > I'm finally trying to build a client that shows this failure mode, and > it's starting to feel quite contrived... The issue that arises is if you have a client built against an old version of libwayland that uses a library built against a newer version. Let's make this extremely concrete: Suppose that we use this wl_surface_get_version() in mesa's EGL implementation to determine whether or not we have a buffer_damage request. We do that, merge it into mesa master, and start shipping it in distros. Then some old client comes along that was built against libwayland 1.3. It can run against libwayland 1.7 just fine because everything's backwards compaible. It can also run against our brand-new mesa because the GL and EGL apis are backwards compatible. However, whatever wayland objects it passes in to EGL will be created using the old api that doesn't have _versioned functions. The result of this is that all objects appear to be version 1 because that's what wl_display starts at. This is ok for buffer_damage() because that's just adding a request. However, if there's something more subtle that happens when an object changes version such that treating a version 3 object as a version 1 object may not always be correct, then we have a problem. While, in general, we should try to avoid these kinds of changes, they can happen so we should let the user of wl_proxy_get_version() be able to distinguish between version 1 and "I don't know". Does that make more sense? It's quite subtle. > Is there a reason we'd want to use wl_proxy_get_version() directly? I > can't imagine having a proxy, needing to know its version, but not > knowing exactly what it's a proxy for... Maybe I'm just not being > imaginative enough though. No, wrappers are fine. > If we make foo_get_version()'s presence known through #defines then the > absence of an appropriate foo_get_version() (ie: old header) could > indicate we need to do whatever kind of fallback. Sure. For that matter, the client can get that from a version #define or from pkg-config. That's a standard "am I building with a recent version" issue. --Jason > Thanks, > Derek > >>> 2015-11-12 22:01 GMT+02:00 Derek Foreman : >>>> From: Jason Ekstrand >>>> >>>> This provides a standardized mechanism for tracking protocol object >>>> versions in client code. The wl_display object is created with version 1. >>>> Every time an object is created from within wl_registry_bind, it gets the >>>> bound version. Every other time an object is created, it simply inherits >>>> it's version from the parent object that created it. >>>> >>>> --- >>>> Sooo, seems to finally fix the EGLSwapBuffersWithDamage problem, we also >>>> need this patch. However, it's not complete. >>>> >>>> I've rebased it and updated it. Here's the original posting: >>>> http://lists.freedesktop.org/archives/wayland-devel/2014-April/014004.html >>>> >>>> Of special importance is: >>>> http://lists.freedesktop.org/archives/wayland-devel/2014-April/014011.html >>>> >>>> And the proposed solution to the new backwards compatibility issue. If we >>>> get a consensus on a way forward, I can pick this up and finish it off... >>>> >>>> src/scanner.c | 29 >>>> src/wayland-client-core.h | 14 ++ >>>> src/wayland-client.c | 112 >>>> +++--- >>>> 3 files changed, 140 insertions(+), 15 deletions(-) >>>> >>>> diff --git a/src/scanner.c b/src/scanner.c >>>> index 8ecdd56..850e72a 100644 >>>> --- a/src/scanner.c >>>> +++ b/src/scanner.c >>>> @@ -889,6 +889,14 @@ emit_stubs(struct wl_list *message_lis
Re: [RFC wayland] Track protocol object versions inside wl_proxy.
On Nov 25, 2015 12:03 PM, "Derek Foreman" wrote: > > On 13/11/15 12:21 PM, Jason Ekstrand wrote: > > On Fri, Nov 13, 2015 at 3:18 AM, Giulio Camuffo wrote: > >> So do i understand correctly that if the app creating the > >> wl_compositor was built against a libwayland without this patch the > >> version returned in mesa by wl_proxy_get_version() for every proxy > >> will be always 1? > > > > Yes, see also the second e-mail link below. I proposed to make > > wl_display version 0 so that libraries can detect whether the client > > was built against old or new libwayland. > > > > --Jason > > We can do a test like... > if (proxy == (struct wl_proxy *)proxy->display) > return 1; > > in wl_proxy_get_version() if we want to return non-0 values from > wl_display_get_version(display); > > Is this worth the trouble, or is it safe to always consider wl_display > to be "unversioned"? I don't think it matters. The wl_display object will only ever report one version. Whether that version is 1 or 0 probably doesn't matter. I'd personally go with 0 but I think you already knew that. ;-) --Jason ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH wayland] client: Use a 0 version to mean indeterminate proxy version
On Nov 26, 2015 1:23 PM, "Derek Foreman" wrote: > > This sets wl_display's version (for proxy version query purposes) > to 0. Any proxy created with unversioned API (this happens when > a client compiled with old headers links against new wayland) > will inherit this 0. > > This gives us a way for new libraries linked by old clients to > realize they can't know a proxy's version. > > wl_display's version being unqueryable (always returning 0) is > an acceptable side effect, since it's a special object you can't > bind specific versions of anyway. > > Signed-off-by: Derek Foreman Reviewed-by: Jason Ekstrand > --- > > This follows on Jason's proxy version patch that I rebased > and reposted a little while ago. > > src/wayland-client.c | 33 +++-- > 1 file changed, 31 insertions(+), 2 deletions(-) > > diff --git a/src/wayland-client.c b/src/wayland-client.c > index f2df0b9..5ba6edc 100644 > --- a/src/wayland-client.c > +++ b/src/wayland-client.c > @@ -928,7 +928,25 @@ wl_display_connect_to_fd(int fd) > display->proxy.queue = &display->default_queue; > display->proxy.flags = 0; > display->proxy.refcount = 1; > - display->proxy.version = 1; > + > + /* We set this version to 0 for backwards compatibility. > +* > +* If a client is using old versions of protocol headers, > +* it will use unversioned API to create proxies. Those > +* proxies will inherit this 0. > +* > +* A client could be passing these proxies into library > +* code newer than the headers that checks proxy > +* versions. When the proxy version is reported as 0 > +* the library will know that it can't reliably determine > +* the proxy version, and should do whatever fallback is > +* required. > +* > +* This trick forces wl_display to always report 0, but > +* since it's a special object that we can't bind > +* specific versions of anyway, this should be fine. > +*/ > + display->proxy.version = 0; > > display->connection = wl_connection_create(display->fd); > if (display->connection == NULL) > @@ -1923,7 +1941,18 @@ wl_proxy_get_user_data(struct wl_proxy *proxy) > /** Get the protocol object version of a proxy object > * > * \param proxy The proxy object > - * \return The protocol object version of the proxy > + * \return The protocol object version of the proxy or 0 > + * > + * Gets the protocol object version of a proxy object, or 0 > + * if the proxy was created with unversioned API. > + * > + * A returned value of 0 means that no version information is > + * available, so the caller must make safe assumptions about > + * the object's real version. > + * > + * wl_display's version will always return 0. > + * > + * \note This should not normally be used by non-generated code. > * > * \memberof wl_proxy > */ > -- > 2.6.2 > ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH wayland v4] protocol: Add wl_surface.damage_buffer
On Nov 26, 2015 1:44 PM, "Derek Foreman" wrote: > > wl_surface.damage uses surface local co-ordinates. > > Buffer scale and buffer transforms came along, and EGL surfaces > have no understanding of them. > > Theoretically, clients pass damage rectangles - in Y-inverted surface > co-ordinates) to EGLSwapBuffersWithDamage, and the EGL implementation > passed them on to wayland. However, for this to work the EGL > implementation must be able to flip those rectangles into the space > the compositor is expecting, but it's unable to do so because it > doesn't know the height of the transformed buffer. > > So, currently, EGLSwapBuffersWithDamage is unusable and EGLSwapBuffers > has to pass (0,0) - (INT32_MAX, INT32_MAX) damage to function. > > wl_surface.damage_buffer allows damage to be registered on a surface > in buffer co-ordinates, avoiding this problem. > > Credit where it's due, these ideas are not entirely my own: > Over a year ago the idea of changing damage co-ordinates to buffer > co-ordinates was suggested (by Jason Ekstrand), and it was at least > partially rejected and abandoned. At the time it was also suggested > (by Pekka Paalanen) that adding a new wl_surface.damage_buffer request > was another option. > > This will eventually resolve: > https://bugs.freedesktop.org/show_bug.cgi?id=78190 > by making the problem irrelevant. > > Signed-off-by: Derek Foreman This looks fine by me Reviewed-by: Jason Ekstrand > --- > > Changes from v3: > replaced the last paragraph with Pekka's version > (with one tiny grammar change) > > protocol/wayland.xml | 49 +++-- > 1 file changed, 47 insertions(+), 2 deletions(-) > > diff --git a/protocol/wayland.xml b/protocol/wayland.xml > index f9e6d76..4e97bb9 100644 > --- a/protocol/wayland.xml > +++ b/protocol/wayland.xml > @@ -176,7 +176,7 @@ > > > > - > + > >A compositor. This object is a singleton global. The >compositor is in charge of combining the contents of multiple > @@ -986,7 +986,7 @@ > > > > - > + > >A surface is a rectangular area that is displayed on the screen. >It has a location, size and pixel contents. > @@ -1109,6 +1109,10 @@ > wl_surface.commit assigns pending damage as the current damage, > and clears pending damage. The server will clear the current > damage as it repaints the surface. > + > + Alternatively, damage can be posted with wl_surface.damage_buffer > + which uses buffer co-ordinates instead of surface co-ordinates, > + and is probably the preferred and intuitive way of doing this. > > > > @@ -1325,6 +1329,47 @@ > > > > + > + > + > + > + This request is used to describe the regions where the pending > + buffer is different from the current surface contents, and where > + the surface therefore needs to be repainted. The compositor > + ignores the parts of the damage that fall outside of the surface. > + > + Damage is double-buffered state, see wl_surface.commit. > + > + The damage rectangle is specified in buffer coordinates. > + > + The initial value for pending damage is empty: no damage. > + wl_surface.damage_buffer adds pending damage: the new pending > + damage is the union of old pending damage and the given rectangle. > + > + wl_surface.commit assigns pending damage as the current damage, > + and clears pending damage. The server will clear the current > + damage as it repaints the surface. > + > + This request differs from wl_surface.damage in only one way - it > + takes damage in buffer co-ordinates instead of surface local > + co-ordinates. While this generally is more intuitive than surface > + co-ordinates, it is especially desirable when using wp_viewport > + or when a drawing library (like EGL) is unaware of buffer scale > + and buffer transform. > + > + Since it is impossible to convert between buffer and surface > + co-ordinates until all the possible parameters affecting the > + surface size and the buffer-surface mapping are known at > + wl_surface.commit time, damage from wl_surface.damage and > + wl_surface.damage_buffer must be accumulated separately in a > + compositor and combined during wl_surface.commit at the earliest. This last paragraph seems a bit out-of-place to me but I don't have a better suggestion at the moment. > + > + > + > + > + > +
Re: [PATCH wayland v4] protocol: Add wl_surface.damage_buffer
On Nov 27, 2015 1:35 AM, "Pekka Paalanen" wrote: > > On Fri, 27 Nov 2015 16:47:19 +0800 > Jonas Ådahl wrote: > > > On Thu, Nov 26, 2015 at 03:44:12PM -0600, Derek Foreman wrote: > > > wl_surface.damage uses surface local co-ordinates. > > > > > > Buffer scale and buffer transforms came along, and EGL surfaces > > > have no understanding of them. > > > > > > Theoretically, clients pass damage rectangles - in Y-inverted surface > > > co-ordinates) to EGLSwapBuffersWithDamage, and the EGL implementation > > > passed them on to wayland. However, for this to work the EGL > > > implementation must be able to flip those rectangles into the space > > > the compositor is expecting, but it's unable to do so because it > > > doesn't know the height of the transformed buffer. > > > > > > So, currently, EGLSwapBuffersWithDamage is unusable and EGLSwapBuffers > > > has to pass (0,0) - (INT32_MAX, INT32_MAX) damage to function. > > > > > > wl_surface.damage_buffer allows damage to be registered on a surface > > > in buffer co-ordinates, avoiding this problem. > > > > > > Credit where it's due, these ideas are not entirely my own: > > > Over a year ago the idea of changing damage co-ordinates to buffer > > > co-ordinates was suggested (by Jason Ekstrand), and it was at least > > > partially rejected and abandoned. At the time it was also suggested > > > (by Pekka Paalanen) that adding a new wl_surface.damage_buffer request > > > was another option. > > > > > > This will eventually resolve: > > > https://bugs.freedesktop.org/show_bug.cgi?id=78190 > > > by making the problem irrelevant. > > > > > > Signed-off-by: Derek Foreman > > > > Reviewed-by: Jonas Ådahl , with a couple of comments > > below. > > Reviewed-by: Pekka Paalanen > > ... > > > > --- > > > > > > Changes from v3: > > > replaced the last paragraph with Pekka's version > > > (with one tiny grammar change) > > > > > > protocol/wayland.xml | 49 +++-- > > > 1 file changed, 47 insertions(+), 2 deletions(-) > > > > > > diff --git a/protocol/wayland.xml b/protocol/wayland.xml > > > index f9e6d76..4e97bb9 100644 > > > --- a/protocol/wayland.xml > > > +++ b/protocol/wayland.xml > > > @@ -176,7 +176,7 @@ > > > > > > > > > > > > - > > > + > > > > > >A compositor. This object is a singleton global. The > > >compositor is in charge of combining the contents of multiple > > > @@ -986,7 +986,7 @@ > > > > > > > > > > > > - > > > + > > > > > >A surface is a rectangular area that is displayed on the screen. > > >It has a location, size and pixel contents. > > > @@ -1109,6 +1109,10 @@ > > > wl_surface.commit assigns pending damage as the current damage, > > > and clears pending damage. The server will clear the current > > > damage as it repaints the surface. > > > + > > > + Alternatively, damage can be posted with wl_surface.damage_buffer > > > + which uses buffer co-ordinates instead of surface co-ordinates, > > > + and is probably the preferred and intuitive way of doing this. > > > > > > > > > > > > @@ -1325,6 +1329,47 @@ > > > > > > > > > > > > + > > > + > > > > Bikeshed mode enabled: missing empty line between "Version N additions" > > and actual additions. > > > > > + > > > + > > > + This request is used to describe the regions where the pending > > > + buffer is different from the current surface contents, and where > > > + the surface therefore needs to be repainted. The compositor > > > + ignores the parts of the damage that fall outside of the surface. > > > + > > > + Damage is double-buffered state, see wl_surface.commit. > > > + > > > + The damage rectangle is specified in buffer coordinates. > > > + > > > + The initial value for pending damage is empty: no damage. > > > + wl_surface.damage_buffer adds pending damage: the new pending > > > + damage is the union of old pending damage and the given rectangle. > > > + >
Re: [RFC wayland] Track protocol object versions inside wl_proxy.
On Dec 7, 2015 7:20 AM, "Pekka Paalanen" wrote: > > On Wed, 25 Nov 2015 10:25:42 -0800 > Jason Ekstrand wrote: > > > On Wed, Nov 25, 2015 at 10:15 AM, Derek Foreman wrote: > > > On 13/11/15 12:21 PM, Jason Ekstrand wrote: > > >> On Fri, Nov 13, 2015 at 3:18 AM, Giulio Camuffo < giuliocamu...@gmail.com> wrote: > > >>> So do i understand correctly that if the app creating the > > >>> wl_compositor was built against a libwayland without this patch the > > >>> version returned in mesa by wl_proxy_get_version() for every proxy > > >>> will be always 1? > > >> > > >> Yes, see also the second e-mail link below. I proposed to make > > >> wl_display version 0 so that libraries can detect whether the client > > >> was built against old or new libwayland. > > >> > > >> --Jason > > > > > > But the generated protocol headers won't have wl_surface_get_version() > > > in that case, so build would fail? > > > > > > I'm finally trying to build a client that shows this failure mode, and > > > it's starting to feel quite contrived... > > > > The issue that arises is if you have a client built against an old > > version of libwayland that uses a library built against a newer > > version. Let's make this extremely concrete: > > > > Suppose that we use this wl_surface_get_version() in mesa's EGL > > implementation to determine whether or not we have a buffer_damage > > request. We do that, merge it into mesa master, and start shipping it > > in distros. Then some old client comes along that was built against > > libwayland 1.3. It can run against libwayland 1.7 just fine because > > everything's backwards compaible. It can also run against our > > brand-new mesa because the GL and EGL apis are backwards compatible. > > However, whatever wayland objects it passes in to EGL will be created > > using the old api that doesn't have _versioned functions. The result > > of this is that all objects appear to be version 1 because that's what > > wl_display starts at. > > > > This is ok for buffer_damage() because that's just adding a request. > > However, if there's something more subtle that happens when an object > > changes version such that treating a version 3 object as a version 1 > > object may not always be correct, then we have a problem. While, in > > general, we should try to avoid these kinds of changes, they can > > happen so we should let the user of wl_proxy_get_version() be able to > > distinguish between version 1 and "I don't know". > > Hi, > > FYI, I've been thinking about this small detail. Binding an object with > version 3, and then using it like you'd use version 1 object is > something we do all the time everywhere without even realizing. I don't > think we can or should ever break that compatibility - nowdays I think > it is part of the protocol stability promise. > > I think this is also part of the reason we started to favour adding a > new request rather than making the semantics of an existing one > version-dependent. > > This is not a comment against the "unversioned version" concept at all, > it is a rule I believe we should follow in protocol design. However, I > bet someone eventually manages to create a mess where changing > behaviour by version becomes the only solution, so "unversioned > version" will probably become useful. But it should be a last resort. I agree. I wasn't claiming that we should actively do that, but that the wl_proxy API should plan for that contingency. Even in the current example, we could take advantage of it. A SwapBuffersWithDamage implementation can do actual damage for wl_surface version 1 but not later versions because they may have rotation. (OK, maybe not because you could crop/scale a version 1 surface, but I think you see what I mean). > Re: versioning wl_display itself, I agree with Jason that it doesn't > matter: wl_display cannot really be extended anyway. We have no way to > negotiate a new version for it - libwayland-client would need to > internally create a wl_registry, fish out a new wl_display global, bind > it with a new version, and then replace the old object with it somehow. > I shudder to think of a situation where this would be necessary... And if they do, then the new wl_display will get created with constructor_versioned and get the correct non-zero version number. However, that would also cause everything that gets created with said wl_display to also have at least version n. Maybe better to just special-case wl_display... --Jason ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: wl_display_connect() falling back to wayland-0
Hi Emilio, On Dec 9, 2013 6:38 AM, "Emilio Pozuelo Monfort" wrote: > > Hi, > > I was looking at making gtk+ try the wayland backend before the x11 one [1]. > This would solve the problem where every gtk+ app uses the x11 backend through > XWayland when the latter is available. > > As you can read on comment #3 and in the patch from comment #1, > wl_display_connect() currently falls back to wayland-0 if WAYLAND_DISPLAY is not > in the environment, which causes the wayland backend initialization to succeed > when running from X11 while weston is running (whether in a different tty or in > an X11 window). > > The workaround I'm using in gtk+ is to directly fail the wayland backend > initialization if WAYLAND_DISPLAY is not set, as Pekka suggested to me. This > works nicely, making gtk+ apps use the x11 backend when launched from within > X11, and use the wayland backend when started within weston. > > So my question is: why are we falling back to wayland-0 when WAYLAND_DISPLAY > isn't set? Is this so that one can easily/quickly run apps from a tty for > debugging purposes? Why do we have a fallback? I don't know. It doesn't seem that hard to just set WAYLAND_DISPLAY if you want to run from another terminal. Why is the fallback wayland-0? I think that's the obvious choice? How do you work around it? What you got from Pekka is half right. You need to check for both WAYLAND_DISPLAY and WAYLAND_SOCKET. The former is the catch-all "connect to this socket; it lives in XDG_RUNTIME_DIR." The other is a mechanism for passing the client a specific file descriptor that it is expected to use for it's connection. This is usually used for launching trusted clients such as the panel. Hope that helps, --Jason Ekstrand > > Cheers, > Emilio > > [1] https://bugzilla.gnome.org/show_bug.cgi?id=719989 > ___ > wayland-devel mailing list > wayland-devel@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/wayland-devel ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: Payload size and text.xml in weston
Elvis, On Dec 12, 2013 2:08 AM, "Elvis Lee(KwangWoong Lee)" wrote: > > Hello. > > It seems that each message has a limitation of payload size, 1024 bytes. I thought it was 4096 bytes (1024 uchar32_t's), but yes, it is there. In particular, this is the maximum size of any particular protocol message that libwayland can transmit. In the case of array or string arguments, the array or string is included in the message size > While using text protocol in Weston, we encountered some cases which exceeds the limitation. > > For example, "set_surrounding_text" carries a string which is longer than 1024 bytes. This is the first time I've heard of someone actually hitting the limit. Not that it hasn't happened before, I just don't remember it. Could you give a more concrete example of exactly what you're doing that hits it? How much surrounding text are you sending and how much is really needed? > I guess and hope that you guys already know how to handle this situation. > > 1. Restricts input size of text box in UX. > > 2. Restricts the string size of “set_surrounding_text”. (Then how can I handle delete_surrounding_text properly?) > > 3. Maybe, I’m totally misunderstood about the payload size. If you're misunderstanding something, it's probably the input_methods protocol. Unfortunately, my knowledge starts to break down here. I don't know too much about input methods but hopefully someone else will. I hope that was helpful. --Jason Ekstrand ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH wayland 1/3] Rename wl_debug to debug_server/client
Signed-off-by: Jason Ekstrand
---
src/wayland-client.c | 10 +-
src/wayland-server.c | 10 +-
2 files changed, 10 insertions(+), 10 deletions(-)
diff --git a/src/wayland-client.c b/src/wayland-client.c
index 363d5dd..799ebab 100644
--- a/src/wayland-client.c
+++ b/src/wayland-client.c
@@ -94,7 +94,7 @@ struct wl_display {
/** \endcond */
-static int wl_debug = 0;
+static int debug_client = 0;
static void
display_fatal_error(struct wl_display *display, int error)
@@ -469,7 +469,7 @@ wl_proxy_marshal_array_constructor(struct wl_proxy *proxy,
abort();
}
- if (wl_debug)
+ if (debug_client)
wl_closure_print(closure, &proxy->object, true);
if (wl_closure_send(closure, proxy->display->connection)) {
@@ -700,7 +700,7 @@ wl_display_connect_to_fd(int fd)
debug = getenv("WAYLAND_DEBUG");
if (debug && (strstr(debug, "client") || strstr(debug, "1")))
- wl_debug = 1;
+ debug_client = 1;
display = malloc(sizeof *display);
if (display == NULL) {
@@ -1038,13 +1038,13 @@ dispatch_event(struct wl_display *display, struct
wl_event_queue *queue)
pthread_mutex_unlock(&display->mutex);
if (proxy->dispatcher) {
- if (wl_debug)
+ if (debug_client)
wl_closure_print(closure, &proxy->object, false);
wl_closure_dispatch(closure, proxy->dispatcher,
&proxy->object, opcode);
} else if (proxy->object.implementation) {
- if (wl_debug)
+ if (debug_client)
wl_closure_print(closure, &proxy->object, false);
wl_closure_invoke(closure, WL_CLOSURE_INVOKE_CLIENT,
diff --git a/src/wayland-server.c b/src/wayland-server.c
index 1459e09..e03316c 100644
--- a/src/wayland-server.c
+++ b/src/wayland-server.c
@@ -117,7 +117,7 @@ struct wl_resource {
wl_dispatcher_func_t dispatcher;
};
-static int wl_debug = 0;
+static int debug_server = 0;
WL_EXPORT void
wl_resource_post_event_array(struct wl_resource *resource, uint32_t opcode,
@@ -137,7 +137,7 @@ wl_resource_post_event_array(struct wl_resource *resource,
uint32_t opcode,
if (wl_closure_send(closure, resource->client->connection))
resource->client->error = 1;
- if (wl_debug)
+ if (debug_server)
wl_closure_print(closure, object, true);
wl_closure_destroy(closure);
@@ -177,7 +177,7 @@ wl_resource_queue_event_array(struct wl_resource *resource,
uint32_t opcode,
if (wl_closure_queue(closure, resource->client->connection))
resource->client->error = 1;
- if (wl_debug)
+ if (debug_server)
wl_closure_print(closure, object, true);
wl_closure_destroy(closure);
@@ -324,7 +324,7 @@ wl_client_connection_data(int fd, uint32_t mask, void *data)
break;
}
- if (wl_debug)
+ if (debug_server)
wl_closure_print(closure, object, false);
if ((resource_flags & WL_MAP_ENTRY_LEGACY) ||
@@ -798,7 +798,7 @@ wl_display_create(void)
debug = getenv("WAYLAND_DEBUG");
if (debug && (strstr(debug, "server") || strstr(debug, "1")))
- wl_debug = 1;
+ debug_server = 1;
display = malloc(sizeof *display);
if (display == NULL)
--
1.8.4.2
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH wayland 3/3] Add a debug handler and use it to print out protocol debug messages
In order to keep from overloading the debug handler, we first squash the
entire message into a string and call wl_debug once.
Signed-off-by: Jason Ekstrand
---
src/connection.c | 54 ---
src/wayland-client.c | 6 ++
src/wayland-client.h | 1 +
src/wayland-private.h | 2 ++
src/wayland-server.c | 6 ++
src/wayland-server.h | 1 +
src/wayland-util.c| 19 ++
7 files changed, 65 insertions(+), 24 deletions(-)
diff --git a/src/connection.c b/src/connection.c
index 1d8b61b..b7f02b4 100644
--- a/src/connection.c
+++ b/src/connection.c
@@ -1088,67 +1088,73 @@ void
wl_closure_print(struct wl_closure *closure, struct wl_object *target, int
send)
{
int i;
+ struct wl_array msg;
struct argument_details arg;
const char *signature = closure->message->signature;
struct timespec tp;
unsigned int time;
+ wl_array_init(&msg);
+ wl_array_add(&msg, 128); /* This should be big enough for most cases */
+ msg.size = 0;
+
clock_gettime(CLOCK_REALTIME, &tp);
time = (tp.tv_sec * 100L) + (tp.tv_nsec / 1000);
- fprintf(stderr, "[%10.3f] %s%s@%u.%s(",
- time / 1000.0,
- send ? " -> " : "",
- target->interface->name, target->id,
- closure->message->name);
+ wl_array_printf(&msg, "[%10.3f] %s%s@%u.%s(",
+ time / 1000.0,
+ send ? " -> " : "",
+ target->interface->name, target->id,
+ closure->message->name);
for (i = 0; i < closure->count; i++) {
signature = get_next_argument(signature, &arg);
if (i > 0)
- fprintf(stderr, ", ");
+ wl_array_printf(&msg, ", ");
switch (arg.type) {
case 'u':
- fprintf(stderr, "%u", closure->args[i].u);
+ wl_array_printf(&msg, "%u", closure->args[i].u);
break;
case 'i':
- fprintf(stderr, "%d", closure->args[i].i);
+ wl_array_printf(&msg, "%d", closure->args[i].i);
break;
case 'f':
- fprintf(stderr, "%f",
- wl_fixed_to_double(closure->args[i].f));
+ wl_array_printf(&msg, "%f",
+ wl_fixed_to_double(closure->args[i].f));
break;
case 's':
- fprintf(stderr, "\"%s\"", closure->args[i].s);
+ wl_array_printf(&msg, "\"%s\"", closure->args[i].s);
break;
case 'o':
if (closure->args[i].o)
- fprintf(stderr, "%s@%u",
- closure->args[i].o->interface->name,
- closure->args[i].o->id);
+ wl_array_printf(&msg, "%s@%u",
+
closure->args[i].o->interface->name,
+ closure->args[i].o->id);
else
- fprintf(stderr, "nil");
+ wl_array_printf(&msg, "nil");
break;
case 'n':
- fprintf(stderr, "new id %s@",
- (closure->message->types[i]) ?
-closure->message->types[i]->name :
- "[unknown]");
+ wl_array_printf(&msg, "new id %s@",
+ (closure->message->types[i]) ?
+closure->message->types[i]->name :
+"[unknown]");
if (closure->args[i].n != 0)
- fprintf(stderr, "%u", closure->args[i].n);
+ wl_array_printf(&msg, "%u", closure->args[i].n);
else
- fprintf(stderr, "nil");
+ wl_array_printf(&msg, "nil");
break;
case
[PATCH wayland 2/3] Add a wl_array_printf function for easy string formatting
Signed-off-by: Jason Ekstrand
---
src/wayland-util.c | 41 +
src/wayland-util.h | 1 +
2 files changed, 42 insertions(+)
diff --git a/src/wayland-util.c b/src/wayland-util.c
index 4fe9c81..c9d6d0e 100644
--- a/src/wayland-util.c
+++ b/src/wayland-util.c
@@ -25,6 +25,7 @@
#include
#include
#include
+#include
#include "wayland-util.h"
#include "wayland-private.h"
@@ -148,6 +149,46 @@ wl_array_copy(struct wl_array *array, struct wl_array
*source)
return 0;
}
+int
+wl_array_printf(struct wl_array *array, const char *fmt, ...)
+{
+ va_list ap;
+ int nchars;
+ size_t old_size;
+
+ while (1) {
+ errno = 0;
+
+ va_start(ap, fmt);
+ nchars = vsnprintf((char *)array->data + array->size,
+ array->alloc - array->size, fmt, ap);
+ va_end(ap);
+
+ /* Depending on whether the library implements the C99
+* printf specification or the ISO printf specification,
+* the return value may be different. C99 printf returns
+* the value of the string that *would* be printed in the
+* case of overflow, ISO printf simply returns -1.
+*
+* In order to differentiate between overflow and an error,
+* we set errno to 0 explicitly.
+*/
+ if (errno != 0)
+ return -1;
+
+ if (nchars < 0) {
+ wl_array_add(array, array->alloc);
+ } else if (nchars >= (ssize_t)(array->alloc - array->size)) {
+ old_size = array->size;
+ wl_array_add(array, nchars + 1);
+ array->size = old_size;
+ } else {
+ array->size += nchars;
+ return nchars;
+ }
+ }
+}
+
union map_entry {
uintptr_t next;
void *data;
diff --git a/src/wayland-util.h b/src/wayland-util.h
index 68d91e2..b8919b9 100644
--- a/src/wayland-util.h
+++ b/src/wayland-util.h
@@ -202,6 +202,7 @@ void wl_array_init(struct wl_array *array);
void wl_array_release(struct wl_array *array);
void *wl_array_add(struct wl_array *array, size_t size);
int wl_array_copy(struct wl_array *array, struct wl_array *source);
+int wl_array_printf(struct wl_array *array, const char *fmt, ...);
typedef int32_t wl_fixed_t;
--
1.8.4.2
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH wayland 0/3] Add a debug handler for printing protocol
This series adds a wl_debug function and associated handler that can be set by the uesr. The first patch is merely a rename to prepare for the third. The second adds a useful helper function used by the third. Jason Ekstrand (3): Rename wl_debug to debug_server/client Add a wl_array_printf function for easy string formatting Add a debug handler and use it to print out protocol debug messages src/connection.c | 54 +- src/wayland-client.c | 16 +- src/wayland-client.h | 1 + src/wayland-private.h | 2 ++ src/wayland-server.c | 16 +- src/wayland-server.h | 1 + src/wayland-util.c| 60 +++ src/wayland-util.h| 1 + 8 files changed, 117 insertions(+), 34 deletions(-) -- 1.8.4.2 ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH wayland 2/3] Add a wl_array_printf function for easy string formatting
Signed-off-by: Jason Ekstrand
---
This version of the patch adds a wl_array_vprintf function if we want it.
Both versions accomplish exactly the same thing as far as the series is
concerned. However, wl_array_vprintf might be nice in the future. Feel
free to use whichever one you prefer.
src/wayland-util.c | 61 ++
src/wayland-util.h | 2 ++
2 files changed, 63 insertions(+)
diff --git a/src/wayland-util.c b/src/wayland-util.c
index 4fe9c81..3e5ac9c 100644
--- a/src/wayland-util.c
+++ b/src/wayland-util.c
@@ -25,6 +25,7 @@
#include
#include
#include
+#include
#include "wayland-util.h"
#include "wayland-private.h"
@@ -148,6 +149,66 @@ wl_array_copy(struct wl_array *array, struct wl_array
*source)
return 0;
}
+int
+wl_array_vprintf(struct wl_array *array, const char *fmt, va_list ap)
+{
+ va_list ap_copy;
+ int nchars;
+ size_t old_size;
+
+ while (1) {
+ errno = 0;
+
+#if defined(va_copy)
+ va_copy(ap_copy, ap);
+#elif defined(__va_copy)
+ __va_copy(ap_copy, ap);
+#else
+ ap_copy = ap;
+#endif
+
+ nchars = vsnprintf((char *)array->data + array->size,
+ array->alloc - array->size, fmt, ap_copy);
+ va_end(ap_copy);
+
+ /* Depending on whether the library implements the C99
+* printf specification or the ISO printf specification,
+* the return value may be different. C99 printf returns
+* the value of the string that *would* be printed in the
+* case of overflow, ISO printf simply returns -1.
+*
+* In order to differentiate between overflow and an error,
+* we set errno to 0 explicitly.
+*/
+ if (errno != 0)
+ return -1;
+
+ if (nchars < 0) {
+ wl_array_add(array, array->alloc);
+ } else if (nchars >= (ssize_t)(array->alloc - array->size)) {
+ old_size = array->size;
+ wl_array_add(array, nchars + 1);
+ array->size = old_size;
+ } else {
+ array->size += nchars;
+ return nchars;
+ }
+ }
+}
+
+int
+wl_array_printf(struct wl_array *array, const char *fmt, ...)
+{
+ va_list ap;
+ int nchars;
+
+ va_start(ap, fmt);
+ nchars = wl_array_vprintf(array, fmt, ap);
+ va_end(ap);
+
+ return nchars;
+}
+
union map_entry {
uintptr_t next;
void *data;
diff --git a/src/wayland-util.h b/src/wayland-util.h
index 68d91e2..e527cfd 100644
--- a/src/wayland-util.h
+++ b/src/wayland-util.h
@@ -202,6 +202,8 @@ void wl_array_init(struct wl_array *array);
void wl_array_release(struct wl_array *array);
void *wl_array_add(struct wl_array *array, size_t size);
int wl_array_copy(struct wl_array *array, struct wl_array *source);
+int wl_array_printf(struct wl_array *array, const char *fmt, ...);
+int wl_array_vprintf(struct wl_array *array, const char *fmt, va_list ap);
typedef int32_t wl_fixed_t;
--
1.8.4.2
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH wayland 0/3] Add a debug handler for printing protocol
Pekka, Thanks for looking at them! On Thu, Dec 19, 2013 at 2:17 AM, Pekka Paalanen wrote: > On Wed, 18 Dec 2013 20:56:17 -0600 > Jason Ekstrand wrote: > > > This series adds a wl_debug function and associated handler that can > > be set by the uesr. The first patch is merely a rename to prepare > > for the third. The second adds a useful helper function used by the > > third. > > > > Jason Ekstrand (3): > > Rename wl_debug to debug_server/client > > Add a wl_array_printf function for easy string formatting > > Add a debug handler and use it to print out protocol debug messages > > Hi, > > this all looks nice. I guess the intention is that all compositors > implement a debug logger, and then WAYLAND_DEBUG=1/server cause it to > get filled. Clients OTOH are usually supposed to not have their own > handler, so that the debug spew gets to stderr as usual. Something like > that? > My reason is fairly simple: I'm embedding libwayland into an Android application and I don't have access to stderr so I needed a way to re-route things. That said, I didn't put too much effort into thinking about the symantics. I think we have three options: 1) make the debug handler only for re-routing and still use the environment variable as before. (This is how it is implemented in my patch) 2) make the wl_debug_set_cleint/server_handler enable debugging. The environment variable enables debugging and installs a default handler if none is found. 3) add an explicit mechanism for enabling debugging possibly even on a per-client basis. The environment variable would then be a legacy fallback. > Some further (old) ideas: > > - For server prints, it would be nice to be able to identify the > connection or the client. > > - For client prints it would be nice to be able to identify the current > connection or process. > > - wl_arrays might be nice to print out in the dump somehow. > > - Strings are not escaped properly, so it is near impossible to write an > automatic dump parser that would get string arguments always right > regardless of their content. > > Though, if this output is never intended to be machine-read, the latter > two don't matter Those are all great ideas. Unfortunately, they're kind of out of the scope of what I'm trying to accomplish. Hopefully someone will work on them some day. Digging through weston logs is a pain because there's always multiple clients embedded in it. Thanks, --Jason Ekstrand ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH 1/2] compositor: Destroy renderer in weston_compositor_shutdown()
Ander,
I don't neciserally disagree we with this change, but shouldn't the GL
renderer clean up after itself better?
--Jason
On Dec 20, 2013 1:07 PM, "Ander Conselvan de Oliveira"
wrote:
> From: Ander Conselvan de Oliveira
>
> Currently we destroy the renderer before the outputs are destroyed, but
> that sometimes leads to an error since a reference to the renderer is
> necessary in order to destroy a gl_renderer_output.
>
> Since destroying the renderer is common among all backends, just move
> that call into weston_compositor_shutdown() immediately after the
> outputs being destroyed.
> ---
> src/compositor-drm.c |2 --
> src/compositor-fbdev.c|2 --
> src/compositor-headless.c |2 --
> src/compositor-rdp.c |1 -
> src/compositor-rpi.c |2 --
> src/compositor-wayland.c |2 --
> src/compositor-x11.c |2 --
> src/compositor.c |3 +++
> 8 files changed, 3 insertions(+), 13 deletions(-)
>
> diff --git a/src/compositor-drm.c b/src/compositor-drm.c
> index f85298a..d637e75 100644
> --- a/src/compositor-drm.c
> +++ b/src/compositor-drm.c
> @@ -2336,8 +2336,6 @@ drm_destroy(struct weston_compositor *ec)
>
> destroy_sprites(d);
>
> - ec->renderer->destroy(ec);
> -
> weston_compositor_shutdown(ec);
>
> if (d->gbm)
> diff --git a/src/compositor-fbdev.c b/src/compositor-fbdev.c
> index e649d43..0d96269 100644
> --- a/src/compositor-fbdev.c
> +++ b/src/compositor-fbdev.c
> @@ -797,8 +797,6 @@ fbdev_compositor_destroy(struct weston_compositor
> *base)
>
> udev_input_destroy(&compositor->input);
>
> - compositor->base.renderer->destroy(&compositor->base);
> -
> /* Destroy the output. */
> weston_compositor_shutdown(&compositor->base);
>
> diff --git a/src/compositor-headless.c b/src/compositor-headless.c
> index 5497455..5a5c1e6 100644
> --- a/src/compositor-headless.c
> +++ b/src/compositor-headless.c
> @@ -141,8 +141,6 @@ headless_destroy(struct weston_compositor *ec)
> {
> struct headless_compositor *c = (struct headless_compositor *) ec;
>
> - ec->renderer->destroy(ec);
> -
> weston_seat_release(&c->fake_seat);
> weston_compositor_shutdown(ec);
>
> diff --git a/src/compositor-rdp.c b/src/compositor-rdp.c
> index 58342b9..942af50 100644
> --- a/src/compositor-rdp.c
> +++ b/src/compositor-rdp.c
> @@ -521,7 +521,6 @@ rdp_restore(struct weston_compositor *ec)
> static void
> rdp_destroy(struct weston_compositor *ec)
> {
> - ec->renderer->destroy(ec);
> weston_compositor_shutdown(ec);
>
> free(ec);
> diff --git a/src/compositor-rpi.c b/src/compositor-rpi.c
> index 1d52a94..399090d 100644
> --- a/src/compositor-rpi.c
> +++ b/src/compositor-rpi.c
> @@ -422,8 +422,6 @@ rpi_compositor_destroy(struct weston_compositor *base)
>
> udev_input_destroy(&compositor->input);
>
> - compositor->base.renderer->destroy(&compositor->base);
> -
> /* destroys outputs, too */
> weston_compositor_shutdown(&compositor->base);
>
> diff --git a/src/compositor-wayland.c b/src/compositor-wayland.c
> index 14ff4af..d2d8942 100644
> --- a/src/compositor-wayland.c
> +++ b/src/compositor-wayland.c
> @@ -1398,8 +1398,6 @@ wayland_destroy(struct weston_compositor *ec)
> {
> struct wayland_compositor *c = (struct wayland_compositor *) ec;
>
> - ec->renderer->destroy(ec);
> -
> weston_compositor_shutdown(ec);
>
> if (c->parent.shm)
> diff --git a/src/compositor-x11.c b/src/compositor-x11.c
> index 2ef1b5d..97fe3b1 100644
> --- a/src/compositor-x11.c
> +++ b/src/compositor-x11.c
> @@ -1415,8 +1415,6 @@ x11_destroy(struct weston_compositor *ec)
> wl_event_source_remove(compositor->xcb_source);
> x11_input_destroy(compositor);
>
> - ec->renderer->destroy(ec);
> -
> weston_compositor_shutdown(ec); /* destroys outputs, too */
>
> XCloseDisplay(compositor->dpy);
> diff --git a/src/compositor.c b/src/compositor.c
> index ff0f3ab..329ee49 100644
> --- a/src/compositor.c
> +++ b/src/compositor.c
> @@ -3734,6 +3734,9 @@ weston_compositor_shutdown(struct weston_compositor
> *ec)
> wl_list_for_each_safe(output, next, &ec->output_list, link)
> output->destroy(output);
>
> + if (ec->renderer)
> + ec->renderer->destroy(ec);
> +
> weston_binding_list_destroy_all(&ec->key_binding_list);
> weston_binding_list_destroy_all(&ec->button_binding_list);
> weston_binding_list_destroy_all(&ec->touch_binding_list);
> --
> 1.7.9.5
>
> ___
> wayland-devel mailing list
> wayland-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/wayland-devel
>
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH 2/2] shell: Handle the desktop shell client destroy signal
Looks like a good idea to me.
--Jason
On Dec 20, 2013 1:07 PM, "Ander Conselvan de Oliveira"
wrote:
> From: Ander Conselvan de Oliveira
>
> Set the internal pointer for the client to NULL. This fixes a
> segmentation fault at shutdown, where the shell would hang up before
> and cause libwayland to call wl_client_destroy(). When the shell was
> destroyed later, another call to wl_client_destroy() would cause the
> crash.
>
> https://bugs.freedesktop.org/show_bug.cgi?id=72550
> ---
> desktop-shell/shell.c | 16
> desktop-shell/shell.h |1 +
> 2 files changed, 17 insertions(+)
>
> diff --git a/desktop-shell/shell.c b/desktop-shell/shell.c
> index 3d586ec..714881b 100644
> --- a/desktop-shell/shell.c
> +++ b/desktop-shell/shell.c
> @@ -5008,6 +5008,17 @@ desktop_shell_sigchld(struct weston_process
> *process, int status)
> }
>
> static void
> +desktop_shell_client_destroy(struct wl_listener *listener, void *data)
> +{
> + struct desktop_shell *shell;
> +
> + shell = container_of(listener, struct desktop_shell,
> +child.client_destroy_listener);
> +
> + shell->child.client = NULL;
> +}
> +
> +static void
> launch_desktop_shell_process(void *data)
> {
> struct desktop_shell *shell = data;
> @@ -5019,6 +5030,11 @@ launch_desktop_shell_process(void *data)
>
> if (!shell->child.client)
> weston_log("not able to start %s\n", shell->client);
> +
> + shell->child.client_destroy_listener.notify =
> + desktop_shell_client_destroy;
> + wl_client_add_destroy_listener(shell->child.client,
> +
> &shell->child.client_destroy_listener);
> }
>
> static void
> diff --git a/desktop-shell/shell.h b/desktop-shell/shell.h
> index d7c34fc..7a8194d 100644
> --- a/desktop-shell/shell.h
> +++ b/desktop-shell/shell.h
> @@ -92,6 +92,7 @@ struct desktop_shell {
> struct weston_process process;
> struct wl_client *client;
> struct wl_resource *desktop_shell;
> + struct wl_listener client_destroy_listener;
>
> unsigned deathcount;
> uint32_t deathstamp;
> --
> 1.7.9.5
>
> ___
> wayland-devel mailing list
> wayland-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/wayland-devel
>
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH wayland] tests/resources-test: Fix wl_resource_post_event call
On Mon, Jan 6, 2014 at 8:34 AM, Mariusz Ceier wrote: > I think both patches (and original code too obviously) are wrong - > WL_DISPLAY_ERROR (opcode 0) event has signature "ous", so Quentin > patch calls event with too many arguments and ignores types, and Marek > patch calls event with too few arguments and incorrect type for the > second argument. > Imo, correct invocation should be either: > wl_resource_post_event(res, WL_DISPLAY_ERROR, res, > WL_DISPLAY_ERROR_INVALID_OBJECT,"This error will be ignored"); > or > wl_resource_post_event(res, WL_DISPLAY_ERROR, NULL, > WL_DISPLAY_ERROR_INVALID_OBJECT,"This error will be ignored"); > am I right ? > Actually... The test is simply wrong. The comment above it says that it expects that event to be ignored since there is no implementation. However, this is backwards. The implementation has nothing to do with events, it is a bunch of request handlers. The wl_resource_post_event should still post an event and send it along the network even if there is no "implementation". There are some places where we do this in weston, in fact. For instance, it used to be that wl_touch and wl_keyboard had no requests (this has since changed) and, at the time, I think we handed in a null implementation. (This has changed in version 3 of wl_seat and they now have "release" requests and handlers.) I'm sorry that I didn't look at this any earlier. Thanks, --Jason Ekstrand ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH] Only update the surface size if a new buffer is attached
This fixes a regression caused by either 918f2dd4 or da75ee1d. In
particular, if a client called commit without attaching a buffer and if the
compositor had already released its reference to the buffer, then a size of
0x0 would be set on the surface. In particular, this affects the wayland
backend because it frequently sends only a frame request in order to cause
a refresh.
Signed-off-by: Jason Ekstrand
---
src/compositor.c | 12 ++--
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/src/compositor.c b/src/compositor.c
index bb1dfa9..8c69f3c 100644
--- a/src/compositor.c
+++ b/src/compositor.c
@@ -2110,10 +2110,10 @@ weston_surface_commit(struct weston_surface *surface)
surface->buffer_viewport = surface->pending.buffer_viewport;
/* wl_surface.attach */
- if (surface->pending.buffer || surface->pending.newly_attached)
+ if (surface->pending.buffer || surface->pending.newly_attached) {
weston_surface_attach(surface, surface->pending.buffer);
-
- weston_surface_set_size_from_buffer(surface);
+ weston_surface_set_size_from_buffer(surface);
+ }
if (surface->configure && surface->pending.newly_attached)
surface->configure(surface,
@@ -2336,12 +2336,12 @@ weston_subsurface_commit_from_cache(struct
weston_subsurface *sub)
surface->buffer_viewport = sub->cached.buffer_viewport;
/* wl_surface.attach */
- if (sub->cached.buffer_ref.buffer || sub->cached.newly_attached)
+ if (sub->cached.buffer_ref.buffer || sub->cached.newly_attached) {
weston_surface_attach(surface, sub->cached.buffer_ref.buffer);
+ weston_surface_set_size_from_buffer(surface);
+ }
weston_buffer_reference(&sub->cached.buffer_ref, NULL);
- weston_surface_set_size_from_buffer(surface);
-
if (surface->configure && sub->cached.newly_attached)
surface->configure(surface, sub->cached.sx, sub->cached.sy);
sub->cached.sx = 0;
--
1.8.4.2
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH 1/2] gl-renderer: Track border damage and only repaint borders on an as-needed basis
Signed-off-by: Jason Ekstrand
---
src/gl-renderer.c | 110 --
1 file changed, 81 insertions(+), 29 deletions(-)
diff --git a/src/gl-renderer.c b/src/gl-renderer.c
index 0e5afbe..c8dfa4b 100644
--- a/src/gl-renderer.c
+++ b/src/gl-renderer.c
@@ -50,18 +50,29 @@ struct gl_shader {
#define BUFFER_DAMAGE_COUNT 2
+enum gl_border_status {
+ BORDER_STATUS_CLEAN = 0,
+ BORDER_TOP_DIRTY = 1 << GL_RENDERER_BORDER_TOP,
+ BORDER_LEFT_DIRTY = 1 << GL_RENDERER_BORDER_LEFT,
+ BORDER_RIGHT_DIRTY = 1 << GL_RENDERER_BORDER_RIGHT,
+ BORDER_BOTTOM_DIRTY = 1 << GL_RENDERER_BORDER_BOTTOM,
+ BORDER_ALL_DIRTY = 0xf,
+ BORDER_SIZE_CHANGED = 0x10
+};
+
struct gl_border_image {
GLuint tex;
int32_t width, height;
int32_t tex_width;
- int dirty;
void *data;
};
struct gl_output_state {
EGLSurface egl_surface;
pixman_region32_t buffer_damage[BUFFER_DAMAGE_COUNT];
+ enum gl_border_status border_damage[BUFFER_DAMAGE_COUNT];
struct gl_border_image borders[4];
+ enum gl_border_status border_status;
};
enum buffer_type {
@@ -597,9 +608,12 @@ repaint_views(struct weston_output *output,
pixman_region32_t *damage)
}
static void
-draw_output_border_texture(struct gl_border_image *img, int32_t x, int32_t y,
+draw_output_border_texture(struct gl_output_state *go,
+ enum gl_renderer_border_side side,
+ int32_t x, int32_t y,
int32_t width, int32_t height)
{
+ struct gl_border_image *img = &go->borders[side];
static GLushort indices [] = { 0, 1, 3, 3, 1, 2 };
if (!img->data) {
@@ -627,7 +641,7 @@ draw_output_border_texture(struct gl_border_image *img,
int32_t x, int32_t y,
glBindTexture(GL_TEXTURE_2D, img->tex);
}
- if (img->dirty) {
+ if (go->border_status & (1 << side)) {
#ifdef GL_EXT_unpack_subimage
glPixelStorei(GL_UNPACK_ROW_LENGTH_EXT, 0);
glPixelStorei(GL_UNPACK_SKIP_PIXELS_EXT, 0);
@@ -636,7 +650,6 @@ draw_output_border_texture(struct gl_border_image *img,
int32_t x, int32_t y,
glTexImage2D(GL_TEXTURE_2D, 0, GL_BGRA_EXT,
img->tex_width, img->height, 0,
GL_BGRA_EXT, GL_UNSIGNED_BYTE, img->data);
- img->dirty = 0;
}
GLfloat texcoord[] = {
@@ -665,7 +678,8 @@ draw_output_border_texture(struct gl_border_image *img,
int32_t x, int32_t y,
}
static void
-draw_output_border(struct weston_output *output)
+draw_output_borders(struct weston_output *output,
+ enum gl_border_status border_status)
{
struct gl_output_state *go = get_output_state(output);
struct gl_renderer *gr = get_renderer(output->compositor);
@@ -674,6 +688,9 @@ draw_output_border(struct weston_output *output)
struct weston_matrix matrix;
int full_width, full_height;
+ if (border_status == BORDER_STATUS_CLEAN)
+ return; /* Clean. Nothing to do. */
+
top = &go->borders[GL_RENDERER_BORDER_TOP];
bottom = &go->borders[GL_RENDERER_BORDER_BOTTOM];
left = &go->borders[GL_RENDERER_BORDER_LEFT];
@@ -696,23 +713,27 @@ draw_output_border(struct weston_output *output)
glUniform1f(shader->alpha_uniform, 1);
glActiveTexture(GL_TEXTURE0);
- draw_output_border_texture(top,
- 0, 0,
- full_width, top->height);
- draw_output_border_texture(left,
- 0, top->height,
- left->width, output->current_mode->height);
- draw_output_border_texture(right,
- full_width - right->width, top->height,
- right->width, output->current_mode->height);
- draw_output_border_texture(bottom,
- 0, full_height - bottom->height,
- full_width, bottom->height);
+ if (border_status & BORDER_TOP_DIRTY)
+ draw_output_border_texture(go, GL_RENDERER_BORDER_TOP,
+ 0, 0,
+ full_width, top->height);
+ if (border_status & BORDER_LEFT_DIRTY)
+ draw_output_border_texture(go, GL_RENDERER_BORDER_LEFT,
+ 0, top->height,
+ left->width,
output->current_mode->height);
+ if (border_status & BORDER_RIGHT_DIRTY)
+ draw_output_border_texture(go, GL_RENDERER_BORDER_RIGHT,
+
[PATCH 2/2] gl-renderer: Use eglSwapBuffersWithDamageEXT when available
Signed-off-by: Jason Ekstrand
---
src/gl-renderer.c | 99 ++-
1 file changed, 98 insertions(+), 1 deletion(-)
diff --git a/src/gl-renderer.c b/src/gl-renderer.c
index c8dfa4b..5654006 100644
--- a/src/gl-renderer.c
+++ b/src/gl-renderer.c
@@ -124,6 +124,8 @@ struct gl_renderer {
PFNEGLCREATEIMAGEKHRPROC create_image;
PFNEGLDESTROYIMAGEKHRPROC destroy_image;
+ PFNEGLSWAPBUFFERSWITHDAMAGEEXTPROC swap_buffers_with_damage;
+
int has_unpack_subimage;
PFNEGLBINDWAYLANDDISPLAYWL bind_display;
@@ -677,6 +679,17 @@ draw_output_border_texture(struct gl_output_state *go,
glDisableVertexAttribArray(0);
}
+static int
+output_has_borders(struct weston_output *output)
+{
+ struct gl_output_state *go = get_output_state(output);
+
+ return go->borders[GL_RENDERER_BORDER_TOP].data ||
+ go->borders[GL_RENDERER_BORDER_RIGHT].data ||
+ go->borders[GL_RENDERER_BORDER_BOTTOM].data ||
+ go->borders[GL_RENDERER_BORDER_LEFT].data;
+}
+
static void
draw_output_borders(struct weston_output *output,
enum gl_border_status border_status)
@@ -732,6 +745,43 @@ draw_output_borders(struct weston_output *output,
}
static void
+output_get_border_damage(struct weston_output *output,
+enum gl_border_status border_status,
+pixman_region32_t *damage)
+{
+ struct gl_output_state *go = get_output_state(output);
+ struct gl_border_image *top, *bottom, *left, *right;
+ int full_width, full_height;
+
+ if (border_status == BORDER_STATUS_CLEAN)
+ return; /* Clean. Nothing to do. */
+
+ top = &go->borders[GL_RENDERER_BORDER_TOP];
+ bottom = &go->borders[GL_RENDERER_BORDER_BOTTOM];
+ left = &go->borders[GL_RENDERER_BORDER_LEFT];
+ right = &go->borders[GL_RENDERER_BORDER_RIGHT];
+
+ full_width = output->current_mode->width + left->width + right->width;
+ full_height = output->current_mode->height + top->height +
bottom->height;
+ if (border_status & BORDER_TOP_DIRTY)
+ pixman_region32_union_rect(damage, damage,
+ 0, 0,
+ full_width, top->height);
+ if (border_status & BORDER_LEFT_DIRTY)
+ pixman_region32_union_rect(damage, damage,
+ 0, top->height,
+ left->width,
output->current_mode->height);
+ if (border_status & BORDER_RIGHT_DIRTY)
+ pixman_region32_union_rect(damage, damage,
+ full_width - right->width,
top->height,
+ right->width,
output->current_mode->height);
+ if (border_status & BORDER_BOTTOM_DIRTY)
+ pixman_region32_union_rect(damage, damage,
+ 0, full_height - bottom->height,
+ full_width, bottom->height);
+}
+
+static void
output_get_damage(struct weston_output *output,
pixman_region32_t *buffer_damage, uint32_t *border_damage)
{
@@ -802,6 +852,9 @@ gl_renderer_repaint_output(struct weston_output *output,
struct gl_renderer *gr = get_renderer(compositor);
EGLBoolean ret;
static int errored;
+ int i, nrects, buffer_height;
+ EGLint *egl_damage, *d;
+ pixman_box32_t *rects;
pixman_region32_t buffer_damage, total_damage;
enum gl_border_status border_damage = BORDER_STATUS_CLEAN;
@@ -847,7 +900,44 @@ gl_renderer_repaint_output(struct weston_output *output,
pixman_region32_copy(&output->previous_damage, output_damage);
wl_signal_emit(&output->frame_signal, output);
- ret = eglSwapBuffers(gr->egl_display, go->egl_surface);
+ if (gr->swap_buffers_with_damage) {
+ pixman_region32_init(&buffer_damage);
+ weston_transformed_region(output->width, output->height,
+ output->transform,
+ output->current_scale,
+ output_damage, &buffer_damage);
+
+ if (output_has_borders(output)) {
+ pixman_region32_translate(&buffer_damage,
+
go->borders[GL_RENDERER_BORDER_LEFT].width,
+
go->borders[GL_RENDERER_BORDER_TOP].height);
+ output_get_border_damage(output, go->border_status,
+&buffer_damage);
+ }
+
+
[PATCH 2/2] gl-renderer: Use eglSwapBuffersWithDamageEXT when available
Signed-off-by: Jason Ekstrand
---
The second version properly sets the EGL_SWAP_BEHAVIOR surface attribute to
EGL_BUFFER_PRESERVED.
src/gl-renderer.c | 104 +-
1 file changed, 103 insertions(+), 1 deletion(-)
diff --git a/src/gl-renderer.c b/src/gl-renderer.c
index c8dfa4b..eddf481 100644
--- a/src/gl-renderer.c
+++ b/src/gl-renderer.c
@@ -124,6 +124,8 @@ struct gl_renderer {
PFNEGLCREATEIMAGEKHRPROC create_image;
PFNEGLDESTROYIMAGEKHRPROC destroy_image;
+ PFNEGLSWAPBUFFERSWITHDAMAGEEXTPROC swap_buffers_with_damage;
+
int has_unpack_subimage;
PFNEGLBINDWAYLANDDISPLAYWL bind_display;
@@ -677,6 +679,17 @@ draw_output_border_texture(struct gl_output_state *go,
glDisableVertexAttribArray(0);
}
+static int
+output_has_borders(struct weston_output *output)
+{
+ struct gl_output_state *go = get_output_state(output);
+
+ return go->borders[GL_RENDERER_BORDER_TOP].data ||
+ go->borders[GL_RENDERER_BORDER_RIGHT].data ||
+ go->borders[GL_RENDERER_BORDER_BOTTOM].data ||
+ go->borders[GL_RENDERER_BORDER_LEFT].data;
+}
+
static void
draw_output_borders(struct weston_output *output,
enum gl_border_status border_status)
@@ -732,6 +745,43 @@ draw_output_borders(struct weston_output *output,
}
static void
+output_get_border_damage(struct weston_output *output,
+enum gl_border_status border_status,
+pixman_region32_t *damage)
+{
+ struct gl_output_state *go = get_output_state(output);
+ struct gl_border_image *top, *bottom, *left, *right;
+ int full_width, full_height;
+
+ if (border_status == BORDER_STATUS_CLEAN)
+ return; /* Clean. Nothing to do. */
+
+ top = &go->borders[GL_RENDERER_BORDER_TOP];
+ bottom = &go->borders[GL_RENDERER_BORDER_BOTTOM];
+ left = &go->borders[GL_RENDERER_BORDER_LEFT];
+ right = &go->borders[GL_RENDERER_BORDER_RIGHT];
+
+ full_width = output->current_mode->width + left->width + right->width;
+ full_height = output->current_mode->height + top->height +
bottom->height;
+ if (border_status & BORDER_TOP_DIRTY)
+ pixman_region32_union_rect(damage, damage,
+ 0, 0,
+ full_width, top->height);
+ if (border_status & BORDER_LEFT_DIRTY)
+ pixman_region32_union_rect(damage, damage,
+ 0, top->height,
+ left->width,
output->current_mode->height);
+ if (border_status & BORDER_RIGHT_DIRTY)
+ pixman_region32_union_rect(damage, damage,
+ full_width - right->width,
top->height,
+ right->width,
output->current_mode->height);
+ if (border_status & BORDER_BOTTOM_DIRTY)
+ pixman_region32_union_rect(damage, damage,
+ 0, full_height - bottom->height,
+ full_width, bottom->height);
+}
+
+static void
output_get_damage(struct weston_output *output,
pixman_region32_t *buffer_damage, uint32_t *border_damage)
{
@@ -802,6 +852,9 @@ gl_renderer_repaint_output(struct weston_output *output,
struct gl_renderer *gr = get_renderer(compositor);
EGLBoolean ret;
static int errored;
+ int i, nrects, buffer_height;
+ EGLint *egl_damage, *d;
+ pixman_box32_t *rects;
pixman_region32_t buffer_damage, total_damage;
enum gl_border_status border_damage = BORDER_STATUS_CLEAN;
@@ -847,7 +900,44 @@ gl_renderer_repaint_output(struct weston_output *output,
pixman_region32_copy(&output->previous_damage, output_damage);
wl_signal_emit(&output->frame_signal, output);
- ret = eglSwapBuffers(gr->egl_display, go->egl_surface);
+ if (gr->swap_buffers_with_damage && gr->has_egl_buffer_age) {
+ pixman_region32_init(&buffer_damage);
+ weston_transformed_region(output->width, output->height,
+ output->transform,
+ output->current_scale,
+ output_damage, &buffer_damage);
+
+ if (output_has_borders(output)) {
+ pixman_region32_translate(&buffer_damage,
+
go->borders[GL_RENDERER_BORDER_LEFT].width,
+
go->borders[GL_RENDERER_BORDER_TOP].height);
+ output_get_border_damag
Re: [PATCH 2/2] gl-renderer: Use eglSwapBuffersWithDamageEXT when available
Never mind, the first one was correct. I misunderstood buffer age.
--Jason
On Jan 16, 2014 10:52 PM, "Jason Ekstrand" wrote:
> Signed-off-by: Jason Ekstrand
> ---
>
> The second version properly sets the EGL_SWAP_BEHAVIOR surface attribute to
> EGL_BUFFER_PRESERVED.
>
> src/gl-renderer.c | 104
> +-
> 1 file changed, 103 insertions(+), 1 deletion(-)
>
> diff --git a/src/gl-renderer.c b/src/gl-renderer.c
> index c8dfa4b..eddf481 100644
> --- a/src/gl-renderer.c
> +++ b/src/gl-renderer.c
> @@ -124,6 +124,8 @@ struct gl_renderer {
> PFNEGLCREATEIMAGEKHRPROC create_image;
> PFNEGLDESTROYIMAGEKHRPROC destroy_image;
>
> + PFNEGLSWAPBUFFERSWITHDAMAGEEXTPROC swap_buffers_with_damage;
> +
> int has_unpack_subimage;
>
> PFNEGLBINDWAYLANDDISPLAYWL bind_display;
> @@ -677,6 +679,17 @@ draw_output_border_texture(struct gl_output_state *go,
> glDisableVertexAttribArray(0);
> }
>
> +static int
> +output_has_borders(struct weston_output *output)
> +{
> + struct gl_output_state *go = get_output_state(output);
> +
> + return go->borders[GL_RENDERER_BORDER_TOP].data ||
> + go->borders[GL_RENDERER_BORDER_RIGHT].data ||
> + go->borders[GL_RENDERER_BORDER_BOTTOM].data ||
> + go->borders[GL_RENDERER_BORDER_LEFT].data;
> +}
> +
> static void
> draw_output_borders(struct weston_output *output,
> enum gl_border_status border_status)
> @@ -732,6 +745,43 @@ draw_output_borders(struct weston_output *output,
> }
>
> static void
> +output_get_border_damage(struct weston_output *output,
> +enum gl_border_status border_status,
> +pixman_region32_t *damage)
> +{
> + struct gl_output_state *go = get_output_state(output);
> + struct gl_border_image *top, *bottom, *left, *right;
> + int full_width, full_height;
> +
> + if (border_status == BORDER_STATUS_CLEAN)
> + return; /* Clean. Nothing to do. */
> +
> + top = &go->borders[GL_RENDERER_BORDER_TOP];
> + bottom = &go->borders[GL_RENDERER_BORDER_BOTTOM];
> + left = &go->borders[GL_RENDERER_BORDER_LEFT];
> + right = &go->borders[GL_RENDERER_BORDER_RIGHT];
> +
> + full_width = output->current_mode->width + left->width +
> right->width;
> + full_height = output->current_mode->height + top->height +
> bottom->height;
> + if (border_status & BORDER_TOP_DIRTY)
> + pixman_region32_union_rect(damage, damage,
> + 0, 0,
> + full_width, top->height);
> + if (border_status & BORDER_LEFT_DIRTY)
> + pixman_region32_union_rect(damage, damage,
> + 0, top->height,
> + left->width,
> output->current_mode->height);
> + if (border_status & BORDER_RIGHT_DIRTY)
> + pixman_region32_union_rect(damage, damage,
> + full_width - right->width,
> top->height,
> + right->width,
> output->current_mode->height);
> + if (border_status & BORDER_BOTTOM_DIRTY)
> + pixman_region32_union_rect(damage, damage,
> + 0, full_height - bottom->height,
> + full_width, bottom->height);
> +}
> +
> +static void
> output_get_damage(struct weston_output *output,
> pixman_region32_t *buffer_damage, uint32_t
> *border_damage)
> {
> @@ -802,6 +852,9 @@ gl_renderer_repaint_output(struct weston_output
> *output,
> struct gl_renderer *gr = get_renderer(compositor);
> EGLBoolean ret;
> static int errored;
> + int i, nrects, buffer_height;
> + EGLint *egl_damage, *d;
> + pixman_box32_t *rects;
> pixman_region32_t buffer_damage, total_damage;
> enum gl_border_status border_damage = BORDER_STATUS_CLEAN;
>
> @@ -847,7 +900,44 @@ gl_renderer_repaint_output(struct weston_output
> *output,
> pixman_region32_copy(&output->previous_damage, output_damage);
> wl_signal_emit(&output->frame_signal, output);
>
> - ret = eglSwapBuffers(gr->egl_display, go->egl_surface);
> + if (gr->swap_buffers_with_damage && gr->has_egl_buffer_age) {
> + pixman_region32_init
Re: weston crash in destroy_shell_surface
Yeah, this is probably better.
--Jason Ekstrand
Reviewed-By: Jason Ekstrand
On Jan 21, 2014 9:02 AM, "Neil Roberts" wrote:
> It looks like the rest of the code assumes that shsurf->children_link
> is always a consistent list. For example, shell_surface_set_parent
> resets children_link to the empty list after it unlinks a child from
> its parent. The destroy_shell_surface code just calls wl_list_remove
> which leaves the list node with invalid NULL pointers. Maybe the
> simplest way to fix it is just to call shell_surface_set_parent in
> that code instead of directly manipulating the list. Here is a patch
> to do that.
>
> Regards,
> - Neil
>
> --- >8 --- (use git am --scissors to automatically chop
> here)
> Subject: shell: Use shell_surface_set_parent to unlink children on destroy
>
> Previously when a shell surface is destroyed it would walk through the
> list of child surfaces and directly remove their children_link from
> the list. This would leave children_link as an invalid list with just
> NULL pointers for prev and next. Other parts of the code such as
> shell_surface_set_parent assume that children_link is always in a
> consistent state (ie, it is the list of sibling surfaces).
> destroy_shell_surface also assumes that it can unconditionally call
> wl_list_remove on the children_link of the surface being destroyed to
> unlink it from its parent if there is one. However, this crashes if
> the child's parent is destroyed first because it would leave the child
> with an invalid children_link.
>
> To fix it this patch makes it call shell_surface_set_parent(..., NULL)
> when unlinking the surface's children instead of directly manipulating
> the list because that function will reset children_link to an empty
> list leaving it in a consistent state.
> ---
> desktop-shell/shell.c | 6 ++
> 1 file changed, 2 insertions(+), 4 deletions(-)
>
> diff --git a/desktop-shell/shell.c b/desktop-shell/shell.c
> index 2f8e610..e3e2c96 100644
> --- a/desktop-shell/shell.c
> +++ b/desktop-shell/shell.c
> @@ -2914,10 +2914,8 @@ destroy_shell_surface(struct shell_surface *shsurf)
> weston_view_destroy(shsurf->view);
>
> wl_list_remove(&shsurf->children_link);
> - wl_list_for_each_safe(child, next, &shsurf->children_list,
> children_link) {
> - wl_list_remove(&child->children_link);
> - child->parent = NULL;
> - }
> + wl_list_for_each_safe(child, next, &shsurf->children_list,
> children_link)
> + shell_surface_set_parent(child, NULL);
>
> wl_list_remove(&shsurf->link);
> free(shsurf);
> --
> 1.8.4.2
>
> ___
> wayland-devel mailing list
> wayland-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/wayland-devel
>
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: What use do swap interval > 1 and OML_sync_control divisor and remainder have?
I'll just chip in one quick thought. On Jan 24, 2014 6:32 AM, "Pekka Paalanen" wrote: > > Hi, > > I am investigating what kind of Wayland protocol extensions would be > needed to support proper presentation timing. Looking at existing > works, I am wondering about two things whether they have any real use. > > Where is swap interval (e.g. eglSwapInterval) greater than one useful? > Intervals 0 and 1 I understand, and Mesa EGL Wayland already supports > those. But when would you like to make your framerate a fraction of the > display's? > > When are the target-MSC related remainder and divisor parameters as > defined in the GLX_OML_sync_control useful? Why does also X11 Present > protocol include remainder and divisor? > > GLX_OML_sync_control defines that for interlaced displays MSC is > incremented for each field. With divisor and remainder you could then > target only top or bottom fields. Is that useful, and do we care about > interlaced displays anymore? I think we do. In particular, we should care about TV set-top boxes. Even though most TVs are LCD, DLP, or similar, hdmi does support interlacing and it is still used (particularly in HDTV). I have no idea what implications this has for a present extension, but I think we could still handle it in a sane way without going for MSC. > I am contemplating on not supporting these, because I am going to > propose using an UST-like clock as the "standard clock language" in > Wayland present extension. Supporting MSC-based timings would add > complexity. Therefore I would like to know where and how the above > mentioned are useful, because I cannot imagine it myself. > > Please, let me know of real actual use cases and existing software, > where these features offer a visible benefit and what that benefit is > exactly. I am not interested in what one might do in theory, I am > interested in real-world examples where they have proved useful. Well, > maybe also theories if they allow taking advantage of some new cool > technology. > > Btw. if you think that using UST for presentation timing and feedback > is nonsense, and MSC is the only right way, let me know and I can start > another email thread about that detail after preparing my material. > > > Thanks, > pq I hope that's helpful, --Jason Ekstrand ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: Make safe double remove?
Jonathan,
I have thought on multiple occations about doing this exact thing. If we
do, we should probably also make wl_list_insert_list safe.
The counter-argument for all this is that you have to keep track of whether
or not your elements are in a list anyway to avoid corruption. For
instance, you still have to watch for double wl_list_insert which will also
break things badly. You could say, "We'll modify wl_list_insert so it's
safe too" but then you inserting an element that is already in a list
becomes "safe" from a segfault perspective and you make it easy to have
program logic errors that don't result in a crash, just things being subtly
wrong and impossible to track down.
All said, I think I'd be in favor of making double-remove safe. However,
it's not a silver bullet.
--Jason Ekstrand
On Jan 28, 2014 11:22 AM, "Jonathan Howard" wrote:
> Code is already using wl_list_init to have the construction of unlinked
> items in a stable state so that they can be destroyed with wl_list_remove.
>
> A problem is it is easy to write code that calls wl_list_remove during the
> structures life and miss that you need to reset with wl_list_init for when
> it is later destroyed.
>
> Having wl_list_remove place items in this unlinked state makes the
> destruction safe (and makes quite a few lines of code redundant.)
>
> --- wayland-util.c 2014-01-28 16:09:13.179052955 +
> +++ wayland-util.c.init 2014-01-28 16:13:32.225704904 +
> @@ -52,8 +52,8 @@
> {
> elm->prev->next = elm->next;
> elm->next->prev = elm->prev;
> - elm->next = NULL;
> - elm->prev = NULL;
> + elm->next = elm;
> + elm->prev = elm;
> }
>
> WL_EXPORT int
>
> ___
> wayland-devel mailing list
> wayland-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/wayland-devel
>
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: Mapping surfaces created through a nested compositor to UI elements
Yeah, Pekka pretty much covered it. I've just got one more observation to add. Are you doing one process per tab? Or do you have one process handling multiple tabs? If you have one process per tab, then you can easily differentiate by which wl_client the surface is attached to. You can know which client is which because (I assume) you should be launching them privately and manually creating the wl_client objects. Thanks, --Jason Ekstrand On Jan 30, 2014 5:35 AM, "Pekka Paalanen" wrote: > On Thu, 30 Jan 2014 10:32:03 +0100 > Iago Toral wrote: > > > Hi, > > > > in the process of porting webkitgtk+ to wayland and following advise > > provided here, I implemented a nested compositor to share surfaces > > between the two processes that do the rendering. This works fine with > > a single widget/surface, but things get a bit more complicated when > > dealing with various widgets (browser tabs, windows). > > > > I am trying to understand if I can solve my problem, which is > > basically a problem of matching wayland surfaces created in the > > nested compositor with their corresponding widgets in the UI, within > > the scope of the wayland protocol or if I need to resort to ad-hoc > > communications between the two processes outside the protocol. Below > > I describe the problem in more detail together with the possible > > solutions I looked into and my conclusions. I'd appreciate a lot if > > someone could confirm whether these are correct: > > > > As far as I understand the code in the nested client example from > > Weston, when there is need to repaint the UI it goes through all the > > surfaces in the compositor and paints them one by one. In our case, > > when GTK needs to repaint it will go through the widgets in the UI > > and ask them to repaint as needed. This means that we need to know, > > for a given widget, which is the surface in the nested compositor > > that provides the contents for it. > > > > However, when the nested compositor receives a request to create a > > surface it will not know for which widget it is creating it (obviously > > information on things like UI widgets is outside the scope of the > > wayland protocol), and as far as I can see, there is no way for the > > client to provide this info to the compositor either when the surface > > is created or after it has been created. > > > > Assuming that this is not something I can so using available APIs, I > > looked into adding this API to my nested compositor implementation, > > so I can have a surface constructor like this: > > > > wl_compositor_create_surface_for_widget(struct wl_compositor*, int); > > > > where that additional 'int' parameter would be used on the > > compositor's side to associate the new surface with a specific UI > > widget. > > > > Unfortunately, for this I would really want to reuse existing code and > > APIs from wayland to do message communication between client and > > compositor, but a good part of this is private to wayland (the > > wl_closure stuff for example) so it feels like I would end up having > > to duplicate quite some code from Wayland in WebKit, which I think is > > really not a good idea. Also, the fact that these APIs have been kept > > internal to Wayland means that this is not something that developers > > are expected to do. > > > > If the above is not a good solution either, I understand there would > > be no solution for my problem within the wayland protocol and I would > > need to add additional messages between the two processes outside the > > protocol after the surface is created in order to associate the > > surface and the widget on the compositor's side. In that case, I > > would need to communicate the widget ID and the ID of the Wayland > > object representing the surface (which I understand I'd get by > > calling wl_proxy_get_id on the client for the surface). > > > > Is my analysis of the problem correct or is there some way in which I > > can achieve my objective within the wayland protocol? > > Hi, > > the short answer to your solution is "write a custom shell extension". > > That means that you would be writing your own private Wayland protocol > extension in the same XML format as all Wayland protocol is already > defined. Let's take xdg_shell as an example: > http://cgit.freedesktop.org/wayland/weston/tree/protocol/xdg-shell.xml > > A compositor advertises a global object of type xdg_shell. The > xdg_shell interface has two requests that allow you to add meaning to > wl_surfaces: get_xdg_
[PATCH weston 2/2 v3] gl-renderer: Use eglSwapBuffersWithDamageEXT when available
Signed-off-by: Jason Ekstrand
---
Version 3 properly #ifdefs around things so it will still build on systems
that do not provide the extension in eglext.h
src/gl-renderer.c | 107 ++
1 file changed, 107 insertions(+)
diff --git a/src/gl-renderer.c b/src/gl-renderer.c
index c8dfa4b..d03bce6 100644
--- a/src/gl-renderer.c
+++ b/src/gl-renderer.c
@@ -124,6 +124,10 @@ struct gl_renderer {
PFNEGLCREATEIMAGEKHRPROC create_image;
PFNEGLDESTROYIMAGEKHRPROC destroy_image;
+#ifdef EGL_EXT_swap_buffers_with_damage
+ PFNEGLSWAPBUFFERSWITHDAMAGEEXTPROC swap_buffers_with_damage;
+#endif
+
int has_unpack_subimage;
PFNEGLBINDWAYLANDDISPLAYWL bind_display;
@@ -677,6 +681,17 @@ draw_output_border_texture(struct gl_output_state *go,
glDisableVertexAttribArray(0);
}
+static int
+output_has_borders(struct weston_output *output)
+{
+ struct gl_output_state *go = get_output_state(output);
+
+ return go->borders[GL_RENDERER_BORDER_TOP].data ||
+ go->borders[GL_RENDERER_BORDER_RIGHT].data ||
+ go->borders[GL_RENDERER_BORDER_BOTTOM].data ||
+ go->borders[GL_RENDERER_BORDER_LEFT].data;
+}
+
static void
draw_output_borders(struct weston_output *output,
enum gl_border_status border_status)
@@ -732,6 +747,43 @@ draw_output_borders(struct weston_output *output,
}
static void
+output_get_border_damage(struct weston_output *output,
+enum gl_border_status border_status,
+pixman_region32_t *damage)
+{
+ struct gl_output_state *go = get_output_state(output);
+ struct gl_border_image *top, *bottom, *left, *right;
+ int full_width, full_height;
+
+ if (border_status == BORDER_STATUS_CLEAN)
+ return; /* Clean. Nothing to do. */
+
+ top = &go->borders[GL_RENDERER_BORDER_TOP];
+ bottom = &go->borders[GL_RENDERER_BORDER_BOTTOM];
+ left = &go->borders[GL_RENDERER_BORDER_LEFT];
+ right = &go->borders[GL_RENDERER_BORDER_RIGHT];
+
+ full_width = output->current_mode->width + left->width + right->width;
+ full_height = output->current_mode->height + top->height +
bottom->height;
+ if (border_status & BORDER_TOP_DIRTY)
+ pixman_region32_union_rect(damage, damage,
+ 0, 0,
+ full_width, top->height);
+ if (border_status & BORDER_LEFT_DIRTY)
+ pixman_region32_union_rect(damage, damage,
+ 0, top->height,
+ left->width,
output->current_mode->height);
+ if (border_status & BORDER_RIGHT_DIRTY)
+ pixman_region32_union_rect(damage, damage,
+ full_width - right->width,
top->height,
+ right->width,
output->current_mode->height);
+ if (border_status & BORDER_BOTTOM_DIRTY)
+ pixman_region32_union_rect(damage, damage,
+ 0, full_height - bottom->height,
+ full_width, bottom->height);
+}
+
+static void
output_get_damage(struct weston_output *output,
pixman_region32_t *buffer_damage, uint32_t *border_damage)
{
@@ -802,6 +854,11 @@ gl_renderer_repaint_output(struct weston_output *output,
struct gl_renderer *gr = get_renderer(compositor);
EGLBoolean ret;
static int errored;
+#ifdef EGL_EXT_swap_buffers_with_damage
+ int i, nrects, buffer_height;
+ EGLint *egl_damage, *d;
+ pixman_box32_t *rects;
+#endif
pixman_region32_t buffer_damage, total_damage;
enum gl_border_status border_damage = BORDER_STATUS_CLEAN;
@@ -847,7 +904,48 @@ gl_renderer_repaint_output(struct weston_output *output,
pixman_region32_copy(&output->previous_damage, output_damage);
wl_signal_emit(&output->frame_signal, output);
+#ifdef EGL_EXT_swap_buffers_with_damage
+ if (gr->swap_buffers_with_damage) {
+ pixman_region32_init(&buffer_damage);
+ weston_transformed_region(output->width, output->height,
+ output->transform,
+ output->current_scale,
+ output_damage, &buffer_damage);
+
+ if (output_has_borders(output)) {
+ pixman_region32_translate(&buffer_damage,
+
go->borders[GL_RENDERER_BORDER_LEFT].width,
+
go->borders[GL_RENDER
[PATCH weston 1/2 v3] gl-renderer: Track border damage and only repaint borders on an as-needed basis
Signed-off-by: Jason Ekstrand
---
No change from previous version. Just resending to keep things together.
src/gl-renderer.c | 110 --
1 file changed, 81 insertions(+), 29 deletions(-)
diff --git a/src/gl-renderer.c b/src/gl-renderer.c
index 0e5afbe..c8dfa4b 100644
--- a/src/gl-renderer.c
+++ b/src/gl-renderer.c
@@ -50,18 +50,29 @@ struct gl_shader {
#define BUFFER_DAMAGE_COUNT 2
+enum gl_border_status {
+ BORDER_STATUS_CLEAN = 0,
+ BORDER_TOP_DIRTY = 1 << GL_RENDERER_BORDER_TOP,
+ BORDER_LEFT_DIRTY = 1 << GL_RENDERER_BORDER_LEFT,
+ BORDER_RIGHT_DIRTY = 1 << GL_RENDERER_BORDER_RIGHT,
+ BORDER_BOTTOM_DIRTY = 1 << GL_RENDERER_BORDER_BOTTOM,
+ BORDER_ALL_DIRTY = 0xf,
+ BORDER_SIZE_CHANGED = 0x10
+};
+
struct gl_border_image {
GLuint tex;
int32_t width, height;
int32_t tex_width;
- int dirty;
void *data;
};
struct gl_output_state {
EGLSurface egl_surface;
pixman_region32_t buffer_damage[BUFFER_DAMAGE_COUNT];
+ enum gl_border_status border_damage[BUFFER_DAMAGE_COUNT];
struct gl_border_image borders[4];
+ enum gl_border_status border_status;
};
enum buffer_type {
@@ -597,9 +608,12 @@ repaint_views(struct weston_output *output,
pixman_region32_t *damage)
}
static void
-draw_output_border_texture(struct gl_border_image *img, int32_t x, int32_t y,
+draw_output_border_texture(struct gl_output_state *go,
+ enum gl_renderer_border_side side,
+ int32_t x, int32_t y,
int32_t width, int32_t height)
{
+ struct gl_border_image *img = &go->borders[side];
static GLushort indices [] = { 0, 1, 3, 3, 1, 2 };
if (!img->data) {
@@ -627,7 +641,7 @@ draw_output_border_texture(struct gl_border_image *img,
int32_t x, int32_t y,
glBindTexture(GL_TEXTURE_2D, img->tex);
}
- if (img->dirty) {
+ if (go->border_status & (1 << side)) {
#ifdef GL_EXT_unpack_subimage
glPixelStorei(GL_UNPACK_ROW_LENGTH_EXT, 0);
glPixelStorei(GL_UNPACK_SKIP_PIXELS_EXT, 0);
@@ -636,7 +650,6 @@ draw_output_border_texture(struct gl_border_image *img,
int32_t x, int32_t y,
glTexImage2D(GL_TEXTURE_2D, 0, GL_BGRA_EXT,
img->tex_width, img->height, 0,
GL_BGRA_EXT, GL_UNSIGNED_BYTE, img->data);
- img->dirty = 0;
}
GLfloat texcoord[] = {
@@ -665,7 +678,8 @@ draw_output_border_texture(struct gl_border_image *img,
int32_t x, int32_t y,
}
static void
-draw_output_border(struct weston_output *output)
+draw_output_borders(struct weston_output *output,
+ enum gl_border_status border_status)
{
struct gl_output_state *go = get_output_state(output);
struct gl_renderer *gr = get_renderer(output->compositor);
@@ -674,6 +688,9 @@ draw_output_border(struct weston_output *output)
struct weston_matrix matrix;
int full_width, full_height;
+ if (border_status == BORDER_STATUS_CLEAN)
+ return; /* Clean. Nothing to do. */
+
top = &go->borders[GL_RENDERER_BORDER_TOP];
bottom = &go->borders[GL_RENDERER_BORDER_BOTTOM];
left = &go->borders[GL_RENDERER_BORDER_LEFT];
@@ -696,23 +713,27 @@ draw_output_border(struct weston_output *output)
glUniform1f(shader->alpha_uniform, 1);
glActiveTexture(GL_TEXTURE0);
- draw_output_border_texture(top,
- 0, 0,
- full_width, top->height);
- draw_output_border_texture(left,
- 0, top->height,
- left->width, output->current_mode->height);
- draw_output_border_texture(right,
- full_width - right->width, top->height,
- right->width, output->current_mode->height);
- draw_output_border_texture(bottom,
- 0, full_height - bottom->height,
- full_width, bottom->height);
+ if (border_status & BORDER_TOP_DIRTY)
+ draw_output_border_texture(go, GL_RENDERER_BORDER_TOP,
+ 0, 0,
+ full_width, top->height);
+ if (border_status & BORDER_LEFT_DIRTY)
+ draw_output_border_texture(go, GL_RENDERER_BORDER_LEFT,
+ 0, top->height,
+ left->width,
output->current_mode->height);
+ if (border_status & BORDER_RIGHT_DIRTY)
[PATCH] nested-client: Fix build error
--- Makefile.am | 1 + 1 file changed, 1 insertion(+) diff --git a/Makefile.am b/Makefile.am index 21b2b17..5e8556b 100644 --- a/Makefile.am +++ b/Makefile.am @@ -485,6 +485,7 @@ weston_nested_CFLAGS = $(CLIENT_CFLAGS) weston_nested_client_SOURCES = clients/nested-client.c weston_nested_client_LDADD = $(SIMPLE_EGL_CLIENT_LIBS) -lm +weston_nested_client_CFLAGS = $(CLIENT_CFLAGS) endif weston_eventdemo_SOURCES = clients/eventdemo.c -- 1.8.5.3 ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: wl_surface_commit from different thread
Hi Prabhu, Could you be a little more specific as to what you are doing. It sounds like you are either writing a client or trying to write the client-side wayland bits for a driver stack. However, it's kind of hard from you description to tell exactly what you're working on. Nevertheless, I will try to answer your question as best I can. While Weston itself is single-threaded, the libwayland client library can handle multiple threads rather well. Look into wl_event_queue which allows you to manage what events get called on what thread. Also, any request can be called from any thread. The only issue is in the synchronization that your app may need to do (know when wl_surface.commit occurs relative to wl_shell_surface.set_maximized for instance). If you want to get a frame completion event in another thread, simply call wl_surface.frame, wl_surface.commit and then add the callback you got from wl_surface.frame to the wl_event_queue for that thread. Also, I'm confused by the relationship between your eglSwapBuffers calls and your wl_surface.commit calls. eglSwapBuffers should be calling wl_surface.commit before it returns. Sometimes this means the underlying graphics drivers pass sync fences or do other things to keep from doing a full glFinish and stalling the GPU. Therefore, in most cases, you shouldn't need to be calling wl_surface.commit manually. Perhaps I'm just misunderstanding something? I hope that helps, --Jason Ekstrand On Fri, Feb 7, 2014 at 8:03 AM, Prabhu S wrote: > Hello, > eglSwapBuffers is a non blocking call. Normally, 3D GPU will be used both > for content creation and composition, in that case everything will end up > the GPU unit. So the end frame will be will be synchronized. (assuming 1 > GPU unit). > > I have a scenario, where 3D GPU being used for creating content and blit > engine used for Weston composition. I'm getting frame completion from 3D > engine in different thread (not in the same thread eglSwapBuffers calling > thread). Currently eglSwapBuffers calling thread need to wait for the frame > completion and then call wl_surface_commit to notify Wayland compositor. > This takes out most of the performance since GPUs are not active all the > time. > > wl_surface_commit cannot be called from different thread AFAIK. Some > advise, suggestions would be appreciated. > > Thanks > Prabhu > > ___ > wayland-devel mailing list > wayland-devel@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/wayland-devel > > ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: wl_surface_commit from different thread
On Fri, Feb 7, 2014 at 12:57 PM, Prabhu S wrote: > Please find my comments inline. > > > > > On Fri, Feb 7, 2014 at 8:18 AM, Jason Ekstrand wrote: > >> Hi Prabhu, >> Could you be a little more specific as to what you are doing. It sounds >> like you are either writing a client or trying to write the client-side >> wayland bits for a driver stack. However, it's kind of hard from you >> description to tell exactly what you're working on. Nevertheless, I will >> try to answer your question as best I can. >> > => Adding support to EGL for wayland > >> >> While Weston itself is single-threaded, the libwayland client library can >> handle multiple threads rather well. Look into wl_event_queue which allows >> you to manage what events get called on what thread. Also, any request can >> be called from any thread. The only issue is in the synchronization that >> your app may need to do (know when wl_surface.commit occurs relative to >> wl_shell_surface.set_maximized for instance). If you want to get a frame >> completion event in another thread, simply call wl_surface.frame, >> wl_surface.commit and then add the callback you got from wl_surface.frame >> to the wl_event_queue for that thread. >> > >> Also, I'm confused by the relationship between your eglSwapBuffers calls >> and your wl_surface.commit calls. eglSwapBuffers should be calling >> wl_surface.commit before it returns. Sometimes this means the underlying >> graphics drivers pass sync fences or do other things to keep from doing a >> full glFinish and stalling the GPU. Therefore, in most cases, you shouldn't >> need to be calling wl_surface.commit manually. Perhaps I'm just >> misunderstanding something? >> > > => I'm taking reference of dri2_swap_buffers_with_damage from > http://cgit.freedesktop.org/mesa/mesa/tree/src/egl/drivers/dri2/platform_wayland.c. > Assume 3D frame is complex and really takes more time for the 3D GPU to > complete the frame, GPU will be still processing the frame after return > from eglSwapBuffers. I will get notification of the frame completion in > different thread. In my assumption calls following wl_surface.attach, > wl_surface.damage, wl_surface.commit, wl_display.flush need to be called > from in different thread. Can this possible by creating callback and > setevent from different thread(frame completion thread). I assumg > wl_surface.frame is the compositor frame completion event?? > Ok, I think I am beginning to see what you mean. First, there are a few guarantees that your EGL implementation needs to provide to Wayland clients/servers: 1) eglSwapBuffers (or eglSwapBuffersWithDamageEXT) must call wl_display.attach, wl_display.damage, and wl_display.commit before it returns. This is so that the client can synchronize various bits of surface state with the wl_display.commit request. It doesn't matter whether or not it happens in another thread, just that it happens by the time eglSwapBuffers reutnrs. 2) As soon as a compositor gets a wl_surface.attach, it is free to pass that buffer resource into eglCreateImageKHR with type EGL_WAYLAND_BUFFER_WL, convert it to a texture, and start drawing with it immediately. Exactly how these tow constraints are met is up to your EGL implementation. On some particular graphics stacks, glFinish is called inside of eglSwapBuffers to ensure that the graphics card has finished with the buffer before passing it to the compositor. As you have already mentioned, this causes the GPU to stall and decreases performance. The way to solve this is to have some sort of a fence associated with each buffer. Instead of eglSwapBuffers waiting for the buffer to be finished before calling wl_surface.attach/damage/commit, it sets up a sync fence, calls wl_surface.attach/damage/commit, and returns immediately. Then it uses that fence to serialize rendering commands somewhere further down the stream. This can mean that eglCreateImageKHR blocks waiting for the buffer to be finished or that you pass the fence further down the line and block in glBindTexture or even just use it to serialize commands to the GPU. How that fence is created, passed around, and waited upon is an internal detail of your EGL implementation and is outside the scope of the core wayland protocol. A final note concerning the wl_surface.frame event. This event has nothing to do with your buffer or with something happening on the GPU as such. It exists for the sole purpose of telling the client that the compositor has started to render using the currently attached buffer and that it can go ahead and start rendering for it's next frame. The only use your EGL implementation should have for wl_surface.frame is to implement eglSwapInterval(1) or simil
Re: [PATCH wayland 3/3] Add a debug handler and use it to print out protocol debug messages
On Wed, Feb 5, 2014 at 11:04 PM, Kristian Høgsberg wrote:
> On Wed, Dec 18, 2013 at 08:56:20PM -0600, Jason Ekstrand wrote:
> > In order to keep from overloading the debug handler, we first squash the
> > entire message into a string and call wl_debug once.
> >
> > Signed-off-by: Jason Ekstrand
> > ---
> > src/connection.c | 54
> ---
> > src/wayland-client.c | 6 ++
> > src/wayland-client.h | 1 +
> > src/wayland-private.h | 2 ++
> > src/wayland-server.c | 6 ++
> > src/wayland-server.h | 1 +
> > src/wayland-util.c| 19 ++
> > 7 files changed, 65 insertions(+), 24 deletions(-)
> >
> > diff --git a/src/connection.c b/src/connection.c
> > index 1d8b61b..b7f02b4 100644
> > --- a/src/connection.c
> > +++ b/src/connection.c
> > @@ -1088,67 +1088,73 @@ void
> > wl_closure_print(struct wl_closure *closure, struct wl_object *target,
> int send)
> > {
> > int i;
> > + struct wl_array msg;
> > struct argument_details arg;
> > const char *signature = closure->message->signature;
> > struct timespec tp;
> > unsigned int time;
> >
> > + wl_array_init(&msg);
> > + wl_array_add(&msg, 128); /* This should be big enough for most
> cases */
> > + msg.size = 0;
> > +
> > clock_gettime(CLOCK_REALTIME, &tp);
> > time = (tp.tv_sec * 100L) + (tp.tv_nsec / 1000);
> >
> > - fprintf(stderr, "[%10.3f] %s%s@%u.%s(",
> > - time / 1000.0,
> > - send ? " -> " : "",
> > - target->interface->name, target->id,
> > - closure->message->name);
> > + wl_array_printf(&msg, "[%10.3f] %s%s@%u.%s(",
> > + time / 1000.0,
> > + send ? " -> " : "",
> > + target->interface->name, target->id,
> > + closure->message->name);
>
> Could we just call wl_debug() instead and avoid building up a string?
> The way you've written this, wl_debug() is always called with one complete
> line of debug text, but do you actually need that?
>
Technically, no. Obviously the debug handler can use something similar to
wl_array_printf to let debug messages build up and then split it into
lines. In fact, this is exactly what I'll do in my code if you prefer (the
debug handler I'm forwarding stuff to needs complete liens). This just
makes it a little easier on the debug handler, especially if stuff is
multi-threaded.
--Jason
>
> > for (i = 0; i < closure->count; i++) {
> > signature = get_next_argument(signature, &arg);
> > if (i > 0)
> > - fprintf(stderr, ", ");
> > + wl_array_printf(&msg, ", ");
> >
> > switch (arg.type) {
> > case 'u':
> > - fprintf(stderr, "%u", closure->args[i].u);
> > + wl_array_printf(&msg, "%u", closure->args[i].u);
> > break;
> > case 'i':
> > - fprintf(stderr, "%d", closure->args[i].i);
> > + wl_array_printf(&msg, "%d", closure->args[i].i);
> > break;
> > case 'f':
> > - fprintf(stderr, "%f",
> > - wl_fixed_to_double(closure->args[i].f));
> > + wl_array_printf(&msg, "%f",
> > +
> wl_fixed_to_double(closure->args[i].f));
> > break;
> > case 's':
> > - fprintf(stderr, "\"%s\"", closure->args[i].s);
> > + wl_array_printf(&msg, "\"%s\"",
> closure->args[i].s);
> > break;
> > case 'o':
> > if (closure->args[i].o)
> > - fprintf(stderr, "%s@%u",
> > -
> closure->args[i].o->interface->name,
> > - closure->args[i].o->id);
> > + wl_array_printf(&msg, "%s@%u",
> > +
> closure->args[i].o->interface->name,
> > +
Re: [PATCH weston.ini.man v2] Improvement of weston.ini.man. Add key:shell and remove tablet-shell
Nobuhiko, These look pretty good assuming they properly compile (I can't compile the man format in my head). One comment, is that we should probably remove tablet-shell from the list of shells while we're at it. It might also be worth noting that this can be used to load other shell plugins than the ones provided with weston. --Jason Ekstrand On Sat, Feb 8, 2014 at 2:01 AM, Nobuhiko Tanibata < nobuhiko_tanib...@xddp.denso.co.jp> wrote: > Add key:shell to CORE SECTION and move a example of desktop-shell from > key:modules to key:shell. > Add cms-colord.so to key:modules of CORE SECTION. > > Signed-off-by: Nobuhiko Tanibata > --- > man/weston.ini.man | 17 ++--- > 1 file changed, 14 insertions(+), 3 deletions(-) > > diff --git a/man/weston.ini.man b/man/weston.ini.man > index ce3f928..7b9ce0a 100644 > --- a/man/weston.ini.man > +++ b/man/weston.ini.man > @@ -92,16 +92,27 @@ The > .B core > section is used to select the startup compositor modules. > .TP 7 > -.BI "modules=" desktop-shell.so,xwayland.so > -specifies the modules to load (string). Available modules in the > +.BI "shell=" desktop-shell.so > +specifies the shell to load (string). Available shells in the > .IR "__weston_modules_dir__" > directory are: > .PP > .RS 10 > .nf > .BR desktop-shell.so > -.BR tablet-shell.so > +.fi > +.RE > +.TP 7 > +.TP 7 > +.BI "modules=" xwayland.so,cms-colord.so > +specifies the modules to load (string). Available modules in the > +.IR "__weston_modules_dir__" > +directory are: > +.PP > +.RS 10 > +.nf > .BR xwayland.so > +.BR cms-colord.so > .fi > .RE > .TP 7 > -- > 1.8.3.1 > > ___ > wayland-devel mailing list > wayland-devel@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/wayland-devel > ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [RFC v2] Wayland presentation extension (video protocol)
> > and seq_lo must be zero. > > > > > >>summary="seconds part of the presentation timestamp"/> > >>summary="nanoseconds part of the presentation timestamp"/> > > >summary="high 32 bits of refresh counter"/> > >>summary="low 32 bits of refresh counter"/> > > > > > > > > > > The content update was never displayed to the user. > > > > > > > > > > > > No-one else asked anything yet, so let me. ;-) > > When a client starts an animation or video playback from scratch, that > is, after a long pause or the first time on the wl_surface, it has to > present at least one frame before it can get any feedback. Because you > want to start playback ASAP and keep it streaming, you have to post > another frame too, before you have time to receive the feedback, and > use it to adjust the submission target times. IOW, the first few frames > have to be scheduled blind, the only information you have is the *set* > of outputs where the wl_surface is on, and the outputs' default refresh > rates. > > Could we somehow calibrate the client's scheduling loop earlier? > > We might add another request to presentation interface: > > > summary="target surface"/> > summary="new feedback object"/> > > > Yeah, this is a second request that creates a presentation_feedback > object, but since both factory requests are in the same interface, > there should not be versioning issues. > > Preroll_feedback would create a new presentation_feedback object, which > the compositor would trigger immediately, providing the timestamp and > MSC of the latest known display refresh (equivalent to vblank), the > surface's current main output, and the output's current refresh period > length. If the compositor cannot implement this, it could just reply > with presentation_feedback.discarded. > > Obviously using this request requires a roundtrip to get the reply from > the compositor, but having the compositor send a continuous stream of > "vblank-events" would be much much worse. > > Once a client gets the reply for preroll_feedback, it then reads the > current UST with clock_gettime() and decides which coming display > refresh it will target. The start should be more accurately in sync > with the display refresh cycle than otherwise. > > However, there are more downsides than just the roundtrip. Even if the > compositor replies immediately, you probably end up starting the > playback a frame later than if you just started blind. If the > compositor cannot access the requested information directly, e.g. it > has not flipped the framebuffer for some time and the drivers don't > support the query, the compositor might need to wait for the next > vblank before it can reply. Or the compositor might reply with so old > data that it is not useful anymore. Any delays would void the benefit > of achieving early sync. > > A more fundamental problem is, that the wl_surface must be mapped > before this request makes sense. If a surface is not mapped, it does > not have size or position, and the compositor does not know which > output it would be syncing to. Hence it cannot reply. > > Considering all this, would it be worth it to have this kind of request? > It seems to me that the client should be able to just read the clock_id, do an initial sync with the audio clock, call clock_gettime, and throw a few "best try" frames at the server. Since the server will try to present it on time, it shouldn't be that far off. Besides, audio sync probably doesn't matter in the first few frames. I'm all for a better solution, but I don't know that creating dummy presentation callbacks is going to gain us enough practical benifit to be worth it. I could be wrong on that point though. --Jason Ekstrand > FWIW, I asked on #dri-devel, and it seems that DRM/KMS should be able > to provide the needed info without waiting for the next vblank. > However, it probably depends on the particular driver in use, does the > driver keep vblank irqs running, and does the hardware have convenient > registers to read the necessary information. To me it sounds a lot more > uncertain than just posting a real frame and seeing when it hits the > user. > > OTOH, GLX_OML_sync_control has glXGetSyncValuesOML() and > glXGetMscRateOML() which provide similar information, so this should be > possible at some level. > > > Thanks, > pq > ___ > wayland-devel mailing list > wayland-devel@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/wayland-devel > ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [RFC v2] Wayland presentation extension (video protocol)
On Wed, Feb 5, 2014 at 1:32 AM, Pekka Paalanen wrote: > On Thu, 30 Jan 2014 17:35:17 +0200 > Pekka Paalanen wrote: > > > Hi, > > > > it's time for a take two on the Wayland presentation extension. > > > > > > 1. Introduction > > > > The v1 proposal is here: > > > http://lists.freedesktop.org/archives/wayland-devel/2013-October/011496.html > > > > In v2 the basic idea is the same: you can queue frames with a > > target presentation time, and you can get accurate presentation > > feedback. All the details are new, though. The re-design started > > from the wish to handle resizing better, preferably without > > clearing the buffer queue. > ... > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > Hi, > > another random thought; should we support queueing frames (buffers) in > reverse chronological order? > > It's not really possible with the scheduling algorithm I wrote down in > the spec. There is no timestamp associated with the currently showing > content, which means that if you queue a frame with a target timestamp > in the past, it will replace the current content, even if the current > content was originally queued with a later target timestamp. > > I wonder if we could define, that the current content effectively has > the timestamp of when it was presented, and all queued updates with > an earlier target timestamp will be discarded. > > That should work, right? > > Now, is there a corner case... output update has been submitted to > hardware but has not been presented yet, which means the content in > flight has no timestamp determined yet... but we won't update the > output again before the update in flight has completed, which gives the > presented timestamp for the was-in-flight current content. > > If we do need the timestamp for content in flight, we could use the > target timestamp it had when queued, or the timestamp the compositor is > targeting. Since clients have a choice between queued and immediate > updates, I guess using the compositor's target timestamp would be > better defined. > > Opinions? > I agree. The current frame (or frame for the currently pending flip) should be treated as if it has a timestamp of its expected next presentation time. I'm still not completely understanding the algorithm for presenting stuff correctly, but it should be possible for the client to sneak a frame in for the next present. I need some time at my chalkboard to try and get my head wrapped around all this. Maybe it would be good if you made a couple of little timeline pictures to go with it? --Jason Ekstrand > I think I should fix it like that. > > Isn't queueing (writing into the audio scanout buffer) audio samples in > reverse chronological order the proper method to update audio content > on the fly with minimal umm... latency? Wonder if some video-like > playback would benefit from a similar algorithm, which minimizes > latency(?) or the difference to wall time at the cost of possibly > skipping older new updates. Errm, to avoid shifting the content > on the time axis. Or something. > > > Thanks, > pq > ___ > wayland-devel mailing list > wayland-devel@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/wayland-devel > ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [RFC v2] Wayland presentation extension (video protocol)
OK, that makes more sense. Thank you for stating it in terms of intervals. I still need to think about it a bit more. On Feb 8, 2014 4:14 PM, "Axel Davy" wrote: > On 08/02/2014, Axel Davy wrote : > > Hi, > > > On 08/02/2014, Jason Ekstrand wrote : > > > >> For each surface with queued content updates and matching main >> output, the compositor picks the update with the highest >> timestamp no later than a half frame period after the predicted >> presentation time. The intent is to pick the content update >> whose target timestamp as rounded to the output refresh period >> granularity matches the same display update as the compositor is >> targeting, while not displaying any content update more than a >> > > I'm not really following 100% here. It's not your fault, this is just a > terribly awkward sort of thing to try and put into English. It sounds to > me like the following: If P0 is the time of the next present and P1 is the > time of the one after that, you look for the largest thing less than the > average of P1 and P2. Is this correct? Why go for the average? The > client is going to have to adjust anyway. > > If you target t, and P0 and P1 are possible pageflips time, > if P0 if (1/2)P0+(1/2)P1 That way the length of the intersection of the interval (t,t+time between > two pageflips) and 'time interval at which it is displayed' is maximized. > > Well it isn't really the reason why this is choosen (else one might say > it would be better to maximize with (t-T/2,t+T/2) with T the time between > two pageflips.). > The reason is more that you want to minimize the time at when the pageflip > happens and t, so the real presentation time and t doesn't differ more than > T/2. > > Axel Davy > > ___ > wayland-devel mailing list > wayland-devel@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/wayland-devel > > ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH weston.ini.man v2] Improvement of weston.ini.man. Add key:shell and remove tablet-shell
On Feb 9, 2014 7:18 AM, "nobuhiko_tanibata" < nobuhiko_tanib...@xddp.denso.co.jp> wrote: > > Hi Jason, > > 2014-02-09 01:54 に Jason Ekstrand さんは書きました: > >> Nobuhiko, >> These look pretty good assuming they properly compile (I can't compile >> the man format in my head). > > > This is snip of result of "man weston.ini". It is exactly compiled. > > CORE SECTION >The core section is used to select the startup compositor modules. > >shell=desktop-shell.so > > specifies the shell to load (string). Available shells in the > /home/user/usrfs/lib/weston directory are: > > desktop-shell.so > >modules=xwayland.so,cms-colord.so > > specifies the modules to load (string). Available modules > in the /home/user/usrfs/lib/weston directory are: > > xwayland.so > cms-colord.so > Thanks. Man is hard to read. > > >> One comment, is that we should probably >> remove tablet-shell from the list of shells while we're at it. It >> might also be worth noting that this can be used to load other shell >> plugins than the ones provided with weston. > > > Yes. Shall we add more clear description like, > "specifies the shells to load (string). This can be used for your own implemented shell or the ones with Weston as default. Available shells with Weston as default in the /home/user/usrfs/lib/weston directory are: > desktop-shell. Yes, I like that. --Jason Ekstrand > > BR, > Nobuhiko > >> --Jason Ekstrand >> >> On Sat, Feb 8, 2014 at 2:01 AM, Nobuhiko Tanibata >> wrote: >> >>> Add key:shell to CORE SECTION and move a example of desktop-shell >>> from key:modules to key:shell. >>> Add cms-colord.so to key:modules of CORE SECTION. >>> >>> Signed-off-by: Nobuhiko Tanibata >>> >>> --- >>> man/weston.ini.man | 17 ++--- >>> 1 file changed, 14 insertions(+), 3 deletions(-) >>> >>> diff --git a/man/weston.ini.man b/man/weston.ini.man >>> index ce3f928..7b9ce0a 100644 >>> --- a/man/weston.ini.man >>> +++ b/man/weston.ini.man >>> @@ -92,16 +92,27 @@ The >>> .B core >>> section is used to select the startup compositor modules. >>> .TP 7 >>> -.BI "modules=" desktop-shell.so,xwayland.so >>> -specifies the modules to load (string). Available modules in the >>> +.BI "shell=" desktop-shell.so >>> +specifies the shell to load (string). Available shells in the >>> .IR "__weston_modules_dir__" >>> directory are: >>> .PP >>> .RS 10 >>> .nf >>> .BR desktop-shell.so >>> -.BR tablet-shell.so >>> +.fi >>> +.RE >>> +.TP 7 >>> +.TP 7 >>> +.BI "modules=" xwayland.so,cms-colord.so >>> +specifies the modules to load (string). Available modules in the >>> +.IR "__weston_modules_dir__" >>> +directory are: >>> +.PP >>> +.RS 10 >>> +.nf >>> .BR xwayland.so >>> +.BR cms-colord.so >>> .fi >>> .RE >>> .TP 7 >>> -- >>> 1.8.3.1 >>> >>> ___ >>> wayland-devel mailing list >>> wayland-devel@lists.freedesktop.org >>> http://lists.freedesktop.org/mailman/listinfo/wayland-devel [1] >> >> >> >> >> Links: >> -- >> [1] http://lists.freedesktop.org/mailman/listinfo/wayland-devel ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [RFC v2] Wayland presentation extension (video protocol)
On Mon, Feb 10, 2014 at 3:53 AM, Pekka Paalanen wrote: > On Sat, 8 Feb 2014 15:23:29 -0600 > Jason Ekstrand wrote: > > > Pekka, > > First off, I think you've done a great job over-all. I think it will > both > > cover most cases and work well I've got a few comments below. > > Thank you for the review. :-) > Replies below. > > > On Thu, Jan 30, 2014 at 9:35 AM, Pekka Paalanen > wrote: > > > > > Hi, > > > > > > it's time for a take two on the Wayland presentation extension. > > > > > > > > > 1. Introduction > > > > > > The v1 proposal is here: > > > > > > > http://lists.freedesktop.org/archives/wayland-devel/2013-October/011496.html > > > > > > In v2 the basic idea is the same: you can queue frames with a > > > target presentation time, and you can get accurate presentation > > > feedback. All the details are new, though. The re-design started > > > from the wish to handle resizing better, preferably without > > > clearing the buffer queue. > > > > > > All the changed details are probably too much to describe here, > > > so it is maybe better to look at this as a new proposal. It > > > still does build on Frederic's work, and everyone who commented > > > on it. Special thanks to Axel Davy for his counter-proposal and > > > fighting with me on IRC. :-) > > > > > > Some highlights: > > > > > > - Accurate presentation feedback is possible also without > > > queueing. > > > > > > - You can queue also EGL-based rendering, and get presentation > > > feedback if you want. Also EGL can do this internally, too, as > > > long as EGL and the app do not try to use queueing at the same time. > > > > > > - More detailed presentation feedback to better allow predicting > > > future display refreshes. > > > > > > - If wl_viewport is used, neither video resolution changes nor > > > surface (window) size changes alone require clearing the queue. > > > Video can continue playing even during resizes. > ... > > > > > > > > > The main features of this interface are accurate presentation > > > timing feedback, and queued wl_surface content updates to ensure > > > smooth video playback while maintaining audio/video > > > synchronization. Some features use the concept of a presentation > > > clock, which is defined in presentation.clock_id event. > > > > > > Requests 'feedback' and 'queue' can be regarded as additional > > > wl_surface methods. They are part of the double-buffered > > > surface state update mechanism, where other requests first set > > > up the state and then wl_surface.commit atomically applies the > > > state into use. In other words, wl_surface.commit submits a > > > content update. > > > > > > Interface wl_surface has requests to set surface related state > > > and buffer related state, because there is no separate interface > > > for buffer state alone. Queueing requires separating the surface > > > from buffer state, and buffer state can be queued while surface > > > state cannot. > > > > > > Buffer state includes the wl_buffer from wl_surface.attach, the > > > state assigned by wl_surface requests frame, > > > set_buffer_transform and set_buffer_scale, and any > > > buffer-related state from extensions, for instance > > > wl_viewport.set_source. This state is inherent to the buffer > > > and the content update, rather than the surface. > > > > > > Surface state includes all other state associated with > > > wl_surfaces, like the x,y arguments of wl_surface.attach, input > > > and opaque regions, damage, and extension state like > > > wl_viewport.destination. In general, anything expressed in > > > surface local coordinates is better as surface state. > > > > > > The standard way of posting new content to a surface using the > > > wl_surface requests damage, attach, and commit is called > > > immediate content submission. This happens when a > > > presentation.queue request has not been sent since the last > > > wl_surface.commit. > > > > > > The new way of posting a
Re: [RFC v2] Wayland presentation extension (video protocol)
On Mon, Feb 10, 2014 at 5:51 AM, Pekka Paalanen wrote: > On Sat, 8 Feb 2014 15:39:57 -0600 > Jason Ekstrand wrote: > > > On Wed, Feb 5, 2014 at 1:32 AM, Pekka Paalanen > wrote: > > > > > On Thu, 30 Jan 2014 17:35:17 +0200 > > > Pekka Paalanen wrote: > > > > > > > Hi, > > > > > > > > it's time for a take two on the Wayland presentation extension. > > > > > > > > > > > > 1. Introduction > > > > > > > > The v1 proposal is here: > > > > > > > > http://lists.freedesktop.org/archives/wayland-devel/2013-October/011496.html > > > > > > > > In v2 the basic idea is the same: you can queue frames with a > > > > target presentation time, and you can get accurate presentation > > > > feedback. All the details are new, though. The re-design started > > > > from the wish to handle resizing better, preferably without > > > > clearing the buffer queue. > > > ... > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > Hi, > > > > > > another random thought; should we support queueing frames (buffers) in > > > reverse chronological order? > > > > > > It's not really possible with the scheduling algorithm I wrote down in > > > the spec. There is no timestamp associated with the currently showing > > > content, which means that if you queue a frame with a target timestamp > > > in the past, it will replace the current content, even if the current > > > content was originally queued with a later target timestamp. > > > > > > I wonder if we could define, that the current content effectively has > > > the timestamp of when it was presented, and all queued updates with > > > an earlier target timestamp will be discarded. > > > > > > That should work, right? > > > > > > Now, is there a corner case... output update has been submitted to > > > hardware but has not been presented yet, which means the content in > > > flight has no timestamp determined yet... but we won't update the > > > output again before the update in flight has completed, which gives the > > > presented timestamp for the was-in-flight current content. > > > > > > If we do need the timestamp for content in flight, we could use the > > > target timestamp it had when queued, or the timestamp the compositor is > > > targeting. Since clients have a choice between queued and immediate > > > updates, I guess using the compositor's target timestamp would be > > > better defined. > > > > > > Opinions? > > > > > > > I agree. The current frame (or frame for the currently pending flip) > > should be treated as if it has a timestamp of its expected next > > presentation time. I'm still not completely understanding the algorithm > > for presenting stuff correctly, but it should be possible for the client > to > > sneak a frame in for the next present. > > > > I need some time at my chalkboard to try and get my head wrapped around > all > > this. Maybe it would be good if you made a couple of little timeline > > pictures to go with it? > > Hi, > > I took the advice and drew a picture, attached. The picture is only > about choosing the right update from the queue for the coming output > refresh, and ignores the surface's existing content. > Thank you! A picture really is worth 1000 words and, in my experience, an equation is worth about 200 or so. That helps clear things up a lot. > > A compositor has a queue with updates T1..T5. The current time is > indicated in the picture, and the next output refresh will be P4. From > the queue, T4 will be selected, and T1, T2, and T3 are discarded. Both > T3 and T4 target the same flip, so the one with the latter timestamp > wins. T5 is too far in the future. > > The decision boundaries are at the middle point between the two refresh > times. > > I drew this without rechecking my spec wording, this is what I meant > there IIRC. > > The already presented content update's timestamp can only be in the > past, so it does not affect the drawn situation.
[RFC v3] Fullscreen shell
This RFC is for the third version of my fullscreen shell implementation. The contents of this RFC are: * A new wl_fullscreen_shell protocol * A weston shell that provides wl_fullscreen_shell * Additions to the Wayland backend for Weston to take advantage of wl_fullscreen_shell * Additions to weston-simple-shm and weston-fullscreen to demonstrate the wl_fullscreen_shell protocol. This RFC improves the second version in a couple of ways: * Changes the possible presentation modes to allow more options for clients * Adds support in weston-fullscreen for fully testing wl_fullscreen_shell * Various bugfixes The previous version of this RFC can be found here: http://lists.freedesktop.org/archives/wayland-devel/2013-October/011626.html The original RFC can be found here: http://lists.freedesktop.org/archives/wayland-devel/2013-August/010720.html This RFC is primarily to provide a preview of the implementation, so I am not going to spam the list with patches. Instead, you can view it in its entirety on my github: https://github.com/jekstrand/weston/tree/fullscreen-shell-RFCv3 Immediately following this e-mail will be a fourth protocol-only RFC that contains substantial changes to the modesetting portion of the protocol. Feedback there is appreciated as well. Thanks, --Jason Ekstrand Jason Ekstrand (11): Add a fullscreen shell protocol Generate/build the fullscreen shell protocol files Add a signal for when a seat updates its capabilities Add a wl_fullscreen_shell implementation simple-shm: Add fullscreen shell support toytoolkit: Only require xdg_shell if the window is not custom Add wl_fullscreen_shell support to weston-fullscreen compositor-wayland: Add support for running on top of wl_fullscreen_shell compositor-wayland: Add a --sprawl option Automatically select the wayland backend if WAYLAND_SOCKET is set Properly handle running inside a compositor that does not provide keymaps Makefile.am | 32 +++ clients/fullscreen.c | 84 +- clients/simple-shm.c | 31 ++- clients/window.c | 2 +- configure.ac | 8 + protocol/fullscreen-shell.xml | 70 + src/compositor-wayland.c | 347 --- src/compositor.c | 3 +- src/compositor.h | 1 + src/fullscreen-shell.c| 626 ++ src/input.c | 2 + 11 files changed, 1145 insertions(+), 61 deletions(-) create mode 100644 protocol/fullscreen-shell.xml create mode 100644 src/fullscreen-shell.c -- 1.8.5.3 ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
[RFC v4] Fullscreen shell protocol
The following is yet another take on the fullscreen shell protocol. Previous versions more-or-less followed the approach taken in wl_shell. This version completely reworks the concept. In particular, the protocol is split into two use-cases. The first is that of a simple client that wants to present a surface or set of surfaces possibly with some scaling. This happens through the present_surface request which looks similar to that of wl_shell only without the modesetting. The second use-case is of a client that wants more control over the outputs. In this case, the client uses the present_surface_for_mode request to present the surface at a particular output mode. This request provides a more-or-less atomic modeset operation. If the compositor can satisfy the requested mode, then the mode is changed and the new surface is presented. Otherwise, the compositor harmlessly falls back to the previously presented surface and the client is informed that the switch failed. This way, the surface is either displayed correctly or not at all. Of course, a client is free to call present_surface_for_mode with the currently presented surface and hope for the best. However, this may result in strange behavior and there is no reliable fallback if the mode switch fails. In particular, I would like feedback on the modesetting portion of this protocol. This is particularly targetted at compositors that want to run inside weston or some other fullscreen compositor. In the next week or so, I will attempt to implement all this in weston and see how well it works. However, I would also like to know how well this will work for other compositors such as KWin or Hawaii. Thanks for your feedback, --Jason Ekstrand = Protocol follows: = Displays a single surface per output. This interface provides a mechanism for a single client to display simple full-screen surfaces. While there technically may be multiple clients bound to this interface, only one of those clients should be shown at a time. To present a surface, the client uses either the present_surface or present_surface_for_mode requests. Presenting a surface takes effect on the next wl_surface.commit. See the individual requests for details about scaling and mode switches. The client can have at most one surface per output at any time. Requesting a surface be presented on an output that already has a surface replaces the previously presented surface. Presenting a null surface removes its content and effectively disables the output. Exactly what happens when an output is "disabled" is compositor-specific. The same surface may be presented multiple outputs simultaneously. Hints to indicate to the compositor how to deal with a conflict between the dimensions of the surface and the dimensions of the output. The compositor is free to ignore this parameter. Present a surface on the given output. If the output is null, the compositor will present the surface on whatever display (or displays) it thinks best. In particular, this may replace any or all surfaces currently presented so it should not be used in combination with placing surfaces on specific outputs. The method parameter is a hit to the compositor for how the surface is to be presented. In particular, it tells the compostior how to handle a size mismatch between the presented surface and the output. The compositor is free to ignore this parameter. The "zoom", "zoom_crop", and "stretch" methods imply a scaling operation on the surface. This will override any kind of output scaling, so the buffer_scale property of the surface is effectively ignored. Presents a surface on the given output for a particular mode. If the current size of the output differs from that of the surface, the compositor will attempt to change the size of the output to match the surface. The result of the mode-swith operation will be returned via the provided wl_fullscreen_shell_mode_feedback object. If the current output mode matches the one requested or if the compositor successfully switches the mode to match the surface, then the mode_successfull event will be sent and the output will contain the contents of the given surface. If the compositor cannot match the output size to the surface size, the mode_failed will be sent and the output will contain the contents of the previously presented surface (if any). If another surface is presented on the given output before either of these has a chance to ha
Re: [RFC v4] Fullscreen shell protocol
Hi Pekka! Thanks for the review. Comments follow. On Fri, Feb 14, 2014 at 1:14 AM, Pekka Paalanen wrote: > Hi Jason > > On Thu, 13 Feb 2014 22:37:53 -0600 > Jason Ekstrand wrote: > > > The following is yet another take on the fullscreen shell protocol. > > Previous versions more-or-less followed the approach taken in wl_shell. > > This version completely reworks the concept. In particular, the protocol > > is split into two use-cases. The first is that of a simple client that > > wants to present a surface or set of surfaces possibly with some scaling. > > This happens through the present_surface request which looks similar to > > that of wl_shell only without the modesetting. > > > > The second use-case is of a client that wants more control over the > > outputs. In this case, the client uses the present_surface_for_mode > > request to present the surface at a particular output mode. This request > > provides a more-or-less atomic modeset operation. If the compositor can > > satisfy the requested mode, then the mode is changed and the new surface > is > > presented. Otherwise, the compositor harmlessly falls back to the > > previously presented surface and the client is informed that the switch > > failed. This way, the surface is either displayed correctly or not at > all. > > Of course, a client is free to call present_surface_for_mode with the > > currently presented surface and hope for the best. However, this may > > result in strange behavior and there is no reliable fallback if the mode > > switch fails. > > > > In particular, I would like feedback on the modesetting portion of this > > protocol. This is particularly targetted at compositors that want to run > > inside weston or some other fullscreen compositor. In the next week or > so, > > I will attempt to implement all this in weston and see how well it works. > > However, I would also like to know how well this will work for other > > compositors such as KWin or Hawaii. > > > > Thanks for your feedback, > > --Jason Ekstrand > > > > = Protocol follows: = > > > > > > > > This interface should have a destructor request IMO. It's not stricly > required, but I think it would be consistent (I think all global > interfaces need an explicit destructor request) and more future-proof. > Thanks for reminding me. I'll get one added. > > > > > Displays a single surface per output. > > > > This interface provides a mechanism for a single client to display > > simple full-screen surfaces. While there technically may be > multiple > > clients bound to this interface, only one of those clients should > be > > shown at a time. > > > > To present a surface, the client uses either the present_surface or > > present_surface_for_mode requests. Presenting a surface takes > effect > > on the next wl_surface.commit. See the individual requests for > > details about scaling and mode switches. > > > > The client can have at most one surface per output at any time. > > Requesting a surface be presented on an output that already has a > > surface replaces the previously presented surface. Presenting a > null > > surface removes its content and effectively disables the output. > > Exactly what happens when an output is "disabled" is > > compositor-specific. The same surface may be presented multiple > > outputs simultaneously. > > If the same surface is presented on multiple outputs, should the client > have a way to say which output is to be considered the surface's main > output, where e.g. presentation feedback is synced to? > That's a good question. Simple clients probably don't care. More complex clients such as compositors probably will. However, I'd expect them to have one surface per output most of the time anyway. I'll give that some thought. > Maybe also note explicitly, that once a surface has been presented on > an output, it stays on that output until explicitly removed, or output > is unplugged? So that simple attach+damage+commit can be used to update > the content, if that is the intention. > Yes, that's a good point. And I do intend to provide that guarantee. > > > > > > > > > > > Hints to indicate to the compositor how to deal with a conflict > > between the dimensions of the surface and the dimensions of the > > output. The compositor is free to ignore this parameter. &
Re: Core protocol change; [RFC v2] Wayland presentation extension
here is a wl_surface.commit. > The practical consequence of that was that a commit without an attach > cannot cause any wl_buffer on this surface to become reserved and > readable by the server, and hence no (new) wl.buffer.release would be > posted either. > > That means that clients already need to re-attach the same wl_buffer > again, if they changed the buffer contents and want to show the new > image. I think this should mitigate the impact of the core protocol > change. > > I guess the only interesting case is the frame callback, and whether > anyone (ab)uses it without an attach. Someone does abuse it right now: Weston. Inside the Weston Wayland backend, every time we want a redraw we frame+commit, wait for the frame callback, and then repaint. The actual reason for this is to maintain the frame callbacks inside of the nested weston, so that can be changed. Also, if we change weston to take a more timing-based approach to repaint scheduling (as you have mentioned in the past) that would also somewhat mitigate this problem. One of the reasons why this is specifically a problem is because it means that EGL clients can't update any surface state without either a full repaint or a new EGL extension. The obvious answer is "just re-send the old wl_buffer". However, EGL clients don't have acess to it and can't re-send. They can only repaing and have EGL send a new buffer. Therefore, if we want to allow for updating surface state without repainting, we need a way to say "just use the previous one" Another thought is what Axel Davy brought up on IRC this morning, that of swapInterval > 1. However, here it isn't a problem because eglSwapInterval is handled by the EGL implementation and it does have access to the previously submitted wl_buffer so it can resubmit. Also (again, Axel pointed this out) there is a race here between the wl_surface.attach and the wl_buffer.release. However, if we specify that wl_buffer.release gets called once for each wl_surface.attach this can be handled by simple client-side reference-counting. Still kind of a pain though. Also on IRC, you brought up the idea of changing the frame callback to be more of a "you should redraw now" and less of a "your buffer just got put onscreen". Obviously, this breaks anything (such as Weston's wayland-backend.so) that uses the frame callback as anything other than a throttling hint. My feeling is that changing the frame callback to be more of a "you should re-draw now" is probably ok as long as we do two things: a) Figure out how many clients actually abuse it and how much damage changing it will cause. b) guarantee that any buffer committed after the frame callback has been sent will be presented strictly later than the one associated with the frame callback. Without this basic guarantee, it would again become useless. There's my two cents. Hope it's useful to someone. --Jason Ekstrand > > Would this proposition be acceptable? > > > Thanks, > pq > > > If a wl_surface has queued content updates when it is destroyed, > > the whole queue is implicitly discarded as if > > presentation.discard_queue was sent immediately prior to the > > destroy request. > > > > > > > > > > Informs the server that the client will not be using this > > protocol object anymore. This does not affect any content > > update queues nor existing objects created by this interface. > > > > > > > > > > > > With this request, presentation feedback will be provided for > > the current content submission of the given surface. A new > > presentation_feedback object is created, and that object will > > deliver the information once. The object is tied to this > > content submission only. Multiple presentation_feedback objects > > may be created for the same submission, and they will all > > return the same information. > > > > For details on what information is returned, see > > presentation_feedback interface. > > > > > >>summary="target surface"/> > >>summary="new feedback object"/> > > > > > > > > > > This request changes the behaviour of the very next > > wl_surface.commit of the given wl_surface and that commit > > only. Instead of immediately applying the pending wl_surface > > state as defined in wl_surface.commit, the commit will queue a > > new content update, using the pending buffer state only. > > > > For a more detailed description and what is buffer state, see > > the documentation for presentation interface. > > > > The value of the target timestamp is in the presentation clock > > domain, see presentation.clock_id. > > > > If queue request has already been sent for the unfinished content > > update submission on the given wl_surface, a new queue request > > will override the previous one. > > > > > >>summary="target surface"/> > >>summary="seconds part of the target timestamp"/> > >>summary="nanoseconds part of the target timestamp"/> > > > > ___ > wayland-devel mailing list > wayland-devel@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/wayland-devel ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH] connection: Use wl_log to report errors
Yeah, I've wanted to do that for a while.
Reviewed-By: Jason Ekstrand
On Feb 17, 2014 6:04 PM, "Jasper St. Pierre" wrote:
> In some cases, like Xwayland, stdout and stderr are redirected to
> /dev/null, losing us valuable information, while wl_log can be
> overridden, allowing us to send it to a log file instead. This
> can help debugging immensely.
> ---
> src/connection.c | 36 ++--
> 1 file changed, 18 insertions(+), 18 deletions(-)
>
> diff --git a/src/connection.c b/src/connection.c
> index d05341a..40a2182 100644
> --- a/src/connection.c
> +++ b/src/connection.c
> @@ -512,7 +512,7 @@ wl_closure_marshal(struct wl_object *sender, uint32_t
> opcode,
>
> count = arg_count_for_signature(message->signature);
> if (count > WL_CLOSURE_MAX_ARGS) {
> - printf("too many args (%d)\n", count);
> + wl_log("too many args (%d)\n", count);
> errno = EINVAL;
> return NULL;
> }
> @@ -557,13 +557,13 @@ wl_closure_marshal(struct wl_object *sender,
> uint32_t opcode,
> fd = args[i].h;
> dup_fd = wl_os_dupfd_cloexec(fd, 0);
> if (dup_fd < 0) {
> - fprintf(stderr, "dup failed: %m");
> + wl_log("dup failed: %m");
> abort();
> }
> closure->args[i].h = dup_fd;
> break;
> default:
> - fprintf(stderr, "unhandled format code: '%c'\n",
> + wl_log("unhandled format code: '%c'\n",
> arg.type);
> assert(0);
> break;
> @@ -615,7 +615,7 @@ wl_connection_demarshal(struct wl_connection
> *connection,
>
> count = arg_count_for_signature(message->signature);
> if (count > WL_CLOSURE_MAX_ARGS) {
> - printf("too many args (%d)\n", count);
> + wl_log("too many args (%d)\n", count);
> errno = EINVAL;
> wl_connection_consume(connection, size);
> return NULL;
> @@ -642,7 +642,7 @@ wl_connection_demarshal(struct wl_connection
> *connection,
> signature = get_next_argument(signature, &arg);
>
> if (arg.type != 'h' && p + 1 > end) {
> - printf("message too short, "
> + wl_log("message too short, "
>"object (%d), message %s(%s)\n",
>*p, message->name, message->signature);
> errno = EINVAL;
> @@ -669,7 +669,7 @@ wl_connection_demarshal(struct wl_connection
> *connection,
>
> next = p + DIV_ROUNDUP(length, sizeof *p);
> if (next > end) {
> - printf("message too short, "
> + wl_log("message too short, "
>"object (%d), message %s(%s)\n",
>closure->sender_id, message->name,
>message->signature);
> @@ -680,7 +680,7 @@ wl_connection_demarshal(struct wl_connection
> *connection,
> s = (char *) p;
>
> if (length > 0 && s[length - 1] != '\0') {
> - printf("string not nul-terminated, "
> + wl_log("string not nul-terminated, "
>"message %s(%s)\n",
>message->name, message->signature);
> errno = EINVAL;
> @@ -695,7 +695,7 @@ wl_connection_demarshal(struct wl_connection
> *connection,
> closure->args[i].n = id;
>
> if (id == 0 && !arg.nullable) {
> - printf("NULL object received on
> non-nullable "
> + wl_log("NULL object received on
> non-nullable "
>"type, message %s(%s)\n",
> message->name,
>message->signature);
> errno = EINVAL;
> @@ -707,7 +707
Re: [RFC v4] Fullscreen shell protocol
I just added an implementation of RFCv4 which can be found here: https://github.com/jekstrand/weston/tree/fullscreen-shell-RFCv4 Thanks, --Jason Ekstrand On Sat, Feb 15, 2014 at 1:12 AM, Pekka Paalanen wrote: > On Fri, 14 Feb 2014 11:11:33 -0600 > Jason Ekstrand wrote: > > > Hi Pekka! Thanks for the review. Comments follow. > > > > > > On Fri, Feb 14, 2014 at 1:14 AM, Pekka Paalanen > > wrote: > > > > > Hi Jason > > > > > > On Thu, 13 Feb 2014 22:37:53 -0600 > > > Jason Ekstrand wrote: > > > > > > > The following is yet another take on the fullscreen shell > > > > protocol. Previous versions more-or-less followed the > > > > approach taken in wl_shell. This version completely reworks > > > > the concept. In particular, the protocol is split into two > > > > use-cases. The first is that of a simple client that wants > > > > to present a surface or set of surfaces possibly with some > > > > scaling. This happens through the present_surface request > > > > which looks similar to that of wl_shell only without the > > > > modesetting. > > > > > > > > The second use-case is of a client that wants more control > > > > over the outputs. In this case, the client uses the > > > > present_surface_for_mode request to present the surface at a > > > > particular output mode. This request provides a more-or-less > > > > atomic modeset operation. If the compositor can satisfy the > > > > requested mode, then the mode is changed and the new surface > > > is > > > > presented. Otherwise, the compositor harmlessly falls back > > > > to the previously presented surface and the client is > > > > informed that the switch failed. This way, the surface is > > > > either displayed correctly or not at > > > all. > > > > Of course, a client is free to call present_surface_for_mode > > > > with the currently presented surface and hope for the best. > > > > However, this may result in strange behavior and there is no > > > > reliable fallback if the mode switch fails. > > > > > > > > In particular, I would like feedback on the modesetting > > > > portion of this protocol. This is particularly targetted at > > > > compositors that want to run inside weston or some other > > > > fullscreen compositor. In the next week or > > > so, > > > > I will attempt to implement all this in weston and see how > > > > well it works. However, I would also like to know how well > > > > this will work for other compositors such as KWin or Hawaii. > > > > > > > > Thanks for your feedback, > > > > --Jason Ekstrand > > > > > > > > = Protocol follows: = > > > > > > > > > > > > > > > > > > This interface should have a destructor request IMO. It's not > > > stricly required, but I think it would be consistent (I think > > > all global interfaces need an explicit destructor request) and > > > more future-proof. > > > > > > > Thanks for reminding me. I'll get one added. > > > > > > > > > > > Displays a single surface per output. > > > > > > > > This interface provides a mechanism for a single client > > > > to display simple full-screen surfaces. While there > > > > technically may be > > > multiple > > > > clients bound to this interface, only one of those > > > > clients should > > > be > > > > shown at a time. > > > > > > > > To present a surface, the client uses either the > > > > present_surface or present_surface_for_mode requests. > > > > Presenting a surface takes > > > effect > > > > on the next wl_surface.commit. See the individual > > > > requests for details about scaling and mode switches. > > > > > > > > The client can have at most one surface per output at > > > > any time. Requesting a surface be presented on an output that > > > > already has a surface replaces the previously presented > > > > surface. Presenting a > > > null > > > > surface removes its content and effectively disables > > > > the output. Exactly what happens when an outpu
Re: Summary of the security discussions around Wayland and privileged clients
On Thu, Feb 20, 2014 at 2:41 PM, Thiago Macieira wrote: > Em qui 20 fev 2014, às 21:31:59, Martin Peres escreveu: > > > Now, the privileged process wants to launch a sub-process. How will the > > > sub- process connect to the compositor? Remember: WAYLAND_SOCKET > contains > > > a file descriptor number that isn't available to the child process. > > > > Ah, I see. You are suggesting un-setting WAYLAND_SOCKET and using > fcntl() to > > set the socket's fd to CLOEXEC? > > Setting the socket to O_CLOEXEC is mandatory after you start using it. Two > processes cannot write to the same streaming socket file descriptor at the > same > time. You might be able to do this with a datagram socket, but that's not > the > case here. > > > It is true that multiple process could end up with the same connection > and > > I didn't think about that. The problem is the same if an application > > connects > > to the compositor by itself and then forks. Not sure how the compositor > > could detect that :s > > It can't. It will get very confused because two applications with > independent > states will start stepping on each other's toes. The best outcome of this > would be if the compositor detected a problem early on and cut the > connection > to both. > > The way I see it, wl_display_connect() must unset the WAYLAND_SOCKET > environment variable after getting the file descriptor number and it must > set > O_CLOEXEC. The socket is not available to child processes. > It already does both of these: http://cgit.freedesktop.org/wayland/wayland/tree/src/wayland-client.c#n763 > > But then the question returns: how do child processes connect to the > compositor, if the environment variable was cleared? How do they find the > compositor? > > Solutions: > > 1) the compositor MUST have a well-known socket name > => not an option, since we want to have multiple concurrent compositors > > 2) wl_display_connect() doesn't clear the environment, but resets it to the > actual socket name. It needs to get the socket name from somewhere. > => problem: if it's getting the name from the compositor, this may take a > few roundtrips and the process may have decided to start the child > process > > 3) use a different environment variable. One variable contains the > traditional > socket path and the other contains the file descriptor. The latter > overrides > the former. > This is what we are doing right now. The WAYLAND_DISPLAY environment variable contains the name of a socket that can be found in XDG_RUNTIME_DIR. The WAYLAND_SOCKET environment variable (if it exists) contains a file descriptor that a single client can use to connect to the compositor. Inside wl_display_connect, we clear WAYLAND_SOCKET but not WAYLAND_DISPLAY. If both are present, the client connects directly and is then free to spawn other clients which will connect via WAYLAND_DISPLAY. However, those other clients will not have the same priviledged status because they didn't use the direct connection. This is the way Weston's desktop-shell panel spawns things right now. > 4) store both settings in WAYLAND_SOCKET. D-Bus does that: > DBUS_SESSION_BUS_ADDRESS can contain multiple addresses, to be attempted in > order. > That's an interesting idea. Not sure what benefit it would have for Wayland right now though. Hope that helps, --Jason Ekstrand ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH wayland v2] protocol: try to clarify frame callback semantics
Pekka, Sorry this e-mail took so long to send. Not much time lately. The first time or two I read this suggested re-wording I didn't like it, but now it's starting to grow on me. I still kind of like the idea of "the buffer you sent is now in use, go ahead and send the next one" but I don't know that it's that much better or that it actually changes anything. The big thing I'd like to leave open (and I think your change does) is the following: Suppose a client commits a buffer and then, several seconds later (after the attached buffer was first used), the user does something that causes the client to refresh. If it does a frame+commit without an attach, the server should be able to respond immediately without waiting for another pageflip. This way the client may be able to render in time for the next flip. Sure, the client might be too slow and miss the flip, but that's really no worse than waiting before sending the frame callback. Point is, it should be a compositor decision and I think you made that clear enough. Looks good to me. --Jason Ekstrand Reviewed-by: Jason Ekstrand On Fri, Feb 21, 2014 at 7:46 AM, Pekka Paalanen wrote: > From: Pekka Paalanen > > "the callback event will arrive after the next output refresh" is wrong, > if you interpret "output refresh" as framebuffer flip or the moment when > the new pixels turn into light the first time. Weston has probably never > worked this way. > > Weston triggers the frame callbacks when it submits repainting commands > to the GPU, which is before the framebuffer flip. > > Strike the incorrect claim, and the rest of the paragraph which no > longer offers useful information. > > As a replacement, expand on the "throttling and driving animations" > characteristic. The main purpose is to let clients animate at the > display refresh rate, while avoiding drawing frames that will never be > presented. > > The new claim is that the server should give some time between > triggering frame callbacks and repainting itself, for clients to draw > and commit. This is somewhat intimate with the repaint scheduling > algorithm a compositor uses, but hopefully the right intention. > > Another point of this update is to imply, that frame callbacks should > not be used to count compositor repaint cycles nor monitor refresh > cycles. It has never been guaranteed to work. Removing the mention of > frame callback without an attach hopefully discourages such use. > > v2: don't just remove a paragraph, but add useful information about the > request's intent. > > Signed-off-by: Pekka Paalanen > Cc: Axel Davy > Cc: Jason Ekstrand > --- > protocol/wayland.xml | 26 ++ > 1 file changed, 18 insertions(+), 8 deletions(-) > > diff --git a/protocol/wayland.xml b/protocol/wayland.xml > index e1edbe5..6e370ad 100644 > --- a/protocol/wayland.xml > +++ b/protocol/wayland.xml > @@ -1059,22 +1059,32 @@ > > > > - > - Request notification when the next frame is displayed. Useful > - for throttling redrawing operations, and driving animations. > + > + Request a notification when it is a good time start drawing a new > + frame, by creating a frame callback. This is useful for throttling > + redrawing operations, and driving animations. > + > + When a client is animating on a wl_surface, it can use the 'frame' > + request to get notified when it is a good time to draw and commit > the > + next frame of animation. If the client commits an update earlier > than > + that, it is likely that some updates will not make it to the > display, > + and the client is wasting resources by drawing too often. > + > The frame request will take effect on the next wl_surface.commit. > The notification will only be posted for one frame unless > requested again. > > + The server must send the notifications so that a client > + will not send excessive updates, while still allowing > + the highest possible update rate for clients that wait for the > reply > + before drawing again. The server should give some time for the > client > + to draw and commit after sending the frame callback events to let > them > + hit the next output refresh. > + > A server should avoid signalling the frame callbacks if the > surface is not visible in any way, e.g. the surface is off-screen, > or completely obscured by other opaque surfaces. > > - A client can request a frame callback even without an attach, > - damage, or any other state changes. wl_surface.commit triggers a > -
Re: [PATCH wayland v2] protocol: try to clarify frame callback semantics
On Feb 22, 2014 2:44 AM, "Axel Davy" wrote: > > Hi, > > I like very much the rewording proposed by Pekka. > > But I dislike your proposition to send frame callbacks right away if the attached buffer has been attached for a long time. > > Your argument seems to be that the client may manage to get to the next pageflip if the frame callback is called right away. But with this argument, I don't see why this behaviour would be only for buffers attached long ago (and then we refresh at a higher frequency than the screen refresh) > > Moreover we may say we can always get the two behaviours with client side code: > . If we keep the current behaviour, the client could know it has attached a buffer for a long time (and that the frame callback it had put, was already called), so if it wants to try to hit next pageflip, it could just commit right away with a new attach Yes, this is what they should be doing. The more I think about the frame callback, the more I think that clients should just put one with every new draw and just track whether that one has been released or not. Unfortunately, we have to do something reasonable in the case where the client requests a frame without drawing. I don't want to restrict the server too much on what it does in that case. It may, for instance, be running on some sort of refresh-on-demand hardware and have no concept of "next flip". When the client asks for a frame callback, it is effectively asking when is a good time to repaint. If now is a good time, the server should be allowed to say so. Pekka, one thing I forgot to mention that should probably be added is that we really should guarantee that frame callbacks get fired in the same order they are requested (per surface order, not global order). > . With your proposition the client could always attach (and perhaps +damage) with a frame+commit (even with the old buffer not released), to be sure to get current behaviour. What do you mean by "current behavior"? And why would they want it? > > I don't think having to do an attach with the old buffer is a good idea, and I favor Pekka's proposition. I wasn't arguing against it. :-) > > Axel Davy > > On 22/02/2014, Jason Ekstrand wrote : >> >> Pekka, >> Sorry this e-mail took so long to send. Not much time lately. The first time or two I read this suggested re-wording I didn't like it, but now it's starting to grow on me. I still kind of like the idea of "the buffer you sent is now in use, go ahead and send the next one" but I don't know that it's that much better or that it actually changes anything. >> >> The big thing I'd like to leave open (and I think your change does) is the following: Suppose a client commits a buffer and then, several seconds later (after the attached buffer was first used), the user does something that causes the client to refresh. If it does a frame+commit without an attach, the server should be able to respond immediately without waiting for another pageflip. This way the client may be able to render in time for the next flip. Sure, the client might be too slow and miss the flip, but that's really no worse than waiting before sending the frame callback. >> >> Point is, it should be a compositor decision and I think you made that clear enough. >> >> Looks good to me. >> --Jason Ekstrand >> >> Reviewed-by: Jason Ekstrand >> >> On Fri, Feb 21, 2014 at 7:46 AM, Pekka Paalanen wrote: >>> >>> From: Pekka Paalanen >>> >>> "the callback event will arrive after the next output refresh" is wrong, >>> if you interpret "output refresh" as framebuffer flip or the moment when >>> the new pixels turn into light the first time. Weston has probably never >>> worked this way. >>> >>> Weston triggers the frame callbacks when it submits repainting commands >>> to the GPU, which is before the framebuffer flip. >>> >>> Strike the incorrect claim, and the rest of the paragraph which no >>> longer offers useful information. >>> >>> As a replacement, expand on the "throttling and driving animations" >>> characteristic. The main purpose is to let clients animate at the >>> display refresh rate, while avoiding drawing frames that will never be >>> presented. >>> >>> The new claim is that the server should give some time between >>> triggering frame callbacks and repainting itself, for clients to draw >>> and commit. This is somewhat intimate with the repaint scheduling >>> algorithm a compositor uses, but hopefully the right intention. >>> >>> >>> Another p
Re: [PATCH wayland v2] protocol: try to clarify frame callback semantics
On Feb 23, 2014 1:45 AM, "Pekka Paalanen" wrote: > > Hi, > > thanks for all the comments, it's encouraging to see a concensus > emerging. One reply below... > > On Sat, 22 Feb 2014 07:50:01 -0600 > Jason Ekstrand wrote: > > > On Feb 22, 2014 2:44 AM, "Axel Davy" wrote: > > > > > > Hi, > > > > > > I like very much the rewording proposed by Pekka. > > > > > > But I dislike your proposition to send frame callbacks right away if the > > attached buffer has been attached for a long time. > > > > > > Your argument seems to be that the client may manage to get to the next > > pageflip if the frame callback is called right away. But with this > > argument, I don't see why this behaviour would be only for buffers attached > > long ago (and then we refresh at a higher frequency than the screen refresh) > > > > > > Moreover we may say we can always get the two behaviours with client side > > code: > > > . If we keep the current behaviour, the client could know it has attached > > a buffer for a long time (and that the frame callback it had put, was > > already called), so if it wants to try to hit next pageflip, it could just > > commit right away with a new attach > > > > Yes, this is what they should be doing. The more I think about the frame > > callback, the more I think that clients should just put one with every new > > draw and just track whether that one has been released or not. > > > > Unfortunately, we have to do something reasonable in the case where the > > client requests a frame without drawing. I don't want to restrict the > > server too much on what it does in that case. It may, for instance, be > > running on some sort of refresh-on-demand hardware and have no concept of > > "next flip". When the client asks for a frame callback, it is effectively > > asking when is a good time to repaint. If now is a good time, the server > > should be allowed to say so. > > > > Pekka, one thing I forgot to mention that should probably be added is that > > we really should guarantee that frame callbacks get fired in the same order > > they are requested (per surface order, not global order). > > Ah, ok. That should be easy to implement by just being careful in > how the callback lists are managed. Can you elaborate on a use case > where this is especially useful? > > Recall that frame callbacks never get "discarded", except when the > surface is destroyed(?). If the update it was part of gets > overridden, the callbacks simply move on to the new update that > overwrote the old one, which means that several update's worth of > callbacks can trigger at the same time. I guess this is somehow > behind your proposition? Yes, it is. First off, we may want to use the language "same order as they are committed" instead of "same order as they are requested" to make all frame callbacks on the same commit the same. I looked at the code and it seems that Weston already does this, so it shouldn't break anything or take any extra work to implement. Really, it's a nice guarantee for the client that takes basically no work to implement in the server. If a client normally throttles on frame callbacks and then has to suddenly repaint or some reason, it may allow it to avoid the extra logic for detecting the old frame callback and throwing it out. I don't know how big of a deal that is because they're all timestamped anyway, but it's kind of nice. More importantly, it cleans up some of the edge-cases. Most of the time, it's going to be one frame call back per application component per attach. However, this may not always be the case. In particular, it makes the frame+commit case slightly better defined by providing the simple guarantee that the callback from the frame+commit will come after the callback from the last attach. Maybe that's not a good reason for the aditional server-side requirement, but it just seems to clean things up nicely. --Jason > > > Thanks, > pq > > > > > . With your proposition the client could always attach (and perhaps > > +damage) with a frame+commit (even with the old buffer not released), to be > > sure to get current behaviour. > > > > What do you mean by "current behavior"? And why would they want it? > > > > > > > > I don't think having to do an attach with the old buffer is a good idea, > > and I favor Pekka's proposition. > > > > I wasn't arguing against it. :-) > > > > > > > > Axel Davy > > > > > > On
Re: [PATCH wayland v2] protocol: try to clarify frame callback semantics
On Feb 23, 2014 1:50 AM, "Pekka Paalanen" wrote: > > On Fri, 21 Feb 2014 21:38:15 -0600 > Jason Ekstrand wrote: > > > Pekka, > > Sorry this e-mail took so long to send. Not much time lately. The first > > time or two I read this suggested re-wording I didn't like it, but now it's > > starting to grow on me. I still kind of like the idea of "the buffer you > > sent is now in use, go ahead and send the next one" but I don't know that > > it's that much better or that it actually changes anything. > > Hi, > > there is a slight difference. If the semantics were written as "the > update you sent is now applied" which is the same as the buffer > getting into use, just in different words, then that implies that > we should be queueing also frame callbacks when an update is queued. Yes, it does change that a bit. > I still do not know if it is better to queue frame callbacks or > not, but my current code does not queue them and it avoids the > corner case of what to do with the callbacks when a queued update > gets discarded. Let's go with that for now. We can always add frame callback queueing if someone decides it's useful, but we can never take it away once it hits libwayland. Also, it's not clear what interaction other extensions may have with the frame callback and we don't want to make it more complicated for them if we don't have to. --Jason Ekstrand > Thanks, > pq > > > The big thing I'd like to leave open (and I think your change does) is the > > following: Suppose a client commits a buffer and then, several seconds > > later (after the attached buffer was first used), the user does something > > that causes the client to refresh. If it does a frame+commit without an > > attach, the server should be able to respond immediately without waiting > > for another pageflip. This way the client may be able to render in time > > for the next flip. Sure, the client might be too slow and miss the flip, > > but that's really no worse than waiting before sending the frame callback. > > > > Point is, it should be a compositor decision and I think you made that > > clear enough. > > > > Looks good to me. > > --Jason Ekstrand > > > > Reviewed-by: Jason Ekstrand > > > > On Fri, Feb 21, 2014 at 7:46 AM, Pekka Paalanen wrote: > > > > > From: Pekka Paalanen > > > > > > "the callback event will arrive after the next output refresh" is wrong, > > > if you interpret "output refresh" as framebuffer flip or the moment when > > > the new pixels turn into light the first time. Weston has probably never > > > worked this way. > > > > > > Weston triggers the frame callbacks when it submits repainting commands > > > to the GPU, which is before the framebuffer flip. > > > > > > Strike the incorrect claim, and the rest of the paragraph which no > > > longer offers useful information. > > > > > > As a replacement, expand on the "throttling and driving animations" > > > characteristic. The main purpose is to let clients animate at the > > > display refresh rate, while avoiding drawing frames that will never be > > > presented. > > > > > > The new claim is that the server should give some time between > > > triggering frame callbacks and repainting itself, for clients to draw > > > and commit. This is somewhat intimate with the repaint scheduling > > > algorithm a compositor uses, but hopefully the right intention. > > > > > > > > Another point of this update is to imply, that frame callbacks should > > > not be used to count compositor repaint cycles nor monitor refresh > > > cycles. It has never been guaranteed to work. Removing the mention of > > > frame callback without an attach hopefully discourages such use. > > > > > > v2: don't just remove a paragraph, but add useful information about the > > > request's intent. > > > > > > Signed-off-by: Pekka Paalanen > > > Cc: Axel Davy > > > Cc: Jason Ekstrand > > > --- > > > protocol/wayland.xml | 26 ++ > > > 1 file changed, 18 insertions(+), 8 deletions(-) > > > > > > diff --git a/protocol/wayland.xml b/protocol/wayland.xml > > > index e1edbe5..6e370ad 100644 > > > --- a/protocol/wayland.xml > > > +++ b/protocol/wayland.xml > > > @@ -1059,22 +1059,32 @@ > > > > > > > > > > >
Re: [RFC v4] Fullscreen shell protocol
Jonas, thanks for reviewing. On Feb 24, 2014 4:03 PM, "Jonas Ådahl" wrote: > > On Thu, Feb 13, 2014 at 10:37:53PM -0600, Jason Ekstrand wrote: > > The following is yet another take on the fullscreen shell protocol. > > Previous versions more-or-less followed the approach taken in wl_shell. > > This version completely reworks the concept. In particular, the protocol > > is split into two use-cases. The first is that of a simple client that > > wants to present a surface or set of surfaces possibly with some scaling. > > This happens through the present_surface request which looks similar to > > that of wl_shell only without the modesetting. > > > > The second use-case is of a client that wants more control over the > > outputs. In this case, the client uses the present_surface_for_mode > > request to present the surface at a particular output mode. This request > > provides a more-or-less atomic modeset operation. If the compositor can > > satisfy the requested mode, then the mode is changed and the new surface is > > presented. Otherwise, the compositor harmlessly falls back to the > > previously presented surface and the client is informed that the switch > > failed. This way, the surface is either displayed correctly or not at all. > > Of course, a client is free to call present_surface_for_mode with the > > currently presented surface and hope for the best. However, this may > > result in strange behavior and there is no reliable fallback if the mode > > switch fails. > > > > In particular, I would like feedback on the modesetting portion of this > > protocol. This is particularly targetted at compositors that want to run > > inside weston or some other fullscreen compositor. In the next week or so, > > I will attempt to implement all this in weston and see how well it works. > > However, I would also like to know how well this will work for other > > compositors such as KWin or Hawaii. > > Hi Jason, > > Some follow up comments from last round inline. > > > > > Thanks for your feedback, > > --Jason Ekstrand > > > > = Protocol follows: = > > > > > > > > > > Displays a single surface per output. > > > > This interface provides a mechanism for a single client to display > > simple full-screen surfaces. While there technically may be multiple > > clients bound to this interface, only one of those clients should be > > shown at a time. > > > > To present a surface, the client uses either the present_surface or > > present_surface_for_mode requests. Presenting a surface takes effect > > on the next wl_surface.commit. See the individual requests for > > details about scaling and mode switches. > > > > The client can have at most one surface per output at any time. > > Requesting a surface be presented on an output that already has a > > surface replaces the previously presented surface. Presenting a null > > surface removes its content and effectively disables the output. > > Exactly what happens when an output is "disabled" is > > compositor-specific. The same surface may be presented multiple > > outputs simultaneously. > > > > > > > > > > Hints to indicate to the compositor how to deal with a conflict > > between the dimensions of the surface and the dimensions of the > > output. The compositor is free to ignore this parameter. > > > > > > > > > > > > > > > > > > > > > > Present a surface on the given output. > > > > If the output is null, the compositor will present the surface on > > whatever display (or displays) it thinks best. In particular, this > > may replace any or all surfaces currently presented so it should > > not be used in combination with placing surfaces on specific > > outputs. > > > > The method parameter is a hit to the compositor for how the surface > > is to be presented. In particular, it tells the compostior how to > > handle a size mismatch between the presented surface and the > > output. The compositor is free to ignore this parameter. > > > > The "zoom", "zoom_crop", and "stretch" methods imply a scaling > > operation on the surface. This will override any kind of output > > scaling, so th
[PATCH] protocol: Change wl_surface.damage to be in buffer coordinates.
When buffer_transform and buffer_scale were first introduced, added, we specified surface damage to be in surface coordinates. However, this does not and will never work properly with EGL. Because the buffer transform and scale are handled directly by the client and not passed through EGL, the EGL implementation cannot damage the buffer in surface coordinates. This is a problem both for regular eglSwapBuffers which damages the entire surface and for eglSwapBuffersWithDamage which only partially damages the surface. As far as I can see, there are three options for fixing this: 1) Add EGL extensions (most likely eglSurfaceAttrib values) to set the buffer scale and transform so that EGL is aware of them. 2) Change the protocol now before the problem spreads. 3) Compositors forever ignore damage on non-shm surfaces. I don't like 1) because we've already worked fairly hard to separate EGL from the rest of the protocol, and I don't think we want to even more EGL extensions. I don't like 3) because it makes damage tracking completely useless. Therefore, I propose 2). Lest people think that we actually have to break protocol here, we don't. Compositors will have to detect surface version 4 and treat damage accordingly. From a client perspective, it simply means that you can't use buffer_transform or buffer_scale with EGL unless you have a version 4 wl_surface. --- If this gets generally approved, I'll hack up some weston patches to implement it. It won't take too much code. Just need something to do inverse transforms of regions. protocol/wayland.xml | 16 +--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/protocol/wayland.xml b/protocol/wayland.xml index bf6acd1..873d0f9 100644 --- a/protocol/wayland.xml +++ b/protocol/wayland.xml @@ -176,7 +176,7 @@ - + A compositor. This object is a singleton global. The compositor is in charge of combining the contents of multiple @@ -962,7 +962,7 @@ - + A surface is a rectangular area that is displayed on the screen. It has a location, size and pixel contents. @@ -1041,7 +1041,9 @@ Damage is double-buffered state, see wl_surface.commit. - The damage rectangle is specified in surface local coordinates. + In versions 3 and previous, the damage rectangle was specified in + surface local coordinates. In versions 4 and above, the damage + rectangle is specified in buffer coordinates. The initial value for pending damage is empty: no damage. wl_surface.damage adds pending damage: the new pending damage @@ -1212,6 +1214,10 @@ Note that if the transform value includes 90 or 270 degree rotation, the width of the buffer will become the surface height and the height of the buffer will become the surface width. + + Note: It is recommended that you do not use buffer_transform with + EGL unless your wl_surface has version at least 4. Otherwise, EGL + will not report damage correctly. @@ -1236,6 +1242,10 @@ Note that if the scale is larger than 1, then you have to attach a buffer that is larger (by a factor of scale in each dimension) than the desired surface size. + + Note: It is recommended that you do not use buffer_scale with + EGL unless your wl_surface has version at least 4. Otherwise, EGL + will not report damage correctly. -- 1.8.5.3 ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH] protocol: Change wl_surface.damage to be in buffer coordinates.
And I forgot reply-all again. This is why I shouldn't respond from my phone. On Tue, Feb 25, 2014 at 8:07 AM, Pekka Paalanen wrote: > On Tue, 25 Feb 2014 07:44:24 -0600 > Jason Ekstrand wrote: > > > I'll do a full reply later, just a quick one now. > > On Feb 25, 2014 2:31 AM, "Pekka Paalanen" wrote: > > > > > > On Mon, 24 Feb 2014 22:32:05 -0600 > > > Jason Ekstrand wrote: > > > > > > > When buffer_transform and buffer_scale were first introduced, added, > we > > > > specified surface damage to be in surface coordinates. However, this > > does > > > > > > Don't forget wl_viewport. > > > > > > > not and will never work properly with EGL. Because the buffer > transform > > > > and scale are handled directly by the client and not passed through > EGL, > > > > the EGL implementation cannot damage the buffer in surface > coordinates. > > > > This is a problem both for regular eglSwapBuffers which damages the > > entire > > > > surface and for eglSwapBuffersWithDamage which only partially damages > > the > > > > surface. > > > > > > With this change, you will allow SwapBuffersWithDamage to report proper > > > damage, but the plain SwapBuffers is still forced to damage the whole. > I > > > guess that's the intention anyway, right? > eglSwapBuffersWithDamage should be blindly passing damage rectangles on to the compositor. It's then up to the client to use it correctly. See below. > > > > > > > As far as I can see, there are three options for fixing this: > > > > 1) Add EGL extensions (most likely eglSurfaceAttrib values) to set > the > > > > buffer scale and transform so that EGL is aware of them. > > > > 2) Change the protocol now before the problem spreads. > > > > 3) Compositors forever ignore damage on non-shm surfaces. > > > > > > > > I don't like 1) because we've already worked fairly hard to separate > EGL > > > > from the rest of the protocol, and I don't think we want to even more > > EGL > > > > extensions. I don't like 3) because it makes damage tracking > completely > > > > useless. Therefore, I propose 2). > > > > > > It's a nice idea... > > > > > > > Lest people think that we actually have to break protocol here, we > > don't. > > > > Compositors will have to detect surface version 4 and treat damage > > > > accordingly. From a client perspective, it simply means that you > can't > > use > > > > buffer_transform or buffer_scale with EGL unless you have a version 4 > > > > wl_surface. > > > > > > ...but this execution causes a subtle change in behaviour based on the > > > interface version in both compositors and clients. Especially clients > > > cannot simply just stay using damage in surface coordinates, unless > > > they also stick to version <4, but OTOH, gently forcing everyone to > > > migrate might be desireable? > Yes it does. And it means that clients will have to be careful. Or they can make their lives simpler by simply not using buffer_transform or buffer_scale unless they have at least version 4. IMHO, I always like damage in buffer coordinates better anyway. > > > > > > > --- > > > > > > > > If this gets generally approved, I'll hack up some weston patches to > > > > implement it. It won't take too much code. Just need something to > do > > > > inverse transforms of regions. > > > > > > Not only that, but saying damage is in buffer coordinates requires that > > > damage will be queueable state in the Presentation extension. It's not > > > a show-stopper, but now we would need to queue or not queue damage > > > based on the bound interface version. It sounds pretty complicated to > > > me. > > > > > > OTOH, queueable damage is something that was raised in the Presentation > > > discussions, but punted as "can add later if needed". It could be > > > useful. > Honestly, I don't see any problem with queuing automatically implying max damage for now. It makes the surface/buffer state split not as nice, but I don't know that it's a problem. If we do decide to queue it, clients won't partially damage out-of-order surfaces anyway because there's a huge race there. > > > > > > > > > > >
[PATCH weston v4 00/15] Add a fullscreen shell protocol
This patch series adds a fullscreen shell protocol as well as support in various clients and pieces of weston to fully demonstrate it. The idea is to make a shell for clients that run entirely fullscreen as the sole client visible to the user. I have four basic use-cases for this. 1) Simple full-screen clients. David Hermann has suggested such a protocol could be used for splash screens, terminal emulators, etc. While DRM/KMS isn't terribly hard to do, this allows simple clients to simply start weston and let it handle DRM/KMS and it simply provides buffers. I'm not sure how much this will actually save us, but it could be more convenient. These clients don't want much control, they basically want to throw surfaces on the screen. 2) As an input/output abstraction. While the "big boys" (GNOME-Shell, E, etc.) will be running on KMS directly, there may be benifit to supporting running on top of wl_fullscreen_shell. One of the things we're losing when we move away from X is a common input/output mechanism. Most of the time, we're running directly on the hardware and we can just use DRM/KMS. However, there are a variety of VNC/RDP servers and other input/output mechanisms that we may want to keep. Previously, if you simply implemented an X server or the appropreate bits to plug into Xorg, all the DE's would happly run on top of it. (GNOME being the major exception here because it 100% requires acceleration). My hope is to provide a mechanism for writing other backends for Wayland compositors by implementing a fullscreen shell compositor. 3) Other compositors. This kind of rolls into 2), but it's worth mention on it's own. Some compositors, such as KWin, would rather not mess with DRM/KMS themselves. Instead, they would rather spawn weston and make it do the actual modesetting. Unlike the simple clients, these clients want to basically do full modesetting. For these clients, we provide a slightly different interface that allows far more direct control over the mode of the output. This interface also allows more-or-less atomic modesetting. 4) Screen sharing and recording. The security discussion on how to allow clients to request screen sharing and things of the like is still going on and I have no intention of solving that question here. Most proposals for screen sharing and capture have been based on weston's screenshooter protocol which allows the screenshooter client to hand a surface to weston that weston then fills with the contents of the screen. While this works for screenshots, there are a lot of race conditions involved in doing it this way. People have also suggested screen sharing protocols. However, these usually involve re-writing most of the wayland protocol in the other (client -> server) direction. For this reason, I propose that, instead of having complicated screen sharing protocols, the screen sharing or recording program simply act as a wayland compositor supporting the fullscreen shell protocol. The primary compositor (the DE) can then spawn the sharing program and connect to it as a client. This way we can use the normal Wayland protocol for communication rather than re-writing it in reverse. To this end, I have written code to implement the fullsreen shell and to demonstrate it in all these capacities: 1) A wl_fullscreen_shell implementation in fullscreen-shell.c. 2) simple-shm now supports wl_fullscreen_shell as a "simple client" 3) Support is added to clients/fullscreen.c to exercise the fullscreen shell protocol. 4) wayland-backend.so now supports running in a wl_fullscreen_shell. It automatically detects and runs on all outputs and has full modesetting support. 5) A new screen-share.so plugin. This plugin registers the CTRL+ALT+s keybinding. When this key combination is typed, it spawns another copy of weston, this time running the RDP backend, and mirrors the current output (where the cursor is) to that RDP session. Seats from RDP are added as aditional seats. I hope you all like it! Feedback is appreciated. Thanks, --Jason Ekstrand Jason Ekstrand (15): Add a fullscreen shell protocol Generate/build the fullscreen shell protocol files Add a signal for when a seat updates its capabilities Add a wl_fullscreen_shell implementation simple-shm: Add fullscreen shell support toytoolkit: Only require xdg_shell if the window is not custom toytoolkit:Expose output make and model Add wl_fullscreen_shell support to weston-fullscreen compositor-wayland: Add support for running on top of wl_fullscreen_shell compositor-wayland: Add a --sprawl option Automatically select the wayland backend if WAYLAND_SOCKET is set Properly handle running inside a compositor that d
[PATCH weston v4 02/15] Generate/build the fullscreen shell protocol files
Signed-off-by: Jason Ekstrand --- Makefile.am | 1 + 1 file changed, 1 insertion(+) diff --git a/Makefile.am b/Makefile.am index dc50da3..0e749b0 100644 --- a/Makefile.am +++ b/Makefile.am @@ -928,6 +928,7 @@ EXTRA_DIST += \ protocol/text-cursor-position.xml \ protocol/wayland-test.xml \ protocol/xdg-shell.xml \ + protocol/fullscreen-shell.xml \ protocol/scaler.xml man_MANS = weston.1 weston.ini.5 -- 1.8.5.3 ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH weston v4 07/15] toytoolkit:Expose output make and model
Signed-off-by: Jason Ekstrand
---
clients/window.c | 22 ++
clients/window.h | 6 ++
2 files changed, 28 insertions(+)
diff --git a/clients/window.c b/clients/window.c
index b4b9588..6e132d5 100644
--- a/clients/window.c
+++ b/clients/window.c
@@ -349,6 +349,8 @@ struct output {
struct wl_list link;
int transform;
int scale;
+ char *make;
+ char *model;
display_output_handler_t destroy_handler;
void *user_data;
@@ -4738,6 +4740,14 @@ display_handle_geometry(void *data,
output->allocation.x = x;
output->allocation.y = y;
output->transform = transform;
+
+ if (output->make)
+ free(output->make);
+ output->make = strdup(make);
+
+ if (output->model)
+ free(output->model);
+ output->model = strdup(model);
}
static void
@@ -4925,6 +4935,18 @@ output_get_scale(struct output *output)
return output->scale;
}
+const char *
+output_get_make(struct output *output)
+{
+ return output->make;
+}
+
+const char *
+output_get_model(struct output *output)
+{
+ return output->model;
+}
+
static void
fini_xkb(struct input *input)
{
diff --git a/clients/window.h b/clients/window.h
index 7ec3537..d00c907 100644
--- a/clients/window.h
+++ b/clients/window.h
@@ -599,6 +599,12 @@ output_get_transform(struct output *output);
uint32_t
output_get_scale(struct output *output);
+const char *
+output_get_make(struct output *output);
+
+const char *
+output_get_model(struct output *output);
+
void
keysym_modifiers_add(struct wl_array *modifiers_map,
const char *name);
--
1.8.5.3
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH weston v4 04/15] Add a wl_fullscreen_shell implementation
Signed-off-by: Jason Ekstrand
---
Makefile.am| 20 ++
configure.ac | 8 +
src/fullscreen-shell.c | 822 +
3 files changed, 850 insertions(+)
create mode 100644 src/fullscreen-shell.c
diff --git a/Makefile.am b/Makefile.am
index 0e749b0..67aece2 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -675,6 +675,26 @@ nodist_desktop_shell_la_SOURCES = \
BUILT_SOURCES += $(nodist_desktop_shell_la_SOURCES)
endif
+if ENABLE_FULLSCREEN_SHELL
+
+module_LTLIBRARIES += fullscreen-shell.la
+
+fullscreen_shell_la_CPPFLAGS = \
+ -I$(top_builddir)/protocol \
+ -I$(top_srcdir)/shared \
+ -I$(top_srcdir)/src \
+ -I$(top_builddir)/src \
+ -DIN_WESTON
+
+fullscreen_shell_la_LDFLAGS = -module -avoid-version
+fullscreen_shell_la_LIBADD = $(COMPOSITOR_LIBS)
+fullscreen_shell_la_CFLAGS = $(GCC_CFLAGS) $(COMPOSITOR_CFLAGS)
+fullscreen_shell_la_SOURCES = \
+ src/fullscreen-shell.c
+nodist_fullscreen_shell_la_SOURCES = \
+ protocol/fullscreen-shell-protocol.c\
+ protocol/fullscreen-shell-server-protocol.h
+endif
if ENABLE_XWAYLAND
diff --git a/configure.ac b/configure.ac
index 00d7123..0809614 100644
--- a/configure.ac
+++ b/configure.ac
@@ -363,6 +363,13 @@ AM_CONDITIONAL(BUILD_SUBSURFACES_CLIENT,
AM_CONDITIONAL(ENABLE_DESKTOP_SHELL, true)
+AC_ARG_ENABLE(fullscreen-shell,
+ AS_HELP_STRING([--disable-fullscreen-shell],
+ [do not build fullscreen-shell server plugin]),,
+ enable_fullscreen_shell=yes)
+AM_CONDITIONAL(ENABLE_FULLSCREEN_SHELL,
+ test "x$enable_fullscreen_shell" = "xyes")
+
# CMS modules
AC_ARG_ENABLE(colord,
AS_HELP_STRING([--disable-colord],
@@ -494,6 +501,7 @@ AC_MSG_RESULT([
dbus${enable_dbus}
Build wcap utility ${enable_wcap_tools}
+ Build Fullscreen Shell ${enable_fullscreen_shell}
weston-launch utility ${enable_weston_launch}
systemd-login support ${have_systemd_login}
diff --git a/src/fullscreen-shell.c b/src/fullscreen-shell.c
new file mode 100644
index 000..939b857
--- /dev/null
+++ b/src/fullscreen-shell.c
@@ -0,0 +1,822 @@
+/*
+ * Copyright © 2013 Jason Ekstrand
+ *
+ * Permission to use, copy, modify, distribute, and sell this software and
+ * its documentation for any purpose is hereby granted without fee, provided
+ * that the above copyright notice appear in all copies and that both that
+ * copyright notice and this permission notice appear in supporting
+ * documentation, and that the name of the copyright holders not be used in
+ * advertising or publicity pertaining to distribution of the software
+ * without specific, written prior permission. The copyright holders make
+ * no representations about the suitability of this software for any
+ * purpose. It is provided "as is" without express or implied warranty.
+ *
+ * THE COPYRIGHT HOLDERS DISCLAIM ALL WARRANTIES WITH REGARD TO THIS
+ * SOFTWARE, INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY AND
+ * FITNESS, IN NO EVENT SHALL THE COPYRIGHT HOLDERS BE LIABLE FOR ANY
+ * SPECIAL, INDIRECT OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER
+ * RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF
+ * CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN
+ * CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
+ */
+
+#include "config.h"
+
+#include
+#include
+#include
+#include
+#include
+#include
+
+#include "compositor.h"
+#include "fullscreen-shell-server-protocol.h"
+
+struct fullscreen_shell {
+ struct wl_client *client;
+ struct wl_listener client_destroyed;
+ struct weston_compositor *compositor;
+
+ struct weston_layer layer;
+ struct wl_list output_list;
+ struct wl_listener output_created_listener;
+
+ struct wl_listener seat_created_listener;
+};
+
+struct fs_output {
+ struct fullscreen_shell *shell;
+ struct wl_list link;
+
+ struct weston_output *output;
+ struct wl_listener output_destroyed;
+
+ struct {
+ struct weston_surface *surface;
+ struct wl_listener surface_destroyed;
+ struct wl_resource *mode_feedback;
+
+ int presented_for_mode;
+ enum wl_fullscreen_shell_present_method method;
+ int32_t framerate;
+ } pending;
+
+ struct weston_surface *surface;
+ struct wl_listener surface_destroyed;
+ struct weston_view *view;
+ struct weston_view *black_view;
+ struct weston_transform transform; /* matrix from x, y */
+
+ int presented_for_mode;
+ enum wl_fullsc
[PATCH weston v4 03/15] Add a signal for when a seat updates its capabilities
Signed-off-by: Jason Ekstrand
---
src/compositor.h | 1 +
src/input.c | 2 ++
2 files changed, 3 insertions(+)
diff --git a/src/compositor.h b/src/compositor.h
index 63e4e2c..7edf7b3 100644
--- a/src/compositor.h
+++ b/src/compositor.h
@@ -494,6 +494,7 @@ struct weston_seat {
struct weston_output *output; /* constraint */
struct wl_signal destroy_signal;
+ struct wl_signal updated_caps_signal;
struct weston_compositor *compositor;
struct wl_list link;
diff --git a/src/input.c b/src/input.c
index e20c870..6c89f11 100644
--- a/src/input.c
+++ b/src/input.c
@@ -602,6 +602,7 @@ seat_send_updated_caps(struct weston_seat *seat)
wl_resource_for_each(resource, &seat->base_resource_list) {
wl_seat_send_capabilities(resource, caps);
}
+ wl_signal_emit(&seat->updated_caps_signal, seat);
}
WL_EXPORT void
@@ -2197,6 +2198,7 @@ weston_seat_init(struct weston_seat *seat, struct
weston_compositor *ec,
wl_signal_init(&seat->selection_signal);
wl_list_init(&seat->drag_resource_list);
wl_signal_init(&seat->destroy_signal);
+ wl_signal_init(&seat->updated_caps_signal);
seat->global = wl_global_create(ec->wl_display, &wl_seat_interface, 3,
seat, bind_seat);
--
1.8.5.3
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH weston v4 08/15] Add wl_fullscreen_shell support to weston-fullscreen
Signed-off-by: Jason Ekstrand
---
Makefile.am | 3 +
clients/fullscreen.c | 165 +++
2 files changed, 156 insertions(+), 12 deletions(-)
diff --git a/Makefile.am b/Makefile.am
index d94f0d0..c1ab74b 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -509,6 +509,9 @@ weston_transformed_LDADD = libtoytoolkit.la
weston_transformed_CFLAGS = $(AM_CFLAGS) $(CLIENT_CFLAGS)
weston_fullscreen_SOURCES = clients/fullscreen.c
+nodist_weston_fullscreen_SOURCES = \
+ protocol/fullscreen-shell-protocol.c\
+ protocol/fullscreen-shell-client-protocol.h
weston_fullscreen_LDADD = libtoytoolkit.la
weston_fullscreen_CFLAGS = $(AM_CFLAGS) $(CLIENT_CFLAGS)
diff --git a/clients/fullscreen.c b/clients/fullscreen.c
index 46a7926..920bc10 100644
--- a/clients/fullscreen.c
+++ b/clients/fullscreen.c
@@ -32,14 +32,25 @@
#include
#include
#include "window.h"
+#include "fullscreen-shell-client-protocol.h"
+
+struct fs_output {
+ struct wl_list link;
+ struct output *output;
+};
struct fullscreen {
struct display *display;
struct window *window;
struct widget *widget;
+ struct wl_fullscreen_shell *fshell;
+ enum wl_fullscreen_shell_present_method present_method;
int width, height;
int fullscreen;
float pointer_x, pointer_y;
+
+ struct wl_list output_list;
+ struct fs_output *current_output;
};
static void
@@ -113,6 +124,7 @@ redraw_handler(struct widget *widget, void *data)
cairo_t *cr;
int i;
double x, y, border;
+ const char *method_name[] = { "default", "center", "zoom", "zoom_crop",
"stretch"};
surface = window_get_surface(fullscreen->window);
if (surface == NULL ||
@@ -138,17 +150,34 @@ redraw_handler(struct widget *widget, void *data)
allocation.y + 25);
cairo_set_source_rgb(cr, 1, 1, 1);
- draw_string(cr,
- "Surface size: %d, %d\n"
- "Scale: %d, transform: %d\n"
- "Pointer: %f,%f\n"
- "Fullscreen: %d\n"
- "Keys: (s)cale, (t)ransform, si(z)e, (f)ullscreen,
(q)uit\n",
- fullscreen->width, fullscreen->height,
- window_get_buffer_scale (fullscreen->window),
- window_get_buffer_transform (fullscreen->window),
- fullscreen->pointer_x, fullscreen->pointer_y,
- fullscreen->fullscreen);
+ if (fullscreen->fshell) {
+ draw_string(cr,
+ "Surface size: %d, %d\n"
+ "Scale: %d, transform: %d\n"
+ "Pointer: %f,%f\n"
+ "Present method: %s\n"
+ "Output: %s\n"
+ "Keys: (s)cale, (t)ransform, si(z)e, (m)ethod,\n"
+ " (o)utput, modes(w)itch, (q)uit\n",
+ fullscreen->width, fullscreen->height,
+ window_get_buffer_scale (fullscreen->window),
+ window_get_buffer_transform (fullscreen->window),
+ fullscreen->pointer_x, fullscreen->pointer_y,
+ method_name[fullscreen->present_method],
+ fullscreen->current_output ?
output_get_model(fullscreen->current_output->output): "null");
+ } else {
+ draw_string(cr,
+ "Surface size: %d, %d\n"
+ "Scale: %d, transform: %d\n"
+ "Pointer: %f,%f\n"
+ "Fullscreen: %d\n"
+ "Keys: (s)cale, (t)ransform, si(z)e, (f)ullscreen,
(q)uit\n",
+ fullscreen->width, fullscreen->height,
+ window_get_buffer_scale (fullscreen->window),
+ window_get_buffer_transform (fullscreen->window),
+ fullscreen->pointer_x, fullscreen->pointer_y,
+ fullscreen->fullscreen);
+ }
y = 100;
i = 0;
@@ -188,6 +217,8 @@ key_handler(struct window *window, struct input *input,
uint32_t time,
struct fullscreen *fullscreen = data;
int transform, scale;
static int current_size = 0;
+ struct fs_output *fsout;
+ struct wl_output *wl_output;
int widths[] = { 640, 320, 800, 400 };
int heights[] = { 480, 240, 600, 300 };
@@ -220,7 +251,70 @@ key_handler(struct window
[PATCH weston v4 14/15] gl-renderer: Fix read_pixels in the case where we have output borders
Signed-off-by: Jason Ekstrand
---
src/gl-renderer.c | 4
1 file changed, 4 insertions(+)
diff --git a/src/gl-renderer.c b/src/gl-renderer.c
index d03bce6..444aa97 100644
--- a/src/gl-renderer.c
+++ b/src/gl-renderer.c
@@ -962,6 +962,10 @@ gl_renderer_read_pixels(struct weston_output *output,
uint32_t width, uint32_t height)
{
GLenum gl_format;
+ struct gl_output_state *go = get_output_state(output);
+
+ x += go->borders[GL_RENDERER_BORDER_LEFT].width;
+ y += go->borders[GL_RENDERER_BORDER_BOTTOM].height;
switch (format) {
case PIXMAN_a8r8g8b8:
--
1.8.5.3
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH weston v4 01/15] Add a fullscreen shell protocol
Signed-off-by: Jason Ekstrand --- protocol/fullscreen-shell.xml | 158 ++ 1 file changed, 158 insertions(+) create mode 100644 protocol/fullscreen-shell.xml diff --git a/protocol/fullscreen-shell.xml b/protocol/fullscreen-shell.xml new file mode 100644 index 000..13bdfcf --- /dev/null +++ b/protocol/fullscreen-shell.xml @@ -0,0 +1,158 @@ + + + + Displays a single surface per output. + + This interface provides a mechanism for a single client to display + simple full-screen surfaces. While there technically may be multiple + clients bound to this interface, only one of those clients should be + shown at a time. + + To present a surface, the client uses either the present_surface or + present_surface_for_mode requests. Presenting a surface takes effect + on the next wl_surface.commit. See the individual requests for + details about scaling and mode switches. + + The client can have at most one surface per output at any time. + Requesting a surface be presented on an output that already has a + surface replaces the previously presented surface. Presenting a null + surface removes its content and effectively disables the output. + Exactly what happens when an output is "disabled" is + compositor-specific. The same surface may be presented multiple + outputs simultaneously. + + Once a surface is presented on an output, it stays on that output + until either the client removes it or the compositor destroys the + output. This way, the client can update the output's contents by + simply attaching a new buffer. + + + + + Release the binding from the wl_fullscreen_shell interface + + This destroys the server-side object and frees this binding. If + the client binds to wl_fullscreen_shell multiple times, it may wish + to free some of those bindings. + + + + + + Hints to indicate to the compositor how to deal with a conflict + between the dimensions of the surface and the dimensions of the + output. The compositor is free to ignore this parameter. + + + + + + + + + + + Present a surface on the given output. + + If the output is null, the compositor will present the surface on + whatever display (or displays) it thinks best. In particular, this + may replace any or all surfaces currently presented so it should + not be used in combination with placing surfaces on specific + outputs. + + The method parameter is a hint to the compositor for how the surface + is to be presented. In particular, it tells the compostior how to + handle a size mismatch between the presented surface and the + output. The compositor is free to ignore this parameter. + + The "zoom", "zoom_crop", and "stretch" methods imply a scaling + operation on the surface. This will override any kind of output + scaling, so the buffer_scale property of the surface is effectively + ignored. + + + + + + + + + Presents a surface on the given output for a particular mode. + + If the current size of the output differs from that of the surface, + the compositor will attempt to change the size of the output to + match the surface. The result of the mode-swith operation will be + returned via the provided wl_fullscreen_shell_mode_feedback object. + + If the current output mode matches the one requested or if the + compositor successfully switches the mode to match the surface, + then the mode_successfull event will be sent and the output will + contain the contents of the given surface. If the compositor + cannot match the output size to the surface size, the mode_failed + will be sent and the output will contain the contents of the + previously presented surface (if any). If another surface is + presented on the given output before either of these has a chance + to happen, the present_canceled event will be sent. + + If the size of the presented surface changes, the resulting output + is undefined. The compositor may attempt to change the output mode + to compensate. However, there is no guarantee that a suitable mode + will be found and the client has no way to be notified of success + or failure. + + The framerate parameter specifies the target framerate for the + output. The compositor is free to ignore this parameter. A value + of 0 indicates that the client has no preference. + + If the surface has a buffer_scale greater than 1, the compositor + may choose a mode that matches either the buffer size or the + surface size. In
[PATCH weston v4 12/15] Properly handle running inside a compositor that does not provide keymaps
Signed-off-by: Jason Ekstrand
---
src/compositor-wayland.c | 61 +---
1 file changed, 37 insertions(+), 24 deletions(-)
diff --git a/src/compositor-wayland.c b/src/compositor-wayland.c
index fc9c9b5..ff02649 100644
--- a/src/compositor-wayland.c
+++ b/src/compositor-wayland.c
@@ -167,6 +167,7 @@ struct wayland_input {
} cursor;
} parent;
+ enum weston_key_state_update keyboard_state_update;
uint32_t key_serial;
uint32_t enter_serial;
int focus;
@@ -1414,40 +1415,52 @@ input_handle_keymap(void *data, struct wl_keyboard
*keyboard, uint32_t format,
struct xkb_keymap *keymap;
char *map_str;
- if (!data) {
- close(fd);
- return;
- }
+ if (!data)
+ goto error;
- if (format != WL_KEYBOARD_KEYMAP_FORMAT_XKB_V1) {
- close(fd);
- return;
- }
+ if (format == WL_KEYBOARD_KEYMAP_FORMAT_XKB_V1) {
+ map_str = mmap(NULL, size, PROT_READ, MAP_SHARED, fd, 0);
+ if (map_str == MAP_FAILED) {
+ weston_log("mmap failed: %m\n");
+ goto error;
+ }
- map_str = mmap(NULL, size, PROT_READ, MAP_SHARED, fd, 0);
- if (map_str == MAP_FAILED) {
- close(fd);
- return;
- }
+ keymap =
xkb_map_new_from_string(input->compositor->base.xkb_context,
+map_str,
+XKB_KEYMAP_FORMAT_TEXT_V1,
+0);
+ munmap(map_str, size);
- keymap = xkb_map_new_from_string(input->compositor->base.xkb_context,
-map_str,
-XKB_KEYMAP_FORMAT_TEXT_V1,
-0);
- munmap(map_str, size);
- close(fd);
+ if (!keymap) {
+ weston_log("failed to compile keymap\n");
+ goto error;
+ }
- if (!keymap) {
- weston_log("failed to compile keymap\n");
- return;
+ input->keyboard_state_update = STATE_UPDATE_NONE;
+ } else if (format == WL_KEYBOARD_KEYMAP_FORMAT_NO_KEYMAP) {
+ weston_log("No keymap provided; falling back to defalt\n");
+ keymap = NULL;
+ input->keyboard_state_update = STATE_UPDATE_AUTOMATIC;
+ } else {
+ weston_log("Invalid keymap\n");
+ goto error;
}
+ close(fd);
+
if (input->base.keyboard)
weston_seat_update_keymap(&input->base, keymap);
else
weston_seat_init_keyboard(&input->base, keymap);
- xkb_map_unref(keymap);
+ if (keymap)
+ xkb_map_unref(keymap);
+
+ return;
+
+error:
+ wl_keyboard_release(input->parent.keyboard);
+ close(fd);
}
static void
@@ -1522,7 +1535,7 @@ input_handle_key(void *data, struct wl_keyboard *keyboard,
notify_key(&input->base, time, key,
state ? WL_KEYBOARD_KEY_STATE_PRESSED :
WL_KEYBOARD_KEY_STATE_RELEASED,
- STATE_UPDATE_NONE);
+ input->keyboard_state_update);
}
static void
--
1.8.5.3
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH weston v4 06/15] toytoolkit: Only require xdg_shell if the window is not custom
Signed-off-by: Jason Ekstrand --- clients/window.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/clients/window.c b/clients/window.c index 91c1ea0..b4b9588 100644 --- a/clients/window.c +++ b/clients/window.c @@ -4423,7 +4423,7 @@ window_create_internal(struct display *display, int custom) surface = surface_create(window); window->main_surface = surface; - fail_on_null(display->xdg_shell); + assert(custom || display->xdg_shell); window->custom = custom; window->preferred_format = WINDOW_PREFERRED_FORMAT_NONE; -- 1.8.5.3 ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH weston v4 10/15] compositor-wayland: Add a --sprawl option
This forces weston to create one output for every parent output. This is
enabled by default if it detects a wl_fullscreen_shell. The --sprawl
option is primarily to enable this on wl_shell.
Signed-off-by: Jason Ekstrand
---
src/compositor-wayland.c | 10 +++---
src/compositor.c | 1 +
2 files changed, 8 insertions(+), 3 deletions(-)
diff --git a/src/compositor-wayland.c b/src/compositor-wayland.c
index 1e97a4f..fc9c9b5 100644
--- a/src/compositor-wayland.c
+++ b/src/compositor-wayland.c
@@ -64,6 +64,7 @@ struct wayland_compositor {
} parent;
int use_pixman;
+ int sprawl_across_outputs;
struct theme *theme;
cairo_device_t *frame_device;
@@ -1710,7 +1711,7 @@ wayland_compositor_register_output(struct
wayland_compositor *c, uint32_t id)
wl_list_init(&output->mode_list);
wl_list_insert(&c->parent.output_list, &output->link);
- if (c->parent.fshell) {
+ if (c->sprawl_across_outputs) {
wl_display_roundtrip(c->parent.wl_display);
wayland_output_create_for_parent_output(c, output);
}
@@ -1991,7 +1992,7 @@ backend_init(struct wl_display *display, int *argc, char
*argv[],
struct wayland_output *output;
struct wayland_parent_output *poutput;
struct weston_config_section *section;
- int x, count, width, height, scale, use_pixman, fullscreen;
+ int x, count, width, height, scale, use_pixman, fullscreen, sprawl;
const char *section_name, *display_name;
char *name;
@@ -2003,6 +2004,7 @@ backend_init(struct wl_display *display, int *argc, char
*argv[],
{ WESTON_OPTION_BOOLEAN, "use-pixman", 0, &use_pixman },
{ WESTON_OPTION_INTEGER, "output-count", 0, &count },
{ WESTON_OPTION_BOOLEAN, "fullscreen", 0, &fullscreen },
+ { WESTON_OPTION_BOOLEAN, "sprawl", 0, &sprawl },
};
width = 0;
@@ -2012,6 +2014,7 @@ backend_init(struct wl_display *display, int *argc, char
*argv[],
use_pixman = 0;
count = 1;
fullscreen = 0;
+ sprawl = 0;
parse_options(wayland_options,
ARRAY_LENGTH(wayland_options), argc, argv);
@@ -2020,7 +2023,8 @@ backend_init(struct wl_display *display, int *argc, char
*argv[],
if (!c)
return NULL;
- if (c->parent.fshell) {
+ if (sprawl || c->parent.fshell) {
+ c->sprawl_across_outputs = 1;
wl_display_roundtrip(c->parent.wl_display);
wl_list_for_each(poutput, &c->parent.output_list, link)
diff --git a/src/compositor.c b/src/compositor.c
index 9de3a90..f04bcf9 100644
--- a/src/compositor.c
+++ b/src/compositor.c
@@ -3955,6 +3955,7 @@ usage(int error_code)
" --fullscreen\t\tRun in fullscreen mode\n"
" --use-pixman\t\tUse the pixman (CPU) renderer\n"
" --output-count=COUNT\tCreate multiple outputs\n"
+ " --sprawl\t\tCreate one fullscreen output for every parent
output\n"
" --display=DISPLAY\tWayland display to connect to\n\n");
#if defined(BUILD_RPI_COMPOSITOR) && defined(HAVE_BCM_HOST)
--
1.8.5.3
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH weston v4 15/15] Add a screen sharing plugin
This adds a plugin called screen-share.so. If the screen-share.so module
is imported, it will add the CTRL+ALT+s keybinding to start a screen
sharing session. If you press CTRL+ALT+S, weston will spawn another copy
of weston, this time with the RDP backend, and mirrors the current screen
to it and adds any seats from RDP as aditional seats. The current screen
is defined as the one with the mouse pointer. Currently the CTRL+ALT+s
keybinding is hardcoded as the only way to activate screen sharing. If, at
some point, shells want more control over the screen sharing process, the
API's should be easy to update and export to make this possible.
For security, the command and path to weston is currently hard-coded. It
would not take much aditional code to make this configurable or to allow a
shell to launch other screen-sharing programs. However, handling those
security issues is outside the scope of this patch so it is hard-coded for
now.
Signed-off-by: Jason Ekstrand
---
Makefile.am| 22 ++
configure.ac | 13 +
src/screen-share.c | 1082
3 files changed, 1117 insertions(+)
create mode 100644 src/screen-share.c
diff --git a/Makefile.am b/Makefile.am
index 838a051..f0fbec1 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -707,6 +707,28 @@ nodist_fullscreen_shell_la_SOURCES =
\
protocol/fullscreen-shell-server-protocol.h
endif
+if ENABLE_SCREEN_SHARING
+
+module_LTLIBRARIES += screen-share.la
+
+screen_share_la_CPPFLAGS = $(AM_CPPFLAGS) -DBINDIR='"$(bindir)"'
+screen_share_la_LDFLAGS = -module -avoid-version
+screen_share_la_LIBADD = \
+ $(COMPOSITOR_LIBS) \
+ $(SCREEN_SHARE_LIBS)\
+ libshared-cairo.la
+screen_share_la_CFLAGS = \
+ $(COMPOSITOR_CFLAGS)\
+ $(SCREEN_SHARE_CFLAGS) \
+ $(GCC_CFLAGS)
+screen_share_la_SOURCES = \
+ src/screen-share.c
+nodist_screen_share_la_SOURCES = \
+ protocol/fullscreen-shell-protocol.c\
+ protocol/fullscreen-shell-client-protocol.h
+
+endif
+
if ENABLE_XWAYLAND
module_LTLIBRARIES += xwayland.la
diff --git a/configure.ac b/configure.ac
index 0809614..27fd536 100644
--- a/configure.ac
+++ b/configure.ac
@@ -212,6 +212,18 @@ if test x$enable_rdp_compositor = xyes; then
CPPFLAGS="$SAVED_CPPFLAGS"
fi
+AC_ARG_ENABLE([screen-sharing], [--enable-screen-sharing],,
+ enable_screen_sharing=no)
+AM_CONDITIONAL([ENABLE_SCREEN_SHARING],
+ [test x$enable_screen_sharing = xyes])
+if test x$enable_screen_sharing = xyes; then
+ PKG_CHECK_MODULES(SCREEN_SHARE, [wayland-client])
+
+ if test x$enable_rdp_compositor != xyes; then
+AC_MSG_WARN([The screen-share.so module requires the RDP backend to work
properly.])
+ fi
+fi
+
AC_ARG_WITH(cairo,
AS_HELP_STRING([--with-cairo=@<:@image|gl|glesv2@:>@]
[Which Cairo renderer to use for the clients]),
@@ -513,6 +525,7 @@ AC_MSG_RESULT([
RPI Compositor ${enable_rpi_compositor}
FBDEV Compositor${enable_fbdev_compositor}
RDP Compositor ${enable_rdp_compositor}
+ Screen Sharing ${enable_screen_sharing}
Raspberry Pi BCM headers${have_bcm_host}
diff --git a/src/screen-share.c b/src/screen-share.c
new file mode 100644
index 000..6cc6bd5
--- /dev/null
+++ b/src/screen-share.c
@@ -0,0 +1,1082 @@
+/*
+ * Copyright © 2008-2011 Kristian Høgsberg
+ * Copyright © 2014 Jason Ekstrand
+ *
+ * Permission to use, copy, modify, distribute, and sell this software and
+ * its documentation for any purpose is hereby granted without fee, provided
+ * that the above copyright notice appear in all copies and that both that
+ * copyright notice and this permission notice appear in supporting
+ * documentation, and that the name of the copyright holders not be used in
+ * advertising or publicity pertaining to distribution of the software
+ * without specific, written prior permission. The copyright holders make
+ * no representations about the suitability of this software for any
+ * purpose. It is provided "as is" without express or implied warranty.
+ *
+ * THE COPYRIGHT HOLDERS DISCLAIM ALL WARRANTIES WITH REGARD TO THIS
+ * SOFTWARE, INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY AND
+ * FITNESS, IN NO EVENT SHALL THE COPYRIGHT HOLDERS BE LIABLE FOR ANY
+ * SPECIAL, INDIRECT OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER
+ * RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF
+ * CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN
+ * CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
+ */
+
+#include
+#include
+#include
+#include
+#
[PATCH weston v4 09/15] compositor-wayland: Add support for running on top of wl_fullscreen_shell
Signed-off-by: Jason Ekstrand
---
Makefile.am | 3 +
src/compositor-wayland.c | 470 ---
2 files changed, 451 insertions(+), 22 deletions(-)
diff --git a/Makefile.am b/Makefile.am
index c1ab74b..838a051 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -226,6 +226,9 @@ wayland_backend_la_CFLAGS = \
$(WAYLAND_COMPOSITOR_CFLAGS)\
$(GCC_CFLAGS)
wayland_backend_la_SOURCES = src/compositor-wayland.c
+nodist_wayland_backend_la_SOURCES =\
+ protocol/fullscreen-shell-protocol.c\
+ protocol/fullscreen-shell-client-protocol.h
endif
if ENABLE_RPI_COMPOSITOR
diff --git a/src/compositor-wayland.c b/src/compositor-wayland.c
index 899c329..1e97a4f 100644
--- a/src/compositor-wayland.c
+++ b/src/compositor-wayland.c
@@ -42,6 +42,7 @@
#include "../shared/image-loader.h"
#include "../shared/os-compatibility.h"
#include "../shared/cairo-util.h"
+#include "fullscreen-shell-client-protocol.h"
#define WINDOW_TITLE "Weston Compositor"
@@ -53,8 +54,11 @@ struct wayland_compositor {
struct wl_registry *registry;
struct wl_compositor *compositor;
struct wl_shell *shell;
+ struct wl_fullscreen_shell *fshell;
struct wl_shm *shm;
+ struct wl_list output_list;
+
struct wl_event_source *wl_source;
uint32_t event_mask;
} parent;
@@ -75,6 +79,10 @@ struct wayland_output {
struct {
int draw_initial_frame;
struct wl_surface *surface;
+
+ struct wl_output *output;
+ uint32_t global_id;
+
struct wl_shell_surface *shell_surface;
int configure_width, configure_height;
} parent;
@@ -103,6 +111,29 @@ struct wayland_output {
uint32_t scale;
};
+struct wayland_parent_output {
+ struct wayland_output *output;
+ struct wl_list link;
+
+ struct wl_output *global;
+ uint32_t id;
+
+ struct {
+ char *make;
+ char *model;
+ int32_t width, height;
+ uint32_t subpixel;
+ } physical;
+
+ int32_t x, y;
+ uint32_t transform;
+ uint32_t scale;
+
+ struct wl_list mode_list;
+ struct weston_mode *preferred_mode;
+ struct weston_mode *current_mode;
+};
+
struct wayland_shm_buffer {
struct wayland_output *output;
struct wl_list link;
@@ -289,10 +320,9 @@ draw_initial_frame(struct wayland_output *output)
sb->output = NULL;
wl_surface_attach(output->parent.surface, sb->buffer, 0, 0);
-
- /* We only need to damage some part, as its only transparant
-* pixels anyway. */
- wl_surface_damage(output->parent.surface, 0, 0, 1, 1);
+ wl_surface_damage(output->parent.surface, 0, 0,
+ output->base.current_mode->width,
+ output->base.current_mode->height);
}
static void
@@ -553,7 +583,8 @@ wayland_output_destroy(struct weston_output *output_base)
wl_egl_window_destroy(output->gl.egl_window);
wl_surface_destroy(output->parent.surface);
- wl_shell_surface_destroy(output->parent.shell_surface);
+ if (output->parent.shell_surface)
+ wl_shell_surface_destroy(output->parent.shell_surface);
if (output->frame)
frame_destroy(output->frame);
@@ -736,6 +767,9 @@ wayland_output_set_fullscreen(struct wayland_output *output,
enum wl_shell_surface_fullscreen_method method,
uint32_t framerate, struct wl_output *target)
{
+ struct wayland_compositor *c =
+ (struct wayland_compositor *)output->base.compositor;
+
if (output->frame) {
frame_destroy(output->frame);
output->frame = NULL;
@@ -743,8 +777,177 @@ wayland_output_set_fullscreen(struct wayland_output
*output,
wayland_output_resize_surface(output);
- wl_shell_surface_set_fullscreen(output->parent.shell_surface,
- method, framerate, target);
+ if (output->parent.shell_surface) {
+ wl_shell_surface_set_fullscreen(output->parent.shell_surface,
+ method, framerate, target);
+ } else if (c->parent.fshell) {
+ wl_fullscreen_shell_present_surface(c->parent.fshell,
+ output->parent.surface,
+ method, target);
+ }
+}
+
+static struct weston_mode *
+wayland_output_choose_mode(struct wayland_output *output,
+ struct we
