[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: [Mesa-dev] [PATCH 0/6] dma-buf: Add an API for exporting sync files (v12)
uf 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, I'm just not finding any time > > anywhere to hack on these patches - too many other fires :-( > > Well I'm here. Let's just agree on the direction and I can do the coding. > > What I need help with is all the auditing
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 patch to document DMA_BUF_IOCTL_SYNC > > - Generally better docs > &
[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: [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/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/gpu/drm/panfrost/panfrost_job.c | 2 +- drivers/g
[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/gpu/drm/vmwgfx/vmwgfx_bo.c| 2 +- include
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
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(, 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, ); > + if (ret) > + goto err_put_fd; > + } else if (arg.flags & DMA_BUF_SYNC_READ) { > + fence = dma_resv_get_excl_rcu(dmabuf->resv); > + } > + > + if (!fence) > + fence = dma_fenc
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: [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 the > number of jobs that the fence is waiting on. > > It can a
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 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: 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 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 and dma_fence data structures. A dma_fence is a tiny object &
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 which does the rendering and the driver will sort it > > &
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
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 to end up being a synchronization point as an > >
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, video encode, etc.) and know which things to submit in what order. For the last few ye
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 Sync extension has these trigger funcs which get called when the fence is signalled. I'm not sure how to handle that with syncobj without a thr
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, we've had sync_file in the kernel and it's > plumbed into some drivers.
Plumbing explicit synchronization through the Linux ecosystem
smatch 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: [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
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: [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
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: [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: [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: [RFC wayland 00/18] Wayland network transparency :)
On Tue, Feb 9, 2016 at 8:55 AM, Derek Foremanwrote: > 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] Track protocol object versions inside wl_proxy.
On Dec 7, 2015 7:20 AM, "Pekka Paalanen" <ppaala...@gmail.com> wrote: > > On Wed, 25 Nov 2015 10:25:42 -0800 > Jason Ekstrand <ja...@jlekstrand.net> wrote: > > > On Wed, Nov 25, 2015 at 10:15 AM, Derek Foreman <der...@osg.samsung.com> 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: [PATCH wayland v4] protocol: Add wl_surface.damage_buffer
On Nov 27, 2015 1:35 AM, "Pekka Paalanen" <ppaala...@gmail.com> wrote: > > On Fri, 27 Nov 2015 16:47:19 +0800 > Jonas Ådahl <jad...@gmail.com> 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 <der...@osg.samsung.com> > > > > Reviewed-by: Jonas Ådahl <jad...@gmail.com>, with a couple of comments > > below. > > Reviewed-by: Pekka Paalanen <pekka.paala...@collabora.co.uk> > > ... > > > > --- > > > > > > 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
Re: [PATCH wayland] client: Use a 0 version to mean indeterminate proxy version
On Nov 26, 2015 1:23 PM, "Derek Foreman" <der...@osg.samsung.com> 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 <der...@osg.samsung.com> Reviewed-by: Jason Ekstrand <ja...@jlekstrand.net> > --- > > 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 = >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" <der...@osg.samsung.com> 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 <der...@osg.samsung.com> This looks fine by me Reviewed-by: Jason Ekstrand <ja...@jlekstrand.net> > --- > > 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: [RFC wayland] Track protocol object versions inside wl_proxy.
On Wed, Nov 25, 2015 at 10:15 AM, Derek Foreman <der...@osg.samsung.com> 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". 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 <der...@osg.samsung.com>: >>>> From: Jason Ekstrand <ja...@jlekstrand.net> >>>> >>>> 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 *mes
Re: [RFC wayland] Track protocol object versions inside wl_proxy.
On Nov 25, 2015 12:03 PM, "Derek Foreman" <der...@osg.samsung.com> 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 > > 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: [RFC wayland] Track protocol object versions inside wl_proxy.
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 > 2015-11-12 22:01 GMT+02:00 Derek Foreman <der...@osg.samsung.com>: >> From: Jason Ekstrand <ja...@jlekstrand.net> >> >> 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, >> +
Re: [RFC wayland] protocol: Add wl_surface.buffer_damage
On Nov 13, 2015 10:43 AM, "Derek Foreman" <der...@osg.samsung.com> wrote: > > On 13/11/15 02:03 AM, Pekka Paalanen wrote: > > On Thu, 12 Nov 2015 13:48:10 -0600 > > Derek Foreman <der...@osg.samsung.com> 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 <der...@osg.samsung.com> 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] 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 <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 <der...@osg.samsung.com> 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 <der...@osg.samsung.com> >>> --- >>> >>> 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 specifi
Re: [PATCH wayland v2 1/3] introduce new headers wayland-client/server-core.h
On Apr 29, 2015 3:30 AM, Pekka Paalanen ppaala...@gmail.com wrote: On Wed, 29 Apr 2015 12:50:48 +0300 Giulio Camuffo giuliocamu...@gmail.com wrote: 2015-04-29 12:13 GMT+03:00 Pekka Paalanen ppaala...@gmail.com: On Tue, 28 Apr 2015 22:57:16 +0300 Giulio Camuffo giuliocamu...@gmail.com wrote: wayland-client.h and wayland-server.h include the protocol headers generated at build time. This means that a libwayland user cannot generate and use protocol code created from a wayland.xml newer than the installed libwayand, because it is not possible to only include the API header. This commit adds wayland-client-core.h and wayland-server-core.h which do not include the protocol headers and any deprecated code. --- v2: move the deprecated api back to wayland-server.h Makefile.am | 2 + src/wayland-client-core.h | 180 + src/wayland-client.h | 155 +- src/wayland-server-core.h | 405 ++ src/wayland-server.h | 377 +- 5 files changed, 592 insertions(+), 527 deletions(-) create mode 100644 src/wayland-client-core.h create mode 100644 src/wayland-server-core.h diff --git a/Makefile.am b/Makefile.am index 0fccf86..a8a0a56 100644 --- a/Makefile.am +++ b/Makefile.am @@ -21,7 +21,9 @@ noinst_LTLIBRARIES = libwayland-util.la include_HEADERS =\ src/wayland-util.h \ src/wayland-server.h\ + src/wayland-server-core.h \ src/wayland-client.h\ + src/wayland-client-core.h \ src/wayland-egl.h \ src/wayland-version.h Hi, you also have to fix doc/doxygen/Makefile.am, otherwise we miss all the documentation that has been moved into the -core.h files. That fixed, this is Reviewed-by: Pekka Paalanen pekka.paala...@collabora.co.uk I do find the wayland-client/server-core.h in the title a bit confusing, to me it reads as server-core.h in directory wayland-client. introduce wayland-client-core.h and wayland-server-core.h? Would be also nice to mention the language bindings use case in the commit message, I think that is a more important case than updating wayland.xml. After all, wayland.xml is installed with libwayland, so they should always be in sync. Well, my use case is exactly to use a wayland.xml different than the one installed ;). I'll mention that, though. Do we have any project already using the new headers and the new --include-core-only option? We should have that before 1.8 final is released, so we have also tested the new way. No, I hacked up some tests but nothing upstreamable. I'll see to make weston use them. It does not have to be Weston, as long as there is something to test with. So no rush. It also occurred to me that it would be nice to install Wayland headers into a wayland/ sub-directory, but that's another story. And I suppose we don't want to actually push people away from the old headers if they work fine, right? Maybe not push people but i don't think saying that using the new ones is preferred would be a bad idea actually. In exchange for having to include one more header you get a nice logical separation between library and protocol api and you do not ned to change a lot of code if in the future you need to separate them. Right. I was pondering between adding nothing, adding a comment you know this is old, right? in the old headers, and adding a #warn in the old headers. I don't know that it much matters. In any case, this all looks good to me. I haven't looked that hard at the generator changes but the header split is Reviewed-by: Jason Ekstrand ja...@jlekstrand.net Maybe a comment to the effect of what you said would be nice? Thanks, pq ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH wayland] introduce new headers wayland-client/server-core.h
Pekka, Thanks for adding me to the CC. I have one general comment that I would have made inline with the code had it lasted this far. Please take this opportunity to remove the deprecated stuff by *not* putting anything deprecated into wayland-*-core.h. We have to keep it to keep old apps building but if someone wants the fancy new headers, they should take the fancy new API with it. On Tue, Apr 28, 2015 at 2:48 AM, Pekka Paalanen ppaala...@gmail.com wrote: On Tue, 28 Apr 2015 09:40:39 +0300 Giulio Camuffo giuliocamu...@gmail.com wrote: 2015-04-28 9:29 GMT+03:00 Marek Chalupa mchqwe...@gmail.com: On Mon, Apr 27, 2015 at 4:06 PM, Giulio Camuffo giuliocamu...@gmail.com wrote: 2015-04-27 21:53 GMT+03:00 Bill Spitzak spit...@gmail.com: On 04/26/2015 11:48 PM, Marek Chalupa wrote: Is the --include-core-headers option necessary? Until now the scanner included wayland-client/server.h which included the %-protocol.h, but since there are inclusion guards, the protocol header was not included again - so including wayland-client/server.h is the same as including wayland-client/server-core.h, isn't it? So including the core headers should be good for all cases. The problem is when you have an extension.xml, and generate extension-client-protocol.h. Then you have some preexisting code including that and relying on it including wayland-client.h and then wayland-client-protocol.h, and if you use something from that it will not build anymore, because wayland-client-core.h doesn't include it. True... So when you want to include only core file and not whole wayland-client/server.h in generated headers? When you are writing new code or porting some old one, and you want to use a wayland.xml from a different version than the libwayland. I'm going to talk about client headers, but it all applies equally to server headers too, so I won't bother writing client/server all the time. Yes, I agree. There could be old code around, that relies on my-client-protocol.h to pull in wayland-client.h which pulls in wayland-client-protocol.h. I do not see a good reason to break that, even though it is a minor issue. What my-client-protocol.h depends on are all found in wayland-client-core.h. The fact that wayland-client.h pulls in wayland-client-protocol.h is mostly a coincidence, but as said above, I think we want to make it opt-in to remove that implication. So the --include-core-headers option does have it's place. perhaps --include-core-only? To me --include-core-headers makes it look like you're adding something when, instead, you're actually taking something away. What this patch allows is to generate headers that will not pull in wayland-client-protocol.h. This is useful not only for using a newer wayland.xml but also for language bindings who should not be wrapping the generated entrypoints from wayland-client-protocol.h at all. Those would never want to pull in wayland-client-protocol.h, nor would they be using wayland-scanner either. I imagine this would be very useful for e.g. C++ bindings. Yes, this all seems sane to me. Also, now we get a more clear separation between what is libwayland ABI and what is just generated helpers. This is explained in http://wayland.freedesktop.org/docs/html/ch04.html#sect-Protocol-Code-Generation and now we can clearly say that everything in wayland-client-core.h is library ABI. Previously it wasn't so clear, because wayland-client.h includes wayland-client-protocol.h. Yeah, better separation there is good. Giulio also mentioned in IRC, that wayland-egl.h needs similar treatment, we cannot simply replace the #include wayland-client.h with wayland-client-core.h there. I'm not sure why wayland-egl.h includes anything really, but since it does, it makes sense to keep the same stability. However, for wayland-egl.h, what if the core version of it did not include anything? It doesn't need to, it just needs forward declarations of wl_surface and... that's it. I would like to see this patch split into three: 1. the header stuff not including wayland-egl.h changes 2. wayland-scanner changes 3. wayland-egl.h changes 2 depends on 1, but 1 would be useful also on its own. Drop all includes from wayland-egl-core.h, and also rebased to master. Seems reasonable. Assuming splitting as per Pekka's comments and my comment above about getting rid of deprecated stuff, Reviewed-by: Jason Ekstrand ja...@jlekstrand.net Even if there is a reason to not include the core headers (I don't see it, however) it certainly would be the default to include them. Therefore the switch should at least be reversed so no switch means to include them. Bill, when you do not specify --include-core-headers, the generated headers will include the old wayland-client.h or wayland-server.h, rather than nothing. The generated headers will always include their needed dependencies. The question
Re: egl streams, trying to gain some knowledge
On Thu, Apr 2, 2015 at 12:35 AM, Dave Airlie airl...@gmail.com wrote: So nvidia have indicated they would like to use an EGLstreams based solution to enable wayland on their binary driver stack at some point. As per their recent XDC talk, they sounded like they had given up on eglstreams for implementing the Wayland protocol and were planning on doing Wayland for real inside their driver. However, it did sound like they were planning on using streams for their I can't believe it's not GBM device abstraction. Which are you referring to here? They each come with different issues. I'm just trying to gauge how people in mesa/wayland feel about this as a solution, is it a solution looking for a problem, when you have EGLstreams everything looks like a nail type situation etc For implementing Wayland, I think it's a terrible plan. It would probably work in the same sense that android's NativeWindow abstraction works; it doesn't. I've got a rather lengthy post on how one seemingly subtle design choice makes it impossible to implement Wayland correctly on top of android's abstractions: http://www.jlekstrand.net/jason/projects/wayland/wayland-android/ I haven't seen that particular issue with EGLStreams yet, but that doesn't mean there isn't one and until I see an implementation, I'm going to be skeptical. I could also go on a rant about drivers doing IPC and how it takes fewer lines of code to implement Wayland in a driver than there are lines of EGLStream spec, but I'll leave that to someone else. :-) For interfacing with KMS, I think it's similarly bad. It would work for the basics of put image on screen but not for the more complex use-cases. For example, a Wayland compositor wants to be able to grab frames of a client and flip them independantly as planes. How does this work with a stream? Do you get a stream from a wl_surface and then just switch to the driver handling everything and the compositor ignoring it and then switch back? Do you create a stream from a single wl_buffer? Do you have some way that you can take buffers and put them on a stream? It seems very ill-defined. I think (and this is me speculating) is that the reason nvidia wants to use streams is because their binary driver is *very* userspace-heavy. On the other hand, most of the FOSS drivers have substantial kernel components that handle buffer allocation and arbitrate between clients. For a primarily userspace driver, streams makes a lot of sense because it's all hidden in the driver and they can put it wherever they want. For a heavily userspace driver, userspace modesetting in X also makes a lot of sense... Also if anyone has any idea if any other EGL vendors are heading down this road, or if this is a one-company extension, ratified to KHR because nobody objected. I can't speak to that one. --Jason ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [Mesa-dev] GL_TEXTURE_2D to wl_buffer
On Sat, Mar 28, 2015 at 6:57 AM, x414e54 x414...@linux.com wrote: I am attempting to pass a GL_TEXTURE_2D directly to a Wayland compositor by first converting it so an EGLImageKHR and then to a wl_buffer. eglCreateImageKHR appears to work fine but when calling glCreateWaylandBufferFromImageWL I get EGL_BAD_MATCH unsupported image format. The GL_TEXTURE_2D is in GL_RGBA format. Is this going to be possible or should I look for an alternative way? I wouldn't expect this to work at all in general. The glCreateWaylandBufferFromImageWL extension was initially created for nested compositor. The intention was to take buffers recieved by a nested compositor from Wayland client and then hand them directly off to the parent compositor. In other words, the buffers were something that already came from a Wayland client and so handing them off across the Wayland protocol was always safe. This was never intended for passing around arbitrary EGLImages. Why not? There may be all sorts of tiling, stride, and color format restrictions required by the compositor half of the graphics stack that may not be required for a texture. For instance, if you want to be able to scan out from the buffer directly, the restrictions are usually fairly heavy. How can you do this? One way would be to use GBM to allocate your buffer and then import it into egl to render and then talk the wl_drm protocol directly. However, this is far more complicated than you probably want and, while it will work with mesa, it is not a general solution. If you gave a more high-level description of what you are trying to do, I may be able to help better. It's quite possible that there is a fairly simple way to do it. I was originally blitting the texture to the default framebuffer and then trying to use eglSwapBuffers. But for some reason eglSwapBuffers was returning EGL_BAD_SURFACE even though eglMakeCurrent had no errors. Can you render to it? That sounds like something is wrong in the way you're setting up your EGLSurface. Of you can render you should be able to blit. --Jason ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [RFC PATCH v2 wayland] protocol: add wl_pointer.axis_source, axis_stop and axis_discrete events
On Tue, Mar 24, 2015 at 5:27 PM, Peter Hutterer peter.hutte...@who-t.net wrote: The axis_source event determines how an axis event was generated. This enables clients to judge when to use kinetic scrolling. The axis_stop event notifies a client about the termination of a scroll sequence, likewise needed to calculate kinetic scrolling parameters. The axis_discrete event provides the wheel click count. Previously the axis value was some hardcoded number (10), with the discrete steps this enables a client to differ between line-based scrolling on a mouse wheel and smooth scrolling with a touchpad. We can't extend the existing wl_pointer.axis events so we introduce a new concept: latching events. These events (axis_source and axis_discrete) are prefixed before a wl_pointer.axis or axis_stop event, i.e. the sequence becomes: wl_pointer.axis_source wl_pointer.axis wl_pointer.axis_source wl_pointer.axis wl_pointer.axis_source wl_pointer.axis wl_pointer.axis_source wl_pointer.axis_stop or: wl_pointer.axis_source wl_pointer.axis_discrete wl_pointer.axis_axis Clients need to combine the state of all events where needed. --- Changes to v1: - introduce axis_stop and axis_discrete as separate events instead of flags - couple of documentation updates - wl_seat/keyboard/touch/pointer bumped to version 5 This is for the API review only so far, I don't have the weston patches to match yet. protocol/wayland.xml | 87 +--- 1 file changed, 83 insertions(+), 4 deletions(-) diff --git a/protocol/wayland.xml b/protocol/wayland.xml index c5963e8..29e6f01 100644 --- a/protocol/wayland.xml +++ b/protocol/wayland.xml @@ -1337,7 +1337,7 @@ wl_seat.get_keyboard or wl_seat.get_touch, the returned object is always of the same version as the wl_seat. -- - interface name=wl_seat version=4 + interface name=wl_seat version=5 description summary=group of input devices A seat is a group of keyboards, pointer and touch devices. This object is published as a global during start up, or when such a @@ -1411,7 +1411,7 @@ /interface !-- for the version see the comment in wl_seat -- - interface name=wl_pointer version=3 + interface name=wl_pointer version=5 description summary=pointer input device The wl_pointer interface represents one or more input devices, such as mice, which control the pointer location and pointer_focus @@ -1572,10 +1572,89 @@ description summary=release the pointer object/ /request +!-- Version 4 additions: none -- + +!-- Version 5 additions -- + +enum name=axis_source + description summary=axis source types + Describes the axis types of scroll events. + /description + entry name=unknown value=0/ + entry name=wheel value=1/ + entry name=finger value=2/ + entry name=continuous value=3/ Please document what these are. finger is fairly obvious. But what's the difference between continuous and wheel? I have a mouse with a continuous wheel. Coincidentally, it also has a button you can click that makes the wheel rotate in discrete steps. Other than that, this looks fine to me. I can't say much about whether or not this is a good way to implement kinetic scrolling, but the protocol looks fine. --Jason +/enum + +event name=axis_source since=5 + description summary=axis source event +Scroll and other axis source notification. + +This event does not occur on its own. It is sent before a +wl_pointer.axis or wl_pointer.axis_stop event and carries the source +information for that event. A client is expected to accumulate the +data in both events before proceeding. + +The axis and timestamp values are identical to the one in the +subsequent wl_pointer.axis or wl_pointer.axis_stop event. For the +interpretation of the axis value, see the wl_pointer.axis event +documentation. + +The source specifies how this event was generated. If the source is +finger, a wl_pointer.axis_stop event will be sent when the user +lifts the finger off the device. + /description + arg name=time type=uint summary=timestamp with millisecond granularity/ + arg name=axis type=uint/ + arg name=axis_source type=uint/ +/event + +event name=axis_stop since=5 + description summary=axis stop event +Scroll and other axis stop notification. + +For some wl_pointer.axis_source type, a wl_pointer.axis_stop event +is sent to notify a client that the axis sequence has terminated. +This enables the client to implement kinetic scrolling. +See the wl_pointer.axis_source documentation for information on when +this event may be generated. + +Any wl_pointer.axis events after this event
Re: [RFC PATCH v2 wayland] protocol: add wl_pointer.axis_source, axis_stop and axis_discrete events
On Mar 24, 2015 6:14 PM, Jonas Ådahl jad...@gmail.com wrote: On Wed, Mar 25, 2015 at 10:27:10AM +1000, Peter Hutterer wrote: The axis_source event determines how an axis event was generated. This enables clients to judge when to use kinetic scrolling. The axis_stop event notifies a client about the termination of a scroll sequence, likewise needed to calculate kinetic scrolling parameters. The axis_discrete event provides the wheel click count. Previously the axis value was some hardcoded number (10), with the discrete steps this enables a client to differ between line-based scrolling on a mouse wheel and smooth scrolling with a touchpad. We can't extend the existing wl_pointer.axis events so we introduce a new concept: latching events. These events (axis_source and axis_discrete) are prefixed before a wl_pointer.axis or axis_stop event, i.e. the sequence becomes: wl_pointer.axis_source wl_pointer.axis wl_pointer.axis_source wl_pointer.axis wl_pointer.axis_source wl_pointer.axis wl_pointer.axis_source wl_pointer.axis_stop or: wl_pointer.axis_source wl_pointer.axis_discrete wl_pointer.axis_axis Clients need to combine the state of all events where needed. --- Changes to v1: - introduce axis_stop and axis_discrete as separate events instead of flags - couple of documentation updates - wl_seat/keyboard/touch/pointer bumped to version 5 This is for the API review only so far, I don't have the weston patches to match yet. protocol/wayland.xml | 87 +--- 1 file changed, 83 insertions(+), 4 deletions(-) diff --git a/protocol/wayland.xml b/protocol/wayland.xml index c5963e8..29e6f01 100644 --- a/protocol/wayland.xml +++ b/protocol/wayland.xml @@ -1337,7 +1337,7 @@ wl_seat.get_keyboard or wl_seat.get_touch, the returned object is always of the same version as the wl_seat. -- - interface name=wl_seat version=4 + interface name=wl_seat version=5 description summary=group of input devices A seat is a group of keyboards, pointer and touch devices. This object is published as a global during start up, or when such a @@ -1411,7 +1411,7 @@ /interface !-- for the version see the comment in wl_seat -- - interface name=wl_pointer version=3 + interface name=wl_pointer version=5 description summary=pointer input device The wl_pointer interface represents one or more input devices, such as mice, which control the pointer location and pointer_focus @@ -1572,10 +1572,89 @@ description summary=release the pointer object/ /request +!-- Version 4 additions: none -- Usually don't tend to add these it seems. If we'd want to be consistent, we'd have to add comments like this in several other places. + +!-- Version 5 additions -- + +enum name=axis_source + description summary=axis source types + Describes the axis types of scroll events. + /description + entry name=unknown value=0/ + entry name=wheel value=1/ + entry name=finger value=2/ + entry name=continuous value=3/ +/enum + +event name=axis_source since=5 + description summary=axis source event +Scroll and other axis source notification. + +This event does not occur on its own. It is sent before a +wl_pointer.axis or wl_pointer.axis_stop event and carries the source +information for that event. A client is expected to accumulate the +data in both events before proceeding. + +The axis and timestamp values are identical to the one in the +subsequent wl_pointer.axis or wl_pointer.axis_stop event. For the +interpretation of the axis value, see the wl_pointer.axis event +documentation. + +The source specifies how this event was generated. If the source is +finger, a wl_pointer.axis_stop event will be sent when the user +lifts the finger off the device. + /description + arg name=time type=uint summary=timestamp with millisecond granularity/ Why have a time parameter here, considering that this state does nothing on its own and which data should simply be just kept around until the committing event is received? Feels redundant. Definitely. a) it's redundant and b) it creates confusion where the user wonders what they should do with all those timestamps. + arg name=axis type=uint/ This one seems also redundant. I agree. You could make some case about sending all the source events then the discrete events and then axis events. However I see no advantage to this. If we want these events to act as optional arguments to the primary axis event then they should just all come in a clump. + arg name=axis_source type=uint/ +/event + +event name=axis_stop
Re: [RFC PATCH wayland] protocol: add wl_pointer.axis_source events
On Sun, Mar 22, 2015 at 8:57 PM, Jonas Ådahl jad...@gmail.com wrote: On Mon, Mar 23, 2015 at 01:21:38PM +1000, Peter Hutterer wrote: On Mon, Mar 23, 2015 at 10:23:18AM +0800, Jonas Ådahl wrote: On Mon, Mar 09, 2015 at 01:28:04PM +1000, Peter Hutterer wrote: The axis source determines how an event was generated. That enables clients to judge when to use kinetic scrolling. Nice to see this happening! I have not looked at the implementation so far, only the protocol. I have some comments below. We can't extend the existing wl_pointer.axis events so instead this new event is prefixed before each wl_pointer.axis event, i.e. the sequence becomes: wl_pointer.axis_source wl_pointer.axis wl_pointer.axis_source wl_pointer.axis wl_pointer.axis_source wl_pointer.axis Clients need to combine the state of the two events where needed. Supported are a flag to notify the client when a sequence stops, which is the only time when no axis events follows the axis_source event. [Note: nothing in the protocol prevents us from sending 0/0 axis events, so we may not need this special case] The step_distance indicates how the value in the following event should be interpreted (i.e. divided) to get a discrete count of scroll events (i.e. wheel clicks. [Note: arguably it is more flexible to include the discrete count here rather than the step_distance. This would split the information across two events though] --- protocol/wayland.xml | 48 +++- 1 file changed, 47 insertions(+), 1 deletion(-) diff --git a/protocol/wayland.xml b/protocol/wayland.xml index 88bbbc0..880f90a 100644 --- a/protocol/wayland.xml +++ b/protocol/wayland.xml @@ -1402,7 +1402,7 @@ /interface - interface name=wl_pointer version=3 + interface name=wl_pointer version=4 Hmm. I think we should also increase the verison of wl_seat, wl_touch and wl_keyboard as part of this. Is there a requirement to do this whenever one of them changes? I honestly don't know, but I'd be surprised if we can't move them independently. wl_touch and wl_keyboard might not be necessary, but you need to at least bump the wl_seat version as well. Any later additions to wl_keyboard and wl_touch would then need to bump to wl_seat.version + 1 (i.e. increase by 1), which is why I thought it could be reasonable to bump all at once to avoid those future confusion, but it doesn't seem what has been done before. Since wl_seat is already at version 4 from a previous addition to wl_kebyoard, we actually need to bump both wl_pointer and wl_seat to 5. Yes, we need to do that. I have a few comments but I want to read through it again and think about it more before making them. Please keep me in the CC. --Jason The reason for the need to also bump the wl_seat version is that clients does not specify a version when creating the wl_pointer object, meaning the wl_pointer/wl_touch/wl_keyboard will always be the same version as the wl_seat they were created from. description summary=pointer input device The wl_pointer interface represents one or more input devices, such as mice, which control the pointer location and pointer_focus @@ -1563,6 +1563,52 @@ description summary=release the pointer object/ /request +!-- Version 4 additions -- + +enum name=axis_source + description summary=axis source types + Describes the axis types of scroll events. + /description + entry name=unknown value=0/ + entry name=wheel value=1/ + entry name=finger value=2/ + entry name=continuous value=3/ +/enum + +enum name=axis_source_flags + description summary=axis flags + /description + entry name=none value=0x0 summary=emty flags/ + entry name=stop_scroll value=0x1 +summary=indicates this is the final scroll event on this sequence/ +/enum + +event name=axis_source since=4 + description summary=axis source event +Scroll and other axis source notification. + +This event is sent before a wl_pointer.axis event and carries the +source information for the following axis event. Unless the +stop_scroll axis flag is set, this event is always followed by a +wl_pointer.axis event. + +The axis and timestamp values are identical to the one in the +wl_pointer.axis event. For the interpretation of the axis value, see +the wl_pointer.axis event documentation. + +The source specifies how this event was generated. + +The step_distance denotes how to interpret the wl_pointer.axis value +in terms of discrete steps (i.e. mouse wheel clicks). If zero, the
Re: [PATCH weston] man: update weston's shells
LGTM On Mar 20, 2015 6:59 AM, Pekka Paalanen ppaala...@gmail.com wrote: From: Pekka Paalanen pekka.paala...@collabora.co.uk Tablet shell is long gone. Might as well list what we have now. Signed-off-by: Pekka Paalanen pekka.paala...@collabora.co.uk --- man/weston.man | 27 +-- 1 file changed, 17 insertions(+), 10 deletions(-) diff --git a/man/weston.man b/man/weston.man index 735235f..d8d924e 100644 --- a/man/weston.man +++ b/man/weston.man @@ -46,6 +46,9 @@ Wayland shell, desktop, or applications. . .\ *** .SH SHELLS +Each of these shells have its own public protocol interface for clients. +This means that a client must be specifically written for a shell protocol, +otherwise it will not work. .TP Desktop shell Desktop shell is like a modern X desktop environment, concentrating @@ -57,16 +60,20 @@ and the special client .B weston-desktop-shell which provides the wallpaper, panel, and screen locking dialog. .TP -Tablet shell -Tablet shell is a graphical user interface aimed for tablet-like -devices, where usually the only input method is a touch screen. -It does not support freely floating windows or many other desktop -features, but intends to provide a natural interface on tablets. -Tablet shell consists of the shell plugin -.I tablet-shell.so -and the special client -.B weston-tablet-shell -which provides the basic user interface. +Fullscreen shell +Fullscreen shell is intended for a client that needs to take over +whole outputs, often all outputs. This is primarily intended for +running another compositor on Weston. The other compositor does not +need to handle any platform-specifics like DRM/KMS or evdev/libinput. +The shell consists only of the shell plugin +.IR fullscreen-shell.so . +.TP +IVI-shell +In-vehicle infotainment shell is a special purpose shell that exposes +a GENIVI Layer Manager compatible API to controller modules, and a very +simple shell protocol towards clients. IVI-shell starts with loading +.IR ivi-shell.so , +and then a controller module which may launch helper clients. . .\ *** .SH XWAYLAND -- 2.0.5 ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: Fbdev rotation broken
On Mar 17, 2015 5:34 AM, Pekka Paalanen ppaala...@gmail.com wrote: On Tue, 17 Mar 2015 13:02:34 +0100 Thilo Cestonaro th...@cestona.ro wrote: Hey! Just curious, why didn't you get the fbdev backend to work? Was it the color format thing? No, I got the color format to work, but I need to rotate the screen, and the rotation in fbdev doesn't work as expected. (I changed compositor-fbdev.c - weston_output_init() or so, to have the Transform which I wanted to try) Normal = 480x272 180° = didn't do anything here, panel still placed like in normal 90° = changed the image resolution to 272x480 but didn't rotate correctly, so half of the screen was invisible 270° = like 90° but the other side was visible But as fbdev doesn't support Transformation from weston.ini, I guess this code isn't thought to be work, right? Anyway I'm glad, drm with pixman backend is there! :) Hmm, I wonder if we broke the fbdev backend recently with the output matrix stuff... Fbdev-backend intends to do the rotation when blitting from shadow to front, but we already do the rotation when compositing to shadow. However, the matrix changes we did weren't supposed to change where each step is done, only the units, so I suppose this has been broken for a good while. Really? Yeah, it's probably been broken for a good long while then. Given that it seems there is no way to rotate an fbdev output other than editing code, no-one certainly has tested it, and we should just remove the remaining code that pretends to support output rotation in compositor-fbdev.c. That might even fix it, too. Seems reasonable to me. --Jason ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [wayland HiDPI support, posible regression?]
On Tue, Mar 10, 2015 at 12:00 PM, Bill Spitzak spit...@gmail.com wrote: On 03/09/2015 10:32 PM, Jasper St. Pierre wrote: Mouse input is reported in a 24.8 fixed-point format. Subpixel mouse locations are entirely possible. Yes events are doing this which is ok. There is a problem that clients must round to the correct pixel. If the rounding done by the client does not match the rounding used by the compositor to position the mouse cursor there may be annoying misalignment of the graphics. However this problem exists for high-precision mice irregardless of high dpi settings, so it is probably best to just document the rounding that must be used to position cursors: to convert a 24.8 mouse position to a pixel use (x*scale+127)8, the offset must be 127 (not 0 or 128) and you must use right shift, not divide by 256 (because that will shift negative values in the wrong direction). Events seem to be ok, but my complaint is that a large number of coordinates in the api other than events are in integer logical pixels, not in high dpi or in fixed-point. The offsets to attach are the biggest culprits. There are also integer clip rectangles in the subsurface and scaling apis. Except for compatibility there is no reason positions in messages cannot be in buffer pixels. Please do not take a thread started by someone who is obviously confused and side-track it into a discussion of things that you think are design-flaws in the current protocol. This is not the appropriate place for a discussion of wl_surface.attach (x, y) coordinate systems and bringing that up only adds to the confusion. --Jason ___ 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 v2 11/16] pixman-renderer: refactor into region_intersect_only_translation()
On Tue, Mar 10, 2015 at 6:01 AM, Pekka Paalanen ppaala...@gmail.com wrote: From: Pekka Paalanen pekka.paala...@collabora.co.uk Move code into a new helper function. No changes. Signed-off-by: Pekka Paalanen pekka.paala...@collabora.co.uk Reviewed-By: Derek Foreman der...@osg.samsung.com --- src/pixman-renderer.c | 31 ++- 1 file changed, 18 insertions(+), 13 deletions(-) diff --git a/src/pixman-renderer.c b/src/pixman-renderer.c index 8fd50e3..96123cb 100644 --- a/src/pixman-renderer.c +++ b/src/pixman-renderer.c @@ -181,6 +181,22 @@ pixman_renderer_compute_transform(pixman_transform_t *transform_out, } static void +region_intersect_only_translation(pixman_region32_t *result_global, + pixman_region32_t *global, + pixman_region32_t *surf, + struct weston_view *view) +{ + float view_x, view_y; We should have an assert in here that it really is only translation. Both for documentation and for sanity. + + /* Convert from surface to global coordinates */ + pixman_region32_copy(result_global, surf); + weston_view_to_global_float(view, 0, 0, view_x, view_y); + pixman_region32_translate(result_global, (int)view_x, (int)view_y); + + pixman_region32_intersect(result_global, result_global, global); +} + +static void repaint_region(struct weston_view *ev, struct weston_output *output, pixman_region32_t *region, pixman_region32_t *surf_region, pixman_op_t pixman_op) @@ -191,7 +207,6 @@ repaint_region(struct weston_view *ev, struct weston_output *output, struct pixman_output_state *po = get_output_state(output); struct weston_buffer_viewport *vp = ev-surface-buffer_viewport; pixman_region32_t final_region; - float view_x, view_y; pixman_transform_t transform; pixman_image_t *mask_image; pixman_color_t mask = { 0, }; @@ -203,18 +218,8 @@ repaint_region(struct weston_view *ev, struct weston_output *output, */ pixman_region32_init(final_region); if (surf_region) { - pixman_region32_copy(final_region, surf_region); - - /* Convert from surface to global coordinates */ - if (!ev-transform.enabled) { - pixman_region32_translate(final_region, ev-geometry.x, ev-geometry.y); - } else { - weston_view_to_global_float(ev, 0, 0, view_x, view_y); - pixman_region32_translate(final_region, (int)view_x, (int)view_y); - } - - /* We need to paint the intersection */ - pixman_region32_intersect(final_region, final_region, region); + region_intersect_only_translation(final_region, region, + surf_region, ev); } else { /* If there is no surface region, just use the global region */ pixman_region32_copy(final_region, region); -- 2.0.5 ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH weston v2 00/16] Fix pixman-renderer cropping
Hey pq, Glad to see this stuff finally look like landing ~1 year later. :-) My reviewing ability on this code has decreased substantially since I wrote it so I'm kind of trusting you to have tested it thuroughly and trusting myself to have known what I was doing back then. That said, Reviewed-by: Jason Ekstrand ja...@jlekstrand.net On Tue, Mar 10, 2015 at 7:48 AM, Derek Foreman der...@osg.samsung.com wrote: On 10/03/15 08:01 AM, Pekka Paalanen wrote: From: Pekka Paalanen pekka.paala...@collabora.co.uk Hi, as requested by Derek, here is a revised series from http://lists.freedesktop.org/archives/wayland-devel/2015-March/020440.html The major addition to the series is that I pulled selected patches from http://lists.freedesktop.org/archives/wayland-devel/2014-October/017801.html recently rebased by Derek in https://github.com/ManMower/weston/commits/transforms to the beginning of my series, so that pixman-renderer would have less code to move around. While doing that, I added the zoom cleanups to the very beginning, and fixed some fallout from the output matrix changes. Jason, for all patches where you are the author, I'd like to get your S-o-b and/or R-b. I did do some minor modifications, those are noted in the commit messages. Derek, I'd appreciate R-b's, and S-o-b for the patch you authored in this series. I have addressed your review comments from the last time, so some patches already have your R-b, but wouldn't hurt to have a quick look again. The series is also available in http://cgit.collabora.com/git/user/pq/weston.git/log/?h=pixmancomplex-3 Thanks, pq Derek Foreman (1): compositor: use weston_matrix_transform for weston_output_transform_coordinate S-O-B me. Jason Ekstrand (4): Use pixel coordinates for weston_output.matrix zoom: Use pixels instead of GL coordinates compositor: Add surface-to-buffer and buffer-to-surface matrices pixman-renderer: simplify the output-to-buffer matrix computation Since we're not allowing zoom at all, I guess we could continue to disallow zoom across this patch instead of rendering funny stuff. Personally, I'm not too concerned about cosmetic brokenness here since it's about to be fixed and didn't work previously. :) Pekka Paalanen (11): zoom: remove animation_xy as unused zoom: remove unused args from weston_zoom_transition rpi-renderer: minimal fix to zoom coordinates Patch looks good, but I have no rpi to test. All patches Reviewed-By: Derek Foreman der...@osg.samsung.com No further comments. :) compositor: add weston_surface_to_buffer_region() pixman-renderer: refactor transformation computation pixman-renderer: refactor into region_intersect_only_translation() pixman-renderer: add view_transformation_is_translation() pixman-renderer: change repaint_region() arguments pixman-renderer: move code to draw_view_translated() pixman-renderer: implement source clipping pixman-renderer: implement view scissor src/compositor.c | 292 ++- src/compositor.h | 18 +- src/gl-renderer.c | 14 +- src/pixman-renderer.c | 475 +- src/rpi-renderer.c| 10 +- src/zoom.c| 98 +++ 6 files changed, 493 insertions(+), 414 deletions(-) ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH weston v2 00/16] Fix pixman-renderer cropping
On Tue, Mar 10, 2015 at 12:51 PM, Bill Spitzak spit...@gmail.com wrote: This all looks really good to me. It is possible to avoid GL coordinates even in GL. Set the projection matrix to an ortho matrix: glMatrixMode(GL_PROJECTION); glLoadIdentity() glOrtho(0, width, height, 0, -1, 1); glTranslate(-width/2, -height/2, 0); glMatrixMode(GL_MODELVIEW); I remember in our code we did this by directly with glLoadMatrix. This avoided any error with how the center was calculated, and made the Z output to be zero. However I am unsure if that is portable to all devices. This would reduce the number of coordinate spaces you need to think about. Except weston's GL render is GLES2 which doesn't have a matrix stack... ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [RFC : xdg_surface_present() - take 2
Manuel, Thanks for keeping this going. I don't have a whole lot of comments at the moment as it's a pretty small protocol change. I would like to hear some commentary from Jasper and some of the other desktop devs though. The one comment I will make about the protocol is that you may want to change the documentation a bit. Instead of specifying that it must be an interactive gui notification that allows the user to bring up the window. It might be better to simply specify that the window has new content and would like the compositor to notify the user. On Tue, Mar 3, 2015 at 1:15 PM, Bill Spitzak spit...@gmail.com wrote: - Pekka Paalanen pointed out the request name was unclear and suggested to use xdg_surface_needs/wants_attention() instead. Jasper St. Pierre pointed out that _NET_WM_STATE_DEMANDS_ATTENTION already existed in X11 and does not do the same thing. We discussed that again yesterday and thought that present() fitted nicely ; What about raise? - Implementing focus stealing prevention between different clients may be easy : just count the delay between the client has been started and its shell surface actually gets mapped, and if it has been too long and the focus is elsewhere, show the surface without focusing it (but with a notification). The notion of the client has been started is vague, but at worst, we can use the time when the client did its initial connection to the compositor ; I suspect that initial connection is going to be far too late. It will be after all the dynamic linking and static object initialization and script interpretation (imagine if the wayland api is written in the interpreted language, it will not open the display until quite a lot of interpretation is done), all of which are the real reason modern apps take forever to start up. Yes, launching another client is quite possibly going to take a long time. Some clients are written in C and will start in less than a second. Other, heavier clients may take substantially longer. External processes will most likely time out. I would put the serial somewhere that the app can get it, perhaps in an environment variable, so it can send it with it's first raise request. I'm not entirely convinced that serials are the right way to do cross-client focus stealing prevention. However, I'm open to the idea. - Within a same client application, however, it is harder. Actually it is impossible without a serial. The serial is the correct solution which makes within the same client easier that between clients. This raises the question : how do we say We have no serial to pass, for the standard case ? We repeatedly suggested 0 (xdg_surface_present (surface, 0)) because serials are incrementing globals, so 0 to be issued would be very-very unlikely. Should we formalize that somewhere in the protocol ? '0' is useful but should force the the serial is too old behavior. Anybody who wants a real raise will have to get an actual serial. By that, do you mean that a serial of 0 should mean Don't raise just flash/bounce/whatever? If that's the case, then I think I would agree. It seems that there are three distinct use-cases here: 1) The client wants to gently alert the user (serial of 0). 2) The client wants to raise a window (real serial). 3) The client doesn't have a serial. I don't know what I think about special-casing 0. I don't see any problem with doing so at the moment. If we were particularly concerned about it, it would be easy enough to ensure that the serial 0 is never handed out. What I'm more concerned about is that there are two different cases in which we don't have a valid serial. If we're intending to have this be something that the client calls when it wants attention after it's first been opened, then this third case may be something we have to consider. For example, if you run a client on the command-line and give it a file to open. We also want to secure the request from garbage random serials ; the implementation behavior is that there is only one serial valid for a few seconds, if the surface has not been focused for some time, it will not be able to raise itself even if it random()ly finds the formerly good serial. It seems to me the compositor will have to remember very few serials. Rather than per-surface I think it would be per-seat. And there would be no timeout, instead it would remember the last N button or key presses (N quite possibley is 1). Yes, per-seat is most likely what we want. How many serials do we want to hold on to? I think that's kind of depends on how badly we want to pass serials across clients. Within a client, one is probably sufficient. If we're going to pass it between clients, we probably want to hold on to more and have a timeout. We probably also want to do something such as clearing all serials when the user raises a window. However, that falls under the category of heuristics, not protocol.
Re: [PATCH weston 10/10] xdg-shell: Bump unstable version
On Feb 27, 2015 5:11 AM, Pekka Paalanen ppaala...@gmail.com wrote: On Fri, 27 Feb 2015 13:38:49 +0200 Pekka Paalanen ppaala...@gmail.com wrote: On Fri, 13 Feb 2015 14:02:02 +0800 Jonas Ådahl jad...@gmail.com wrote: From: Jasper St. Pierre jstpie...@mecheye.net --- clients/simple-damage.c | 2 +- clients/simple-egl.c| 2 +- clients/simple-shm.c| 2 +- clients/window.c| 2 +- desktop-shell/shell.c | 2 +- protocol/xdg-shell.xml | 2 +- 6 files changed, 6 insertions(+), 6 deletions(-) .. Reviewed-by: Pekka Paalanen pekka.paala...@collabora.co.uk Jonas, would you like to push? I went ahead and pushed the whole series. 502f5e0..5ba1e1d master - master Woohoo! 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: [PATCH] fullscreen-shell: Fix modeset on transformed outputs
Hi Imran, Looking over things some more, it looks like the patch I sent is correct and so is the DRM backend. The problem is that weston's wayland backend isn't handling transformed outputs nicely. In particular, it doesn't swap the width and height of the surface. As far as I can tell, it does nothing whatsoever with the transform it receives. It would be good if someone fixed that. The real question is how best to handle it. I think the answer to that is that we want to just follow the parent compositor's lead and transform the window the same way it does. However, this gets tricky if the parent compositor says an output is transformed by 90 and the child wants it transformed by 180. Do you add the transformations? Just let the client override? Lots of interesting questions there. --Jason Ekstrand On Thu, Jan 8, 2015 at 1:37 PM, Imran Zaman imran.za...@gmail.com wrote: Hi Ekstrand! I have tested it in nested compositors case; unfortunately it is not enough and doesn't work :-) The reason is that drm compositor (choose_mode) function rejects the mode if width and height are swapped so similar patch is needed for choose_mode in drm compositor based on transform. I tested the setup as below.. - System compositor (drm backend) with weston.ini file containing transform=90 for the output. - Child compositor (wayland backend) - child compositor receives e.g. 1920x1080 + transform= 90 info. - it creates the surface and DOESN'T do any transformation (other than swapping width and height) as it is disabled. - then surface is presented to system compositor's fullscreen shell - fullscreen shell uses the outputs' data received from system compositor when module is initialized and surface data received from child compositor which may not be in sync.. - fullscreen shell then ask system compositor to switch mode - system compositor verifies the mode first (if not it fails) - then system compositor does the transformation Hope it gives u some idea to update the patch. I would be eager to test if you update the patch :-) BR imran On Thu, Jan 8, 2015 at 6:57 PM, Jason Ekstrand ja...@jlekstrand.net wrote: Previously, we blindly created a mode for the output based on surface size and completely ignoring the output transform. This caused modesets to fail on outputs that were transformed by 90 or 270 degrees. We should be swapping the width and the height in this case. --- fullscreen-shell/fullscreen-shell.c | 23 +-- 1 file changed, 21 insertions(+), 2 deletions(-) diff --git a/fullscreen-shell/fullscreen-shell.c b/fullscreen-shell/fullscreen-shell.c index 35e6d8f..5fd7cd6 100644 --- a/fullscreen-shell/fullscreen-shell.c +++ b/fullscreen-shell/fullscreen-shell.c @@ -463,9 +463,28 @@ fs_output_configure_for_mode(struct fs_output *fsout, surf_x, surf_y, surf_width, surf_height); + /* The actual output mode is in physical units. We need to +* transform the surface size to physical unit size by flipping ans +* possibly scaling it. +*/ + switch (fsout-output-transform) { + case WL_OUTPUT_TRANSFORM_90: + case WL_OUTPUT_TRANSFORM_FLIPPED_90: + case WL_OUTPUT_TRANSFORM_270: + case WL_OUTPUT_TRANSFORM_FLIPPED_270: + mode.width = surf_height * fsout-output-native_scale; + mode.height = surf_width * fsout-output-native_scale; + break; + + case WL_OUTPUT_TRANSFORM_NORMAL: + case WL_OUTPUT_TRANSFORM_FLIPPED: + case WL_OUTPUT_TRANSFORM_180: + case WL_OUTPUT_TRANSFORM_FLIPPED_180: + default: + mode.width = surf_width * fsout-output-native_scale; + mode.height = surf_height * fsout-output-native_scale; + } mode.flags = 0; - mode.width = surf_width * fsout-output-native_scale; - mode.height = surf_height * fsout-output-native_scale; mode.refresh = fsout-pending.framerate; ret = weston_output_mode_switch_to_temporary(fsout-output, mode, -- 2.2.0 ___ 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
[PATCH] fullscreen-shell: Fix modeset on transformed outputs
Previously, we blindly created a mode for the output based on surface size and completely ignoring the output transform. This caused modesets to fail on outputs that were transformed by 90 or 270 degrees. We should be swapping the width and the height in this case. --- fullscreen-shell/fullscreen-shell.c | 23 +-- 1 file changed, 21 insertions(+), 2 deletions(-) diff --git a/fullscreen-shell/fullscreen-shell.c b/fullscreen-shell/fullscreen-shell.c index 35e6d8f..5fd7cd6 100644 --- a/fullscreen-shell/fullscreen-shell.c +++ b/fullscreen-shell/fullscreen-shell.c @@ -463,9 +463,28 @@ fs_output_configure_for_mode(struct fs_output *fsout, surf_x, surf_y, surf_width, surf_height); + /* The actual output mode is in physical units. We need to +* transform the surface size to physical unit size by flipping ans +* possibly scaling it. +*/ + switch (fsout-output-transform) { + case WL_OUTPUT_TRANSFORM_90: + case WL_OUTPUT_TRANSFORM_FLIPPED_90: + case WL_OUTPUT_TRANSFORM_270: + case WL_OUTPUT_TRANSFORM_FLIPPED_270: + mode.width = surf_height * fsout-output-native_scale; + mode.height = surf_width * fsout-output-native_scale; + break; + + case WL_OUTPUT_TRANSFORM_NORMAL: + case WL_OUTPUT_TRANSFORM_FLIPPED: + case WL_OUTPUT_TRANSFORM_180: + case WL_OUTPUT_TRANSFORM_FLIPPED_180: + default: + mode.width = surf_width * fsout-output-native_scale; + mode.height = surf_height * fsout-output-native_scale; + } mode.flags = 0; - mode.width = surf_width * fsout-output-native_scale; - mode.height = surf_height * fsout-output-native_scale; mode.refresh = fsout-pending.framerate; ret = weston_output_mode_switch_to_temporary(fsout-output, mode, -- 2.2.0 ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: Feedback on xdg-shell from Plasma crew
Hi Martin! First off, thanks for looking at it and giving your thoughs. I'm going to reply to a few things below from a sort-of general wayland perspective I'll leave the hashing out of details to the people who actually work on real compositors. Hopefully, the rest of the list shows similar restraint *sigh*. On Mon, Dec 15, 2014 at 5:30 AM, Martin Gräßlin mgraess...@kde.org wrote: Hi all, I had a look at the xdg-shell proposal in weston (see [1]) and want to provide some feedback from a Plasma/KWin point of view. First of all: thanks for the work so far on the xdg-shell. The work looks quite good and promising. If you reply to the thread, please keep both the plasma and wayland mailing list on CC: not all domain experts from Plasma are on the wayland mailing list and I don't want to play proxy with my peer developers ;-) Done. Hopefully, not being a member of that list won't be a problem. I know they'll have to subscribe to the wayland list in order to be able to write to it. I'm grouping the feedback by some categories. Decorations --- My understanding is that xdg-shell surfaces are supposed to do client-side- decorations. If that is the case I consider the protocol as not yet sufficient for our needs. It would render huge regressions as several features are no longer possible. This includes: * putting windows on all desktops * putting a window to a keep above/keep below layer * shading windows (personally I don't mind if that goes away) For those three we have dedicated decoration buttons which can be globally configured. We would like to still be able to provide them. It seems a bit odd to me that those are common enough to need to be single-click. That said, it's still a regression if you can't do it. At some point in the past, xdg_shell had a way for clients to request these states. Then, for the sake of better separation (they're not really client things), they were moved into the compositor-handled context menu. In addition there are quite some interaction limitations: * configurable mouse actions: a right click might not trigger a context menu, mouse wheel support It doesn't have to. The client can use whatever button or key it wants to use to trigger the context menu. * multiple and configurable mode on the maximize button: KWin allows to maximize horizontally/vertically and fully depending which mouse button was used on the maximize button Yes, these are not modes that most other compositors have/support. They could be plasma-specific states. However, I immagine you'd like to be able to do that to firefox and other non-KDE apps. Also for integration with the desktop environment I see problems: * how to specify the button order? We don't want apps to do things like Opera did: putting the buttons on the wrong side ;-) Not much you can do about that in some ways. Then again, maybe Opera is annoyed that you keep moving their buttons. ;-) * specifying a drag delay to trigger move/resize (or better pass this into the compositor?) In general I fear that in the current state it would render a for us rather unsuitable system exposing the same problems as GTK's CSD in the X11 world. Ah, CSD, the age-old argument... I really hope we can avoid an all-out war on this. A few comments in general. First off, If we want windows to be decorated, we have to pick one of either the client or the server, that is required to support decorations. In Wayland, the choice has been made that clients always have to be able to decorate. That doesn't mean that WM's never can, but someone has to, and that someone is clients. I really don't think that's going to change, so let's just avoid debating it. That said, We could come up with some sort of negotiation. I think the preference for the moment (of myself and Kristian; can't speak for anyone else) would be to initially do this as its own extension which would be advertised by kwin and clients would support. We could eventually make it mainline, but let's get it working first. One note about said extensions is the question of who wins? KWin wants to decorate windows. GNOME apps want to do it themselves and frequently have reworked interfaces around it. Do we force them to be server-side or does KWin budge and let them be client-side? That's a question that I don't think will be easy to answer. Convergence One of our convergence features is that we adjust the window controls. E.g. * on Active/Touch no window has controls * on Netbook only dialogs have controls, while all other windows have no controls * On Desktop the user is allowed to decide per window whether a window should have controls I'm not completely sure on how to provide this. I'd say that this should be another state to tell the surface whether it should render controls. Also the client should tell whether it's currently rendering controls to prevent that a desktop shell
Re: Where should project Weston go?
Bryce, Thanks for your thoughts. I've got a few of my own, but I decided to reply to your e-mail as it seemed the best branch-point for the actual discussion without replying to everything. On Mon, Dec 8, 2014 at 3:26 PM, Bryce Harrington br...@osg.samsung.com wrote: On Mon, Dec 08, 2014 at 02:01:32PM +0200, Pekka Paalanen wrote: Dear Wayland community, I would like to start a discussion on what Weston really is, and where it should go, if only to confirm that our concensus still holds. Thanks for opening discussion on this. I suspect a lot of us have been pondering these questions for a while. To reiterate the main points: - the reference implementation - fast (performant?) - suitable for embedded and mobile (and now automotive?) Performance is really only relevant in context to a use case; otherwise this can lead us to pre-maturely optimizing stuff that no one cares about. So, maybe fast for embedded/mobile/automotive. Or maybe there's an implied use case in the original statement (i.e. fast desktop) that should be made explicit. I think that Weston's performance matters a lot in some respects and not much in others. We don't want to overcomplicate things by prematurely optimizing or squeezing out every clock cycle we can. However, Weston does give us the opportunity to show off some of cool performance things the Wayland protocol allows us to do. One example of this is the planes support in the drm backend. Weston may be the only desktop compositor around today that take a single subsurface out of a window, put it in a plane, and flip it directly to the screen. This is fantastic for performance and power usage and is one of the advantages of the Wayland protocol. If people come to us complaining that you can't do that in Wayland just because GNOME or KWin aren't doing it, we can point to Weston and say, See, yes you can, they just aren't. Another fantastic example of this is the way the RPi backend uses planes. So I think showing off performance things we can do thanks to Wayland is important, squeezing out clock cycles isn't. - minimal :-) When I see a FOSS project describe itself as lightweight I read it as We consider our lack of features to be a Feature! But I imagine 'minimal' is intended here in more of an engineering sense, and interpret it myself to mean something like: Focuses on principle features not superfluous stuff better handled by other projects; doesn't overengineer algorithms to squeeze a few drops of performance; feature selection by what fits nicely and makes sense, not by marketing demands; favors short and concise function implementations that are easy to maintain and understand, even if it limits performance/portability/features a bit. Weston is also a test bed for new protocols and features. We tend to land experimental protocols in Weston, try them out there (while in this stage, third party projects need to copy the XML files from Weston), and when they stabilize, we move protocols into the Wayland project and install the XML files from there for everyone to use. Judging from what I've seen on the mailing lists there certainly is a strong demand pushing weston to fulfil this role as a laboratory of experimentation. But Weston's development processes seem to be set up more for product development than for freeform RD. Namely, we have one single true tree with a gatekeeper to control what gets committed, reviewers to catch quality issues, and a growing testsuite. I'm not saying this is wrong, just that this is probably not ideal for facilitating experimentation; folks may prefer to gravitate towards other compositor development projects where they can turn ideas and patches around more swiftly. And that's not necessarily a bad thing... but it's different than considering weston as a test bed. Perhaps what we want here is easily forkable. Not that we want people to be trying to fork the project but that we want to make it easy for someone to work on a feature branch in their own github repo and make things easy to merge back in or rebase on upstream. What does an easily forkable project look like? That's a good question. First would be to encourage people to work in feature branches rather than have to have everything on the list and/or in Weston core. While I think most things may want to be upstream eventually, trying to do upstream everything as you go is hard when people are trying to use it as a test-bed and you're trying to do stable release at the same time. We may even want to allow stuff to go upstream by merges rather then everything as a linear series on the ML. Another thing might be to make things structured a little better to try and structure things in a more compartmentalized model. Right now, Weston has a lot of files that are thousands of lines and should probably be broken up a bit (I'm looking at you, compositor.c). Having things structured a little better
Re: [PATCH weston 13/17] Introduce wl_relative_pointer interface
Haven't looked at the weston implementation but the protocol bits look pretty good to me. Sounds like what we discussed. --Jason On Tue, Dec 2, 2014 at 5:49 AM, Jonas Ådahl jad...@gmail.com wrote: A wl_relative_pointer object is an extension to the wl_pointer interface only used for emitting relative pointer events. It will only emit events when the parent pointer has focus. To get a relative pointer object, use the get_relative_pointer request of the global wl_relative_pointer_manager object. When stabilizing it might make more sense to just add it to wl_seat instead of having a single use global interface. All interface names are currently prefixed with underscore in order to avoid any future conflicts with stable protocol. Signed-off-by: Jonas Ådahl jad...@gmail.com --- Makefile.am | 7 +- protocol/relative-pointer.xml | 90 + src/compositor.c | 3 + src/compositor.h | 5 ++ src/input.c | 184 +- 5 files changed, 266 insertions(+), 23 deletions(-) create mode 100644 protocol/relative-pointer.xml diff --git a/Makefile.am b/Makefile.am index e942850..860564d 100644 --- a/Makefile.am +++ b/Makefile.am @@ -81,7 +81,9 @@ nodist_weston_SOURCES = \ protocol/presentation_timing-protocol.c \ protocol/presentation_timing-server-protocol.h \ protocol/scaler-protocol.c \ - protocol/scaler-server-protocol.h + protocol/scaler-server-protocol.h \ + protocol/relative-pointer-protocol.c\ + protocol/relative-pointer-server-protocol.h BUILT_SOURCES += $(nodist_weston_SOURCES) @@ -1003,7 +1005,8 @@ EXTRA_DIST += \ protocol/xdg-shell.xml \ protocol/fullscreen-shell.xml \ protocol/presentation_timing.xml\ - protocol/scaler.xml + protocol/scaler.xml \ + protocol/relative-pointer.xml man_MANS = weston.1 weston.ini.5 diff --git a/protocol/relative-pointer.xml b/protocol/relative-pointer.xml new file mode 100644 index 000..f56c912 --- /dev/null +++ b/protocol/relative-pointer.xml @@ -0,0 +1,90 @@ +?xml version=1.0 encoding=UTF-8? +protocol name=relative_pointer + + copyright +Copyright © 2014 Jonas Ådahl + +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. + /copyright + + interface name=_wl_relative_pointer_manager version=1 +description summary=get relative pointer objects + A global interface used for getting the relative pointer object for a + given seat. +/description + +request name=get_relative_pointer + description summary=get a relative pointer object +Create a relative pointer interface for the pointer of the given seat. + +This request only takes effect if the seat has the pointer capability. + /description + + arg name=id type=new_id interface=_wl_relative_pointer/ + arg name=seat type=object interface=wl_seat/ +/request + /interface + + interface name=_wl_relative_pointer version=1 +description summary=relative pointer object + A wl_relative_pointer object is an extension to the wl_pointer interface + only used for emitting relative pointer events. It will only emit events + when the parent pointer has focus. +/description + +request name=release type=destructor + description summary=release the relative pointer object/ +/request Maybe this should be called destroy. I know that we call it release on wl_pointer and friends, but this is something of a historical accident. The problem is that if you
Re: [PATCH weston 14/17] Introduce pointer locking and confinement protocol
A couple of doc comments below, but the protocol otherwise looks pretty good. Again, I've only glanced at the implementation. On Tue, Dec 2, 2014 at 5:49 AM, Jonas Ådahl jad...@gmail.com wrote: This patch introduces a new protocol for locking and confining a pointer. It consists of a new global object with two requests; one for locking the surface to a position, one for confining the pointer to a given region. See pointer-lock.xml for details of the protocol. In this patch, only the locking part is fully implemented as in specified in the protocol, while confinement is only implemented for when the union of the passed region and the input region of the confined surface is a single rectangle. Note that the interfaces are prefixed with an underscore in order to avoid future incompatibilities with a future stable interface with an equivalent name. Signed-off-by: Jonas Ådahl jad...@gmail.com --- What should the serial in locked_pointer_set_cursor_position_hint be checked against? It doesn't seem to make sense to use the same method as in pointer_set_cursor. As far as I understand, it should be equal to the last serial sent via that pointer device, but I can't see we do that anywhere else. What am I missing? Makefile.am | 3 + protocol/pointer-lock.xml | 208 src/compositor.c | 4 + src/compositor.h | 21 ++ src/input.c | 589 -- 5 files changed, 806 insertions(+), 19 deletions(-) create mode 100644 protocol/pointer-lock.xml diff --git a/Makefile.am b/Makefile.am index 860564d..8adc343 100644 --- a/Makefile.am +++ b/Makefile.am @@ -82,6 +82,8 @@ nodist_weston_SOURCES = \ protocol/presentation_timing-server-protocol.h \ protocol/scaler-protocol.c \ protocol/scaler-server-protocol.h \ + protocol/pointer-lock-protocol.c\ + protocol/pointer-lock-server-protocol.h \ protocol/relative-pointer-protocol.c\ protocol/relative-pointer-server-protocol.h @@ -1006,6 +1008,7 @@ EXTRA_DIST += \ protocol/fullscreen-shell.xml \ protocol/presentation_timing.xml\ protocol/scaler.xml \ + protocol/pointer-lock.xml \ protocol/relative-pointer.xml man_MANS = weston.1 weston.ini.5 diff --git a/protocol/pointer-lock.xml b/protocol/pointer-lock.xml new file mode 100644 index 000..3233ede --- /dev/null +++ b/protocol/pointer-lock.xml @@ -0,0 +1,208 @@ +?xml version=1.0 encoding=UTF-8? +protocol name=pointer_lock + + copyright +Copyright © 2014 Jonas Ådahl + +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. + /copyright + + interface name=_wl_pointer_lock version=1 +description summary=lock pointer to a surface + The global interface exposing pointer locking functionality. It exposes + two requests; lock_pointer for locking the pointer to its position, and + confine_pointer for locking the pointer to a region. + + The lock_pointer and confine_pointer creates the objects wl_locked_pointer + and wl_confined_pointer respectively, and the client can use these objects + to interact with the lock. + + There may not be another lock of any kind active when requesting a lock, + and if there is, an error will be raised. +/description + +request name=lock_pointer + description summary=lock pointer to a position +The lock_pointer request lets the client disable absolute pointer +movements, locking the pointer to a position. + +There may not be another lock of
Re: [PATCH weston 00/17] Relative pointer motions, locking and confinement
On Tue, Dec 2, 2014 at 5:49 AM, Jonas Ådahl jad...@gmail.com wrote: Hi again, At XDC2014 some of us sat down and talked through how pointer locking and related protocols should look like, and this series is more or less work-in-progress result of that discussion. The series contains two parts (and one bonus patch). The bonus patch is the initial white space only patch formatting the input method protocols making it slightly more readable and consistent with other protocol files. The first part (2 - 12) is preparation for the implementation of the new interfaces. The second part (13 - 17) is the introduction, implementation and application of the new protocol interfaces. Accompanied with this series is also a patch to libinput (Introduce non-accelerated motion event vectors), and implementations in GLFW https://github.com/jadahl/glfw/commits/pointer-lock and SDL2 https://bitbucket.org/jadahl/sdl/commits/branch/pointer-lock. In GLFW you can try the wave example, and for SDL, you could try for example ioquake3 or openarena. One of the things we concluded was that relative pointer motions should be treated separately from locking and confinement, so of the second part, patch 13 introduces the new relative pointer object that is an additional interface to the wl_pointer object. Think of it in the same way, as in you get it from a seat, and it emits motion events, only ones with relative motion deltas. One can create as many as one wants, and they don't interfere with each other, just as wl_pointer. It might be better to simply extend wl_seat when stabilizing this protocol, but it's put separately for testing purposes. For details of how the protocol works, please read the protocol XML file. In short, locking and confinement are two locking modes, of which one may only be active at once. From the discussions, there is one difference I can think of and that is that locking/confining an already locked/confined pointer is invalid, instead of queued, as it simplified the implementation quite a lot, and I think for most cases unlocking and then locking again will work good enough. For the cases where such a scenario would break the lock and fail to relock, we could for example change the heuristics the compositor applies to get a better behavior, without making the interface more complex. Please tell if you think queued locking is indeed needed. Yeah, I think this is probably ok. And you're right that it simplifies things substantially. It should be easy enough for compositors to add a little heuristic to make it practical. Let's see how this goes and I think we can probably deal with it later if we want. One thing of the specification is currently not fully implemented as there are some semantical choices that needs to be made and specified before it makes sense to go forward, and that is how to deal with non rectangular locking regions. The current implementation will simply never lock when the union of the input region and the locking region is a non rectangular region. The semantical ambiguity is how to treat motion vectors that cross corners or other borders. Consider the following ASCII art examples: --- --- | ^ | | ^ | |\| |\| | | | | -- or| - | \ | | \ | |\ | |\ | Simply clamping as done where there is no corners or anything would be result in: --- --- | | | | | | | | | | | | -- or| - | ^ || ^ | |\ ||\ | Should these kind of clamping take place or not, or should they be done some other way? Naturally, only clamping to a rectangle is the simplest non-ambiguous way to go and we could simply say that locking is to the largest rectangle within the union of the locking region and the input region, but as input regions can be split, we'd have to deal with that situation as well. Any ideas of the preferable way to deal with the locking regions? First off, I don't think we want to specify this in the protocol. That would be insane. If the client wants it to be particularly predictable, it should use a rectangle. If the client gives a more complex region, It gets what it gets. However, we do need to restrict to the region (that's the point of a confinement) so we can't just take the extents. With that out of the way, I'm not sure what algorithm would be best to use. Basically what we're doing is collision detection and there several algorithms for doing that reasonably efficiently.
Re: [PATCH weston] Make border width and ball radius configurable
I don't see anything wrong with this patch, but I'm forced to ask the question: What's the point? I mean, it's a little bouncing ball, why does it need to be configurable? On Sat, Nov 29, 2014 at 5:36 AM, Seedo Eldho Paul seedoeldhop...@gmail.com wrote: Also change simple-shm exiting to simple-damage exiting Signed-off-by: Seedo Eldho Paul seedoeldhop...@gmail.com --- clients/simple-damage.c | 21 ++--- 1 file changed, 14 insertions(+), 7 deletions(-) diff --git a/clients/simple-damage.c b/clients/simple-damage.c index fe532fe..9f3d58f 100644 --- a/clients/simple-damage.c +++ b/clients/simple-damage.c @@ -172,8 +172,6 @@ window_init_game(struct window *window) gettimeofday(tv, NULL); srand(tv.tv_usec); - window-ball.radius = 10; - ax1 = window-border + window-ball.radius; ay1 = window-border + window-ball.radius; ax2 = window-width - window-border - window-ball.radius; @@ -233,8 +231,8 @@ window_advance_game(struct window *window, uint32_t timestamp) static struct window * create_window(struct display *display, int width, int height, - enum wl_output_transform transform, int scale, - enum window_flags flags) + int border, int radius, enum wl_output_transform transform, + int scale, enum window_flags flags) { struct window *window; @@ -268,7 +266,8 @@ create_window(struct display *display, int width, int height, window-display = display; window-width = width; window-height = height; - window-border = 10; + window-border = border; + window-ball.radius = radius; window-flags = flags; window-transform = transform; window-scale = scale; @@ -772,6 +771,8 @@ print_usage(int retval) --version=VERSION\tVersion of wl_surface to use\n --width=WIDTH\t\tWidth of the window\n --height=HEIGHT\tHeight of the window\n + --border=BORDER\tBorder width of the window\n + --radius=RADIUS\tRadius of the ball\n --scale=SCALE\t\tScale factor for the surface\n --transform=TRANSFORM\tTransform for the surface\n --rotating-transform\tUse a different buffer_transform for each frame\n @@ -818,6 +819,7 @@ main(int argc, char **argv) int i, ret = 0; int version = -1; int width = 300, height = 200, scale = 1; + int border = 10, radius = 10; enum wl_output_transform transform = WL_OUTPUT_TRANSFORM_NORMAL; enum window_flags flags = 0; @@ -839,6 +841,10 @@ main(int argc, char **argv) continue; } else if (sscanf(argv[i], --height=%d, height) 0) { continue; + } else if (sscanf(argv[i], --border=%d, border) 0) { + continue; + } else if (sscanf(argv[i], --radius=%d, radius) 0) { + continue; } else if (strncmp(argv[i], --transform=, 12) == 0 parse_transform(argv[i] + 12, transform) 0) { continue; @@ -858,7 +864,8 @@ main(int argc, char **argv) display = create_display(version); - window = create_window(display, width, height, transform, scale, flags); + window = create_window(display, width, height, border, radius, + transform, scale, flags); if (!window) return 1; @@ -872,7 +879,7 @@ main(int argc, char **argv) while (running ret != -1) ret = wl_display_dispatch(display-display); - fprintf(stderr, simple-shm exiting\n); + fprintf(stderr, simple-damage exiting\n); destroy_window(window); destroy_display(display); -- 1.9.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: [PATCH weston 5/6] xdg-shell: Rewrite documentation
I agree with Pekka's comments, I also have a couple of my own below. (Most of them are pretty trivial). With Pekka's and my comments addressed, this series looks good to me. --Jason On Sat, Nov 22, 2014 at 12:28 PM, Jasper St. Pierre jstpie...@mecheye.net wrote: This rewrites basically all of the text inside xdg-shell to be up to date, clearer, and rid of wl_shell and X11 terminology. --- protocol/xdg-shell.xml | 256 ++--- 1 file changed, 156 insertions(+), 100 deletions(-) diff --git a/protocol/xdg-shell.xml b/protocol/xdg-shell.xml index 360179d..3359cf7 100644 --- a/protocol/xdg-shell.xml +++ b/protocol/xdg-shell.xml @@ -31,11 +31,15 @@ interface name=xdg_shell version=1 description summary=create desktop-style surfaces - This interface is implemented by servers that provide - desktop-style user interfaces. - - It allows clients to associate a xdg_surface with - a basic surface. + xdg_shell allows clients to turn a wl_surface into a real window + which can be dragged, resized, stacked, and moved around by the + user. Everything about this interface is suited towards traditional + desktop environments. + + Destroying a bound xdg_shell object while there are surfaces + still alive with roles from this interface is illegal and will + result in a protocol error. Make sure to destroy all surfaces + before destroying this object. /description enum name=version @@ -65,33 +69,26 @@ request name=get_xdg_surface description summary=create a shell surface from a surface - Create a shell surface for an existing surface. - - This request gives the surface the role of xdg_surface. If the - surface already has another role, it raises a protocol error. - - Only one shell or popup surface can be associated with a given - surface. + This creates an xdg_surface for the given surface and gives it the + xdg_surface role. See the documentation of xdg_surface for more details. /description arg name=id type=new_id interface=xdg_surface/ arg name=surface type=object interface=wl_surface/ /request request name=get_xdg_popup - description summary=create a shell surface from a surface - Create a popup surface for an existing surface. - - This request gives the surface the role of xdg_popup. If the - surface already has another role, it raises a protocol error. + description summary=create a popup for a surface + This creates an xdg_popup for the given surface and gives it the + xdg_popup role. See the documentation of xdg_surface for more details. - Only one shell or popup surface can be associated with a given - surface. + This request must be used in response to some sort of user action + like a button press, key press, or touch down event. /description arg name=id type=new_id interface=xdg_popup/ arg name=surface type=object interface=wl_surface/ arg name=parent type=object interface=wl_surface/ - arg name=seat type=object interface=wl_seat summary=the wl_seat whose pointer is used/ - arg name=serial type=uint summary=serial of the implicit grab on the pointer/ + arg name=seat type=object interface=wl_seat summary=the wl_seat of the user event/ + arg name=serial type=uint summary=the serial of the user event/ arg name=x type=int/ arg name=y type=int/ /request @@ -107,7 +104,7 @@ respond to the ping request, or in what timeframe. Clients should try to respond in a reasonable amount of time. /description - arg name=serial type=uint summary=pass this to the callback/ + arg name=serial type=uint summary=pass this to the pong request/ /event request name=pong @@ -120,8 +117,7 @@ /interface interface name=xdg_surface version=1 - -description summary=desktop-style metadata interface +description summary=A desktop window An interface that may be implemented by a wl_surface, for implementations that provide a desktop-style user interface. @@ -136,20 +132,22 @@ /description request name=destroy type=destructor - description summary=remove xdg_surface interface - The xdg_surface interface is removed from the wl_surface object - that was turned into a xdg_surface with - xdg_shell.get_xdg_surface request. The xdg_surface properties, - like maximized and fullscreen, are lost. The wl_surface loses - its role as a xdg_surface. The wl_surface is unmapped. + description summary=Destroy the xdg_surface + Unmap and destroy the window. The window will be effectively + hidden from the user's point of view, and all state like + maximization, fullscreen, and so
Re: Recognizing wl_display pointer for EGL (Re: Smart comparing of wl_display_interfaces)
On Thu, Oct 16, 2014 at 11:16 PM, Pekka Paalanen ppaala...@gmail.com wrote: On Thu, 16 Oct 2014 10:40:20 +0300 Pekka Paalanen ppaala...@gmail.com wrote: On Thu, 16 Oct 2014 09:46:39 +0300 Pekka Paalanen ppaala...@gmail.com wrote: On Wed, 15 Oct 2014 22:44:25 +0400 Dmitry Cherkassov dcherkas...@gmail.com wrote: Hi list! The definition of wl_display_interface symbol can come from libwayland-server.so or libwayland-client.so. There is a code in closed source EGL implementation that does interfaces comparison via pointers, yielding negative results when addresses are different (one address is saved by QtWayland, the other one is from wl_display_interface in EGL source code). Is there a smarter way to compare wl_display_interface's? There is this function: wl_interface_equal(const struct wl_interface *a, const struct wl_interface *b) { return a == b || strcmp(a-name, b-name) == 0; } But as you can see it's not safe to use it alone because in case of bad pointer we will get undefined results. Currently struct wl_interface is the following: struct wl_interface { const char *name; int version; int method_count; const struct wl_message *methods; int event_count; const struct wl_message *events; }; Since there is no magic field at the beginning it is not possible to see whether a pointer points to the valid wl_interface structure. So my question is: how can we compare wl_interfaces in more clever and safer way? Hmm, why would you ever have a bad pointer? Bad pointers in general tend to cause crashes and stuff, so you shouldn't be having a bad one in the first place. What is the background for your question/problem? Ok, I suppose this was discussed in IRC last night: the actual problem behind the question is how to detect whether the void pointer (EGLNativeDisplayType) given to eglCreateDisplay is actually a wl_display. That's tough. A couple of hacks come to mind. bool _eglNativePlatformDetectWayland(void *nativeDisplay) { void *f; if (!dereferencable(nativeDisplay)) return false; f = *(void **)nativeDisplay; The first is to dlopen both libwayland-client and libwayland-server in turn with RTLD_NOLOAD | RTLD_LOCAL, and fetching the wl_display_interface symbol from each, comparing those to 'f'. If either matches, return true. While discussing in IRC, I realized half of this is nonsense. eglGetDisplay() can only get a wl_display created by libwayland-client, and never one created by libwayland-server. Assuming the dynamic linking is sane, the interface pointer in wl_display is *always* the one from libwayland-client.so. Now we just need to make sure, that we compare 'f' to the interface pointer from libwayland-client.so. That could probably be done with the dlopen/dlsym trick, or... Another would be to check if 'f' is dereferencable (and not equal to the default wl_display_interface), and check if (char *)f starts with wl_d. If yes, do a full string comparison with wl_display. It's not reliable, but maybe works enough. The proper solution is for apps to use EGL_KHR_platform_wayland to explicitly say the pointer is a wl_display. That of course doesn't work on old apps, so you might implement a workaround through environment like Mesa does: EGL_PLATFORM=wayland bypasses all autodetection and assumes it is a wl_display. In any case, EGL really should implement EGL_KHR_platform_wayland so we have a chance to get rid of the inherently unreliable autodetect. And we should start using that in our demo apps... If we consider autodetecting client wl_display pointers worth to solve properly, we should probably add a function for that in libwayland-client.so. However, implementing that portably might be difficult. ...adding a function int/bool wl_is_wl_display_pointer(void *p) into libwayland-client.so. As that function would be built into libwayland-client.so, it naturally uses the client.so version of wl_display_interface symbol (unless dynamic linking miraculously manages to mess that up?). 'p' would be the tentative 'struct wl_display *', so that the implementation details of struct wl_display would be contained in libwayland-client.so, and not leaked elsewhere like e.g. in Mesa. All this means we should be able to get away with just one pointer comparison, after assuring that the first sizeof(void*) bytes pointed to by the given unknown pointer (EGLNativeDisplayType) are dereferencable without a segfault or other fatal error. So, a quick fix for EGL implementations right now would be to try the dlopen hack, until they can start to depend on a new libwayland-client.so providing wl_is_wl_display_pointer(). I am unsure about the exact definition of
Re: Recognizing wl_display pointer for EGL (Re: Smart comparing of wl_display_interfaces)
On Oct 19, 2014 12:03 AM, Pekka Paalanen ppaala...@gmail.com wrote: On Sat, 18 Oct 2014 11:19:55 -0700 Jason Ekstrand ja...@jlekstrand.net wrote: On Thu, Oct 16, 2014 at 11:16 PM, Pekka Paalanen ppaala...@gmail.com wrote: On Thu, 16 Oct 2014 10:40:20 +0300 Pekka Paalanen ppaala...@gmail.com wrote: On Thu, 16 Oct 2014 09:46:39 +0300 Pekka Paalanen ppaala...@gmail.com wrote: On Wed, 15 Oct 2014 22:44:25 +0400 Dmitry Cherkassov dcherkas...@gmail.com wrote: Hi list! The definition of wl_display_interface symbol can come from libwayland-server.so or libwayland-client.so. There is a code in closed source EGL implementation that does interfaces comparison via pointers, yielding negative results when addresses are different (one address is saved by QtWayland, the other one is from wl_display_interface in EGL source code). Is there a smarter way to compare wl_display_interface's? There is this function: wl_interface_equal(const struct wl_interface *a, const struct wl_interface *b) { return a == b || strcmp(a-name, b-name) == 0; } But as you can see it's not safe to use it alone because in case of bad pointer we will get undefined results. Currently struct wl_interface is the following: struct wl_interface { const char *name; int version; int method_count; const struct wl_message *methods; int event_count; const struct wl_message *events; }; Since there is no magic field at the beginning it is not possible to see whether a pointer points to the valid wl_interface structure. So my question is: how can we compare wl_interfaces in more clever and safer way? Hmm, why would you ever have a bad pointer? Bad pointers in general tend to cause crashes and stuff, so you shouldn't be having a bad one in the first place. What is the background for your question/problem? Ok, I suppose this was discussed in IRC last night: the actual problem behind the question is how to detect whether the void pointer (EGLNativeDisplayType) given to eglCreateDisplay is actually a wl_display. That's tough. A couple of hacks come to mind. bool _eglNativePlatformDetectWayland(void *nativeDisplay) { void *f; if (!dereferencable(nativeDisplay)) return false; f = *(void **)nativeDisplay; The first is to dlopen both libwayland-client and libwayland-server in turn with RTLD_NOLOAD | RTLD_LOCAL, and fetching the wl_display_interface symbol from each, comparing those to 'f'. If either matches, return true. While discussing in IRC, I realized half of this is nonsense. eglGetDisplay() can only get a wl_display created by libwayland-client, and never one created by libwayland-server. Assuming the dynamic linking is sane, the interface pointer in wl_display is *always* the one from libwayland-client.so. Now we just need to make sure, that we compare 'f' to the interface pointer from libwayland-client.so. That could probably be done with the dlopen/dlsym trick, or... Another would be to check if 'f' is dereferencable (and not equal to the default wl_display_interface), and check if (char *)f starts with wl_d. If yes, do a full string comparison with wl_display. It's not reliable, but maybe works enough. The proper solution is for apps to use EGL_KHR_platform_wayland to explicitly say the pointer is a wl_display. That of course doesn't work on old apps, so you might implement a workaround through environment like Mesa does: EGL_PLATFORM=wayland bypasses all autodetection and assumes it is a wl_display. In any case, EGL really should implement EGL_KHR_platform_wayland so we have a chance to get rid of the inherently unreliable autodetect. And we should start using that in our demo apps... If we consider autodetecting client wl_display pointers worth to solve properly, we should probably add a function for that in libwayland-client.so. However, implementing that portably might be difficult. ...adding a function int/bool wl_is_wl_display_pointer(void *p) into libwayland-client.so. As that function would be built into libwayland-client.so, it naturally uses the client.so version of wl_display_interface symbol (unless dynamic linking miraculously manages to mess that up?). 'p' would be the tentative 'struct wl_display *', so that the implementation details of struct wl_display would be contained in libwayland-client.so, and not leaked elsewhere like e.g. in Mesa. All this means we should be able to get away with just one pointer comparison, after
Re: [PATCH 2/2] Support for adjusting socket access rights to allow group of users to connect to the socket.
On Thu, Oct 16, 2014 at 9:23 AM, Imran Zaman imran.za...@gmail.com wrote: This is used for nested compositor architectures. Could you please provide a little more explanation than that. What kind of nesting are you doing? Also, why are you doing this through environment variables and not something explicit? For instance, the compositor can easily grab the socket and chmod it. It has the privileges and knows what socket it is. --Jason Ekstrand Signed-off-by: Imran Zaman imran.za...@gmail.com --- src/wayland-server.c | 27 +++ 1 file changed, 27 insertions(+) diff --git a/src/wayland-server.c b/src/wayland-server.c index 09e8903..721fabe 100644 --- a/src/wayland-server.c +++ b/src/wayland-server.c @@ -39,6 +39,8 @@ #include fcntl.h #include sys/file.h #include sys/stat.h +#include sys/types.h +#include grp.h #include ffi.h #include wayland-private.h @@ -1117,6 +1119,10 @@ static int _wl_display_add_socket(struct wl_display *display, struct wl_socket *s) { socklen_t size; + const char *socket_mode_str; + const char *socket_group_str; + const struct group *socket_group; + unsigned socket_mode; s-fd = wl_os_socket_cloexec(PF_LOCAL, SOCK_STREAM, 0); if (s-fd 0) { @@ -1134,6 +1140,27 @@ _wl_display_add_socket(struct wl_display *display, struct wl_socket *s) return -1; } + socket_group_str = getenv(WAYLAND_SERVER_GROUP); + if (socket_group_str != NULL) { + socket_group = getgrnam(socket_group_str); + if (socket_group != NULL) { + if (chown(s-addr.sun_path, + -1, socket_group-gr_gid) != 0) + wl_log(chown(\%s\) failed: %s, + s-addr.sun_path, + strerror(errno)); + } + } + socket_mode_str = getenv(WAYLAND_SERVER_MODE); + if (socket_mode_str != NULL) { + if (sscanf(socket_mode_str, %o, socket_mode) 0) + if (chmod(s-addr.sun_path, socket_mode) != 0) { + wl_log(chmod(\%s\) failed: %s, + s-addr.sun_path, + strerror(errno)); + } + } + s-source = wl_event_loop_add_fd(display-loop, s-fd, WL_EVENT_READABLE, socket_data, display); -- 1.9.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: [PATCH wayland] wl_strtol and wl_strtoul utility functions are added
I don't see how this belongs in libwayland. Sure, we use strtol twice, but I don't think that warrants adding 100 lines of wrapper functions and test cases. --Jason Ekstrand On Wed, Oct 15, 2014 at 6:16 AM, Rémi Denis-Courmont r...@remlab.net wrote: Le 2014-10-15 16:14, Imran Zaman a écrit : Hi The patch is used to replace strtol and strtoul with wl_strtol and wl_strtoul with inputs and result checks. I don't know where Wayland developers stand on this, but I would rather the client library function calls not clobber errno to zero. -- Rémi Denis-Courmont ___ 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] gl-renderer: don't move memory in output_rotate_damage
Looks good to me. Pushed. ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: Dual display clone mode in Wayland
On Thu, Oct 9, 2014 at 12:28 AM, Sathishkumar Sivagurunathan sathishkumar.sivagurunat...@igate.com wrote: Hi, I am trying to bring up Weston on laptop. My aim is to see the display on two screens (Laptop screen [1366x768] and HDMI display [1920x1080]).. Is there a way that I can mentioning in weston.ini file so that the displays are clone of each other.. As far as I know, this is not yet implemented in Weston. There was someone working on it at one point, but I haven't heard anything about it in a while. --Jason Ekstrand Currently, they seem to be in extend more.. Thanks, Sathish ~~Disclaimer~~~ Information contained and transmitted by this e-mail is confidential and proprietary to IGATE and its affiliates and is intended for use only by the recipient. If you are not the intended recipient, you are hereby notified that any dissemination, distribution, copying or use of this e-mail is strictly prohibited and you are requested to delete this e-mail immediately and notify the originator or mailad...@igate.com. IGATE does not enter into any agreement with any party by e-mail. Any views expressed by an individual do not necessarily reflect the view of IGATE. IGATE is not responsible for the consequences of any actions taken on the basis of information provided, through this email. The contents of an attachment to this e-mail may contain software viruses, which could damage your own computer system. While IGATE has taken every reasonable precaution to minimise this risk, we cannot accept liability for any damage which you sustain as a result of software viruses. You should carry out your own virus checks before opening an attachment. To know more about IGATE please visit www.igate.com. ___ 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 libwayland] Track protocol object versions inside wl_proxy.
On Mon, Oct 6, 2014 at 12:20 AM, Pekka Paalanen ppaala...@gmail.com wrote: On Sun, 5 Oct 2014 18:48:27 -0700 Jasper St. Pierre jstpie...@mecheye.net wrote: One of my issues is what happens for objects that have multiple parents, like wl_buffer or wl_callback. What do we do if another person wants to introduce another (like the recently proposed pointer lock extension)? We can recommend that users never use wl_callback directly and instead use their own types instead because there's no reason *not* to, but what do we do about wl_buffer? Or others? Do we prohibit that going forward? Do we strongly word against it? Do we invent new versioning rules for those? My opinion is that: - wl_buffer is beyond repair; we need it, it cannot be extended by version bumps so it is always equivalent to its version 1, and new factories will arise which create wl_buffer objects. As long as wl_buffer definition version is never bumped, we are ok. - wl_callback is similarly stuck to version 1, but we can discourage new cases trying to use, especially if there is *any* imaginable reason one might need more than done with an arbitary uint32 arg. And we should probably discourage wl_callback in new protocols anyway. - If someone tries to introduce a new case where an object that had only one factory root (the global at the root of the interface factory ancestry) will now have more than one, we first just NAK the patch. If the submitter claims there is no other way to make it work, we have to look closer and find another way. We need to try hard to not repeat the wl_buffer design, but I acknowledge that there might be a case where the wl_buffer design is the only working design. Agreed on all 3 points What about use cases much like wl_buffer, which really wants a subtyping mechanism? Recommend users use a role mechanism akin to wl_surface? Should we consider adding more infrastructure and helpers in libwayland to support role-d objects, much like what Pekka added to Weston recently? I don't want to propose any generic rules here for now. Let's see case by case if a pattern emerges later. In the wl_buffer case, we could not extend wl_buffer itself even with associate interface (like wl_viewport is for wl_surface, or the role mechanism), because EGL does not normally expose wl_buffer to client code. On Sun, Oct 5, 2014 at 5:30 PM, Jason Ekstrand ja...@jlekstrand.net wrote: Ping On Fri, Apr 11, 2014 at 1:36 AM, Jason Ekstrand ja...@jlekstrand.net wrote: On Fri, Apr 11, 2014 at 2:03 AM, Pekka Paalanen ppaala...@gmail.com wrote: On Thu, 10 Apr 2014 09:42:55 -0500 Jason Ekstrand ja...@jlekstrand.net wrote: On Thu, Apr 10, 2014 at 6:37 AM, Pekka Paalanen ppaala...@gmail.com wrote: Hi Jason, thanks for working on this, it does seem very useful, practically a mandatory feature to support. Hi Pekka, Yeah, I've been itching to knock this out for a while. Just finally got around to it. Comments below. ... On Wed, 2 Apr 2014 09:48:29 -0500 Jason Ekstrand ja...@jlekstrand.net wrote: It's worth noting that there is one small backwards-compatability issue here. Namely, if the client is built against protocol stubs from an earlier version of libwayland but links against a library built against a newer version, then all objects created by the client will report a version of 1. This is because the old api uses wl_proxy_marshal_constructor in wl_registry_bind so all objects will inherit the protocol version of wl_display which is 1. The library the client linked against is aware of the wl_proxy_version function but has no way of knowing that the library does not. I was about to say that wl_registry_bind() does pass the version to wl_proxy_marshal_constructor, but that indeed is in the request arguments and not a mandatory argument. But wl_proxy_marshal_constructor() would still have all the information it needs, if we special-case wl_registry.bind inside it. Ugly, but I guess it'd work for wl_registry. Would that make the backward-compatibility issue go away? In all other cases you would take the version from the parent wl_proxy, which you always have available in wl_proxy_marshal_constructor(), and the versioned variant would not be needed? Yes, we could do that, and I considered it. However, it would only bump the compatibility issue back to wl_proxy_marshal_constructor. Older code that uses wl_proxy_create inside of wl_registry_bind is still in trouble. I don't think it's a huge savings to just bump the compatibility issues back to 1.3 rather than 1.4/1.5. In the long run, I don't think it's worth the mess that we would create
Re: [PATCH wayland] client: add wl_proxy_get_version function
On Oct 6, 2014 12:45 AM, Pekka Paalanen ppaala...@gmail.com wrote: Re-adding CCs and some more... On Mon, 06 Oct 2014 09:17:41 +0300 Rémi Denis-Courmont r...@remlab.net wrote: Hello, Le 2014-10-06 03:29, Jason Ekstrand a écrit : Remi, While this would probably be nice, your approach isnt going to work. Yeah, I figured that much after I submitted the patch. And then I rediscovered that the client library does not actually know the version of the project objects, and wl_registry_bind() cannot be modified without breaking backward compatibility. But now, I am even wondering whether this makes any sense. Assuming wl_proxy_get_version() gets implemented somehow, what should the library do if the proxy version is *higher* than version the library knows of? Say a new version Y of an interface FOO adds a new request and a new event compared to older version X. If a FOO object is instantiated with version Y but the new request is never used and there is no event handler for the new event, will it still work as with version X? I guess not :-( If there is no event handler set, the client will abort/crash if the event is ever sent. In my specific case, can the Wayland client treat a wl_surface version 4 or higher as a wl_surface version 3? If not, I cannot rely on 'wl_proxy_get_version(surface) = 2'... You have to do some sort of negotiation between The client and the library. If you have the get_version stuff you can enforce it somewhat by giving the client an error if it goes too high. Changing the semantics[5] of wl_surface.damage is a good example where this would fail indeed. If a library only knows up to version 3, but the app gives it a wl_surface of version 4, the damage coordinates would be wrong. Originally we had the idea, that all version bumps are backwards compatible. Simply testing for version = X would always work. The wl_surface.damage change is not backwards compatible in that sense. Looks to me like if we don't fix damage, then this versioning problem would be solvable... :-( I'll respond to that on the bug. I don't want to side-track this conversation with damage. Also somewhat related to this question, what happens (or should happen) if a Wayland client binds a version supported by the display server, but not supported by the client's protocol bindings? That would happen if the client blindly copies the version from wl_registry.global to wl_registry.bind. Such blind copy is a bug: a client will always need some code changes to support new features added to an interface, so it always needs to bind with MIN(advertized, my_supported). And my_supported cannot be higher than the available XML definition during client build. But here is a problem with client-side libraries: my_supported cannot be higher than the supported version in any used library, if not all protocol version bumps are backward compatible. And even then, I can imagine a case where it would still fail: app creates version 5 of a global, hands the global to a library that only knows up to version 4. The library creates a new object with the version 4 interface, automatically gets version *5* new object, the new object delivers an event added in version 5, kaboom. A compositor does not advertise a version that libwayland-server does not support, but that includes the assumption that both server and client use the same libwayland version... which might not be true. And it's not really remedied by adding new protocols as XML-only into libwayland, either. Such protocols would simply be built in to the users, i.e. the client apps and libraries, which brings a problem if a library is built with version lower than what the app uses. Oh dear... :-/ I'm not sure there is any way around *bidirectional* explicit version negotiation between an app the libraries it uses. Bidirectional means that the library tells the max versions it can support and the app needs to obey that, and any objects the app passes to the library needs to pass the version, too. Well, the latter case might be able to be automated via wl_proxy_get_version(), but not the former AFAICS. Yes, I don't think we can get around that. --Jason (...) http://lists.freedesktop.org/archives/wayland-devel/2014-April/014004.html [4] I see, thanks. Thanks, pq [5] https://bugs.freedesktop.org/show_bug.cgi?id=78190 ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH wayland] client: add wl_proxy_get_version function
Remi, While this would probably be nice, your approach isn't going to work. The version provided in wl_interface is the version that was compiled into libwayland, not the version requested when the global was bound. In order to get this correct, it requires a bit more tracking. I did implement this a while ago but it never got merged. You can find it here: http://lists.freedesktop.org/archives/wayland-devel/2014-April/014004.html --Jason Ekstrand On Sun, Oct 5, 2014 at 2:02 AM, Rémi Denis-Courmont r...@remlab.net wrote: In some scenarii, a component will obtain a reference to a proxy object created by another component. It may then be necessary to check the interface version of the proxy object at run-time to determine if a certain interface request is supported by the proxy object. For instance, a media player GUI can pass a wl_surface to a video playback library. If the video is rotated, the library will want to use wl_surface.set_buffer_transform, which is only available in version 2 of wl_surface. Signed-off-by: Rémi Denis-Courmont r...@remlab.net --- src/wayland-client.c | 13 + src/wayland-client.h | 1 + 2 files changed, 14 insertions(+) diff --git a/src/wayland-client.c b/src/wayland-client.c index b0f77b9..51216ae 100644 --- a/src/wayland-client.c +++ b/src/wayland-client.c @@ -1721,6 +1721,19 @@ wl_proxy_get_class(struct wl_proxy *proxy) return proxy-object.interface-name; } +/** Get the interface version of a proxy object + * + * \param proxy The proxy object + * \return The interface version of the object associated with the proxy + * + * \memberof wl_proxy + */ +WL_EXPORT uint32_t +wl_proxy_get_version(struct wl_proxy *proxy) +{ + return proxy-object.interface-version; +} + /** Assign a proxy to an event queue * * \param proxy The proxy object diff --git a/src/wayland-client.h b/src/wayland-client.h index 0801a81..60d87e6 100644 --- a/src/wayland-client.h +++ b/src/wayland-client.h @@ -146,6 +146,7 @@ void wl_proxy_set_user_data(struct wl_proxy *proxy, void *user_data); void *wl_proxy_get_user_data(struct wl_proxy *proxy); uint32_t wl_proxy_get_id(struct wl_proxy *proxy); const char *wl_proxy_get_class(struct wl_proxy *proxy); +uint32_t wl_proxy_get_version(struct wl_proxy *proxy); void wl_proxy_set_queue(struct wl_proxy *proxy, struct wl_event_queue *queue); #include wayland-client-protocol.h -- 2.1.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 libwayland] Track protocol object versions inside wl_proxy.
Ping On Fri, Apr 11, 2014 at 1:36 AM, Jason Ekstrand ja...@jlekstrand.net wrote: On Fri, Apr 11, 2014 at 2:03 AM, Pekka Paalanen ppaala...@gmail.com wrote: On Thu, 10 Apr 2014 09:42:55 -0500 Jason Ekstrand ja...@jlekstrand.net wrote: On Thu, Apr 10, 2014 at 6:37 AM, Pekka Paalanen ppaala...@gmail.com wrote: Hi Jason, thanks for working on this, it does seem very useful, practically a mandatory feature to support. Hi Pekka, Yeah, I've been itching to knock this out for a while. Just finally got around to it. Comments below. ... On Wed, 2 Apr 2014 09:48:29 -0500 Jason Ekstrand ja...@jlekstrand.net wrote: It's worth noting that there is one small backwards-compatability issue here. Namely, if the client is built against protocol stubs from an earlier version of libwayland but links against a library built against a newer version, then all objects created by the client will report a version of 1. This is because the old api uses wl_proxy_marshal_constructor in wl_registry_bind so all objects will inherit the protocol version of wl_display which is 1. The library the client linked against is aware of the wl_proxy_version function but has no way of knowing that the library does not. I was about to say that wl_registry_bind() does pass the version to wl_proxy_marshal_constructor, but that indeed is in the request arguments and not a mandatory argument. But wl_proxy_marshal_constructor() would still have all the information it needs, if we special-case wl_registry.bind inside it. Ugly, but I guess it'd work for wl_registry. Would that make the backward-compatibility issue go away? In all other cases you would take the version from the parent wl_proxy, which you always have available in wl_proxy_marshal_constructor(), and the versioned variant would not be needed? Yes, we could do that, and I considered it. However, it would only bump the compatibility issue back to wl_proxy_marshal_constructor. Older code that uses wl_proxy_create inside of wl_registry_bind is still in trouble. I don't think it's a huge savings to just bump the compatibility issues back to 1.3 rather than 1.4/1.5. In the long run, I don't think it's worth the mess that we would create inside wl_proxy_marshal_constructor. Ok, I didn't even know to think that far back. Makes sense. But I guess it would still be broken on any other request that used the interfaceless format of new_id? Nope, that's not a problem. The wayland-scanner program doesn't actually special case wl_registry_bind but interfaceless new_id's in general. Anything else that specifies a new_id with no interface will hit the same code-path and get wl_proxy_marshal_constructor_versioned. I meant if we special-case wl_registry.bind, then all other requests using interfaceless new_ids would still be in trouble. But yes, a moot point now. One possible solution for this is to set the version of wl_display to zero and use zero to mean unversioned. Then, if a library wants to use something that's not strictly backwards-compatable, it can check for zero and use whatever it's non-versioned fallback is. Thoughts on this? ^^ Well... if you don't know the version, is there a difference between assuming it is 1, and knowning it is unknown and then assuming whatever? As I see it, the only benefit of knowing when you don't know, is that you could then explicitly assume a higher version than 1, and then die on a protocol error if you are wrong. I'm not sure if that is better than assuming 1 which will always work if the application only accepts that. There is a case where I would do something different on unknown vs. version == 1. In fact, it's the exact came case that inspired me to actually sit down and write this. The wl_surface.damage request, from the perspective of EGL implementations, is completely broken on wl_surface versions 2 and 3 (trying to fix it for 4). An EGL implementation could check for unknown, 2, or 3 and just do wl_surface.damage(0, 0, INT32_MAX, INT32_MAX) in both eglSwapBuffers and eglSwapBuffersWithDamageEXT to work around this. Then, if the version is 2 or = 4, they could just damage the surface correctly. It's kind of a specific example and the only reason why we care is because we broke stuff at wl_surface version 2 but it's an example. --Jason It looks like a trade-off between an unknown method protocol error / events that never come, and the application complaining the server is too old, when things go wrong. I can't say. Thanks, pq ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH 4/4] wayland-server: Abort if a read from a client gives 0 length
On Sun, Oct 5, 2014 at 2:23 PM, Karsten Otto karsten.o...@posteo.de wrote: Am 29.09.2014 um 06:31 schrieb Jason Ekstrand ja...@jlekstrand.net: On Sep 28, 2014 11:49 AM, Karsten Otto karsten.o...@posteo.de wrote: From: Philip Withnall phi...@tecnocode.co.uk This happens if the socket has been gracefully closed. [KAO: It prevents a potential infinite loop when using a different event handling mechanism than epoll, if said mechanism cannot distinguish EOF from regular read (e.g. select).] --- src/wayland-server.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/wayland-server.c b/src/wayland-server.c index 674aeca..85741cb 100644 --- a/src/wayland-server.c +++ b/src/wayland-server.c @@ -260,7 +260,7 @@ wl_client_connection_data(int fd, uint32_t mask, void *data) len = 0; if (mask WL_EVENT_READABLE) { len = wl_connection_read(connection); - if (len 0 errno != EAGAIN) { + if (len == 0 || (len 0 errno != EAGAIN)) { wl_client_destroy(client); return 1; It seems to me as if we should push this up the pipe to wl_connection_read so we fix it server-side too. What do you mean? Calling wl_client_destroy within wl_client_read? Or making sure the connection is properly disposed of in both client and server code? Regarding the first, you would have to pass the wl_client reference to the read function, which would mix API layers unnecessarily. wl_client_read is basically just a convenient wrapper around recvmsg, so it should expose the same result semantics of 0 (OK), == 0 (EOF), and 0 (error), as it already does. It is up to the caller to handle the cases, if only to provide different log messages for error and EOF. Regarding the second, wl_connection_read is part of the wayland private/internal API, and only used in two places: wayland-client.c, which already handles EOF in an appropriate manner, and wayland-server.c, where the proposed patch handles the missing case as well. If anybody else needs to use wl_connection_read within the wayland libraries in the future, they are hopefully aware of the recv result semantics. If necessary, maybe add some documentation to wayland-private.h. What I meant was to just make wl_connection_read return something consistent ( 0 on EOF). That said, if we're doing it this way in wayland-client, then I'm fine with it. I didn't actually go look. --Jason Ekstrand } -- 1.9.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 weston 13/16] compositor: Add a function to test if images transformed by a matrix should be bilinearly filtered
On Fri, Oct 3, 2014 at 8:21 AM, Derek Foreman der...@osg.samsung.com wrote: On 02/10/14 07:09 PM, Jason Ekstrand wrote: On Thu, Oct 2, 2014 at 3:21 PM, Derek Foreman der...@osg.samsung.com mailto:der...@osg.samsung.com wrote: On 02/10/14 02:37 PM, Jason Ekstrand wrote: On Oct 2, 2014 12:37 AM, Pekka Paalanen ppaala...@gmail.com mailto:ppaala...@gmail.com mailto:ppaala...@gmail.com mailto:ppaala...@gmail.com wrote: On Wed, 1 Oct 2014 18:09:32 -0700 Jason Ekstrand ja...@jlekstrand.net mailto:ja...@jlekstrand.net mailto:ja...@jlekstrand.net mailto:ja...@jlekstrand.net wrote: Allow me to chip in here. Sorry that I haven't had a chance to really look over things carefully. I have been reading this thread, just haven't had a chance to respond. On Wed, Oct 1, 2014 at 12:41 AM, Pekka Paalanen ppaala...@gmail.com mailto:ppaala...@gmail.com mailto:ppaala...@gmail.com mailto:ppaala...@gmail.com wrote: On Tue, 30 Sep 2014 14:35:24 -0500 Derek Foreman der...@osg.samsung.com mailto: der...@osg.samsung.com mailto:der...@osg.samsung.com mailto:der...@osg.samsung.com wrote: Thanks for taking a look! On 26/09/14 05:48 PM, Bill Spitzak wrote: 90 degree rotation about x or y will require filtering. Yup, you're right. You test y scale twice, must be a typo. I think you intended to test z, but in fact z scale is not relevant so you should not test it at all. Argh - thanks. Why isn't Z scale relevant? I'm worried about making assumptions about the transformations these matrices represent and having those assumptions violated in the future... For Z not to matter are we assuming projection will always be orthographic? Weston never uses the final Z coordinate for anything, so in that sense it is always orthographic. Essentially, we could just do with 3x3 matrices perfectly fine. 3x3 supports 2D-projective which is enough to implement fake-3D effects like http://people.collabora.com/~pq/rotate3d-fun.webm (The gl-renderer does not route the W element at all, I had to patch that. Pixman-renderer OTOH just worked.) Weston also hardcodes the input Z coordinate always to 0, no matter which way you are going between buffer and output spaces. I suppose the 4x4 matrix was originally chosen to fit the OpenGL API. And maybe with some speculation about a desktop cube implementation or something, but I don't really see the cube or such coming, not as a generic thing anyway as only the gl-renderer could support it with true 3D space. Translation by non-integer will also require filtering. Good point. I recommend instead of checking the rotation to instead look for zeros in the correct locations in the matrix. Matrix must be of the form: |sx 0 0 tx| |0 sy 0 ty| |? ? ? ?| |0 0 0 1| or |0 sx 0 tx| |sy 0 0 ty| |? ? ? ?| |0 0 0 1| sx and sy must be ą1, and tx and ty must be integers. The ? can be any value. That could save us the very expensive matrix decomposition. I'll try this out. Thanks. I think this may be better than decomposition for deciding to use video planes in compositor-drm as well. (In fact, you've got me wondering if we ever need to split a matrix into basic transformations at all...) I'd be wondering about that, too. My intuition would say there is no need to really decompose. Just checking the elements should suffice, and when the matrix is supportable for whatever, then you pick the right elements (which is a bit like decomposition, but no need to be generic at all). Yeah, I'm not convinced we need to be able to do a full decomposition either. My original intention was something like this: bool weston_matrix_to_integer_transform(const weston_matrix *mat, enum wl_output_transform transform, int *scale, int *x, int *y) Why would there be 'transform' parameter? That implies that the matrix is not really the total transformation, which I find odd here. (Total transformation is between
Re: [RFC weston 13/16] compositor: Add a function to test if images transformed by a matrix should be bilinearly filtered
On Oct 2, 2014 12:37 AM, Pekka Paalanen ppaala...@gmail.com wrote: On Wed, 1 Oct 2014 18:09:32 -0700 Jason Ekstrand ja...@jlekstrand.net wrote: Allow me to chip in here. Sorry that I haven't had a chance to really look over things carefully. I have been reading this thread, just haven't had a chance to respond. On Wed, Oct 1, 2014 at 12:41 AM, Pekka Paalanen ppaala...@gmail.com wrote: On Tue, 30 Sep 2014 14:35:24 -0500 Derek Foreman der...@osg.samsung.com wrote: Thanks for taking a look! On 26/09/14 05:48 PM, Bill Spitzak wrote: 90 degree rotation about x or y will require filtering. Yup, you're right. You test y scale twice, must be a typo. I think you intended to test z, but in fact z scale is not relevant so you should not test it at all. Argh - thanks. Why isn't Z scale relevant? I'm worried about making assumptions about the transformations these matrices represent and having those assumptions violated in the future... For Z not to matter are we assuming projection will always be orthographic? Weston never uses the final Z coordinate for anything, so in that sense it is always orthographic. Essentially, we could just do with 3x3 matrices perfectly fine. 3x3 supports 2D-projective which is enough to implement fake-3D effects like http://people.collabora.com/~pq/rotate3d-fun.webm (The gl-renderer does not route the W element at all, I had to patch that. Pixman-renderer OTOH just worked.) Weston also hardcodes the input Z coordinate always to 0, no matter which way you are going between buffer and output spaces. I suppose the 4x4 matrix was originally chosen to fit the OpenGL API. And maybe with some speculation about a desktop cube implementation or something, but I don't really see the cube or such coming, not as a generic thing anyway as only the gl-renderer could support it with true 3D space. Translation by non-integer will also require filtering. Good point. I recommend instead of checking the rotation to instead look for zeros in the correct locations in the matrix. Matrix must be of the form: |sx 0 0 tx| |0 sy 0 ty| |? ? ? ?| |0 0 0 1| or |0 sx 0 tx| |sy 0 0 ty| |? ? ? ?| |0 0 0 1| sx and sy must be ±1, and tx and ty must be integers. The ? can be any value. That could save us the very expensive matrix decomposition. I'll try this out. Thanks. I think this may be better than decomposition for deciding to use video planes in compositor-drm as well. (In fact, you've got me wondering if we ever need to split a matrix into basic transformations at all...) I'd be wondering about that, too. My intuition would say there is no need to really decompose. Just checking the elements should suffice, and when the matrix is supportable for whatever, then you pick the right elements (which is a bit like decomposition, but no need to be generic at all). Yeah, I'm not convinced we need to be able to do a full decomposition either. My original intention was something like this: bool weston_matrix_to_integer_transform(const weston_matrix *mat, enum wl_output_transform transform, int *scale, int *x, int *y) Why would there be 'transform' parameter? That implies that the matrix is not really the total transformation, which I find odd here. (Total transformation is between buffer pixel coords and output/scanout pixel coords, i.e. buffer-to-output.) I'm sorry I mistyped but I meant the transform to be an output parameter. That way you know of the matrix is a 90-degree rotation or flip. Not sure if this is needed but for figuring out GL_LINEAR vs GL_NEAREST we don't want to fail if there is a 90-degree rotation. (do we use bool in weston? Maybe just return int). We may need both x and y scales and it may be useful to get those as floats. I'm not sure on that. Pekka, what would the RPi backend use? The rpi-renderer uses pretty much the same as what DRM planes/overlays offer wrt. coordinates, IIRC: integer position and size on the output, and 16.16 fixed point position and size on input (buffer). Whether scaling factor is integer or not is irrelevant there. I do not recall there being an option for sampling (nearest/linear/...) in either DispmanX nor DRM. Basically, we want to be able to do 2 things: First, detect if it's an entirely integer transform and use GL_NEAREST instead of GL_LINEAR and second, know how to flip the surface around in cases when we can do some simple transformations but can't do an arbitrary matrix transformation. One example here is DRM planes. We can only use a plane when there's no scale and the buffer-to-output transform has no rotation. We need to check for that condition and then pull the needed data out. I
Re: [RFC weston 13/16] compositor: Add a function to test if images transformed by a matrix should be bilinearly filtered
On Thu, Oct 2, 2014 at 3:21 PM, Derek Foreman der...@osg.samsung.com wrote: On 02/10/14 02:37 PM, Jason Ekstrand wrote: On Oct 2, 2014 12:37 AM, Pekka Paalanen ppaala...@gmail.com mailto:ppaala...@gmail.com wrote: On Wed, 1 Oct 2014 18:09:32 -0700 Jason Ekstrand ja...@jlekstrand.net mailto:ja...@jlekstrand.net wrote: Allow me to chip in here. Sorry that I haven't had a chance to really look over things carefully. I have been reading this thread, just haven't had a chance to respond. On Wed, Oct 1, 2014 at 12:41 AM, Pekka Paalanen ppaala...@gmail.com mailto:ppaala...@gmail.com wrote: On Tue, 30 Sep 2014 14:35:24 -0500 Derek Foreman der...@osg.samsung.com mailto:der...@osg.samsung.com wrote: Thanks for taking a look! On 26/09/14 05:48 PM, Bill Spitzak wrote: 90 degree rotation about x or y will require filtering. Yup, you're right. You test y scale twice, must be a typo. I think you intended to test z, but in fact z scale is not relevant so you should not test it at all. Argh - thanks. Why isn't Z scale relevant? I'm worried about making assumptions about the transformations these matrices represent and having those assumptions violated in the future... For Z not to matter are we assuming projection will always be orthographic? Weston never uses the final Z coordinate for anything, so in that sense it is always orthographic. Essentially, we could just do with 3x3 matrices perfectly fine. 3x3 supports 2D-projective which is enough to implement fake-3D effects like http://people.collabora.com/~pq/rotate3d-fun.webm (The gl-renderer does not route the W element at all, I had to patch that. Pixman-renderer OTOH just worked.) Weston also hardcodes the input Z coordinate always to 0, no matter which way you are going between buffer and output spaces. I suppose the 4x4 matrix was originally chosen to fit the OpenGL API. And maybe with some speculation about a desktop cube implementation or something, but I don't really see the cube or such coming, not as a generic thing anyway as only the gl-renderer could support it with true 3D space. Translation by non-integer will also require filtering. Good point. I recommend instead of checking the rotation to instead look for zeros in the correct locations in the matrix. Matrix must be of the form: |sx 0 0 tx| |0 sy 0 ty| |? ? ? ?| |0 0 0 1| or |0 sx 0 tx| |sy 0 0 ty| |? ? ? ?| |0 0 0 1| sx and sy must be ±1, and tx and ty must be integers. The ? can be any value. That could save us the very expensive matrix decomposition. I'll try this out. Thanks. I think this may be better than decomposition for deciding to use video planes in compositor-drm as well. (In fact, you've got me wondering if we ever need to split a matrix into basic transformations at all...) I'd be wondering about that, too. My intuition would say there is no need to really decompose. Just checking the elements should suffice, and when the matrix is supportable for whatever, then you pick the right elements (which is a bit like decomposition, but no need to be generic at all). Yeah, I'm not convinced we need to be able to do a full decomposition either. My original intention was something like this: bool weston_matrix_to_integer_transform(const weston_matrix *mat, enum wl_output_transform transform, int *scale, int *x, int *y) Why would there be 'transform' parameter? That implies that the matrix is not really the total transformation, which I find odd here. (Total transformation is between buffer pixel coords and output/scanout pixel coords, i.e. buffer-to-output.) btw, what exactly is the buffer-to-output transform? I think in the pixman renderer that's already calculated in a convenient location (in matrix in repaint_region() We create it here, among other things: https://github.com/ManMower/weston/blob/transforms/src/pixman-renderer.c#L213 Basically, it's just the full transformation from buffer pixels to output pixels. If that's not scaled or rotated, we want NEAREST filtering. For gl-renderer, I suspect I need to build it myself in draw_view(): weston_matrix_init(foo); weston_matrix_multiply(foo, ev-surface-buffer_to_surface_matrix); if (ev-transform.enabled) weston_matrix_multiply(foo, ev-transform.matrix) weston_matrix_multiply(foo, output-matrix); Is that right? Do I have the order backwards? I'd like to test just that one matrix and no additional if (ev-tranform.enabled) etc to decide on whether to use linear or nearest... I'm sorry I mistyped but I meant the transform to be an output
Re: [RFC weston 13/16] compositor: Add a function to test if images transformed by a matrix should be bilinearly filtered
Allow me to chip in here. Sorry that I haven't had a chance to really look over things carefully. I have been reading this thread, just haven't had a chance to respond. On Wed, Oct 1, 2014 at 12:41 AM, Pekka Paalanen ppaala...@gmail.com wrote: On Tue, 30 Sep 2014 14:35:24 -0500 Derek Foreman der...@osg.samsung.com wrote: Thanks for taking a look! On 26/09/14 05:48 PM, Bill Spitzak wrote: 90 degree rotation about x or y will require filtering. Yup, you're right. You test y scale twice, must be a typo. I think you intended to test z, but in fact z scale is not relevant so you should not test it at all. Argh - thanks. Why isn't Z scale relevant? I'm worried about making assumptions about the transformations these matrices represent and having those assumptions violated in the future... For Z not to matter are we assuming projection will always be orthographic? Weston never uses the final Z coordinate for anything, so in that sense it is always orthographic. Essentially, we could just do with 3x3 matrices perfectly fine. 3x3 supports 2D-projective which is enough to implement fake-3D effects like http://people.collabora.com/~pq/rotate3d-fun.webm (The gl-renderer does not route the W element at all, I had to patch that. Pixman-renderer OTOH just worked.) Weston also hardcodes the input Z coordinate always to 0, no matter which way you are going between buffer and output spaces. I suppose the 4x4 matrix was originally chosen to fit the OpenGL API. And maybe with some speculation about a desktop cube implementation or something, but I don't really see the cube or such coming, not as a generic thing anyway as only the gl-renderer could support it with true 3D space. Translation by non-integer will also require filtering. Good point. I recommend instead of checking the rotation to instead look for zeros in the correct locations in the matrix. Matrix must be of the form: |sx 0 0 tx| |0 sy 0 ty| |? ? ? ?| |0 0 0 1| or |0 sx 0 tx| |sy 0 0 ty| |? ? ? ?| |0 0 0 1| sx and sy must be ±1, and tx and ty must be integers. The ? can be any value. That could save us the very expensive matrix decomposition. I'll try this out. Thanks. I think this may be better than decomposition for deciding to use video planes in compositor-drm as well. (In fact, you've got me wondering if we ever need to split a matrix into basic transformations at all...) I'd be wondering about that, too. My intuition would say there is no need to really decompose. Just checking the elements should suffice, and when the matrix is supportable for whatever, then you pick the right elements (which is a bit like decomposition, but no need to be generic at all). Yeah, I'm not convinced we need to be able to do a full decomposition either. My original intention was something like this: bool weston_matrix_to_integer_transform(const weston_matrix *mat, enum wl_output_transform transform, int *scale, int *x, int *y) (do we use bool in weston? Maybe just return int). We may need both x and y scales and it may be useful to get those as floats. I'm not sure on that. Pekka, what would the RPi backend use? Basically, we want to be able to do 2 things: First, detect if it's an entirely integer transform and use GL_NEAREST instead of GL_LINEAR and second, know how to flip the surface around in cases when we can do some simple transformations but can't do an arbitrary matrix transformation. One example here is DRM planes. We can only use a plane when there's no scale and the buffer-to-output transform has no rotation. We need to check for that condition and then pull the needed data out. Point is, we don't need a full matrix decomposition. Also, it's worth throwing out there that the caching probably doesn't help us at all because we're going to usually be calling this on freshly computed matrices such as the above mentioned buffer-to-output transform. Does that make sense? --Jason And you take take advantage of the fact that incoming Z=0 and outgoing Z is ignored, I believe. That is, for the final, total transformation matrix between buffer and output spaces. Yes, we can vastly simplify things by taking that into account. Basically, you can throw away the 2nd row and column and not care about them at all. Also pixman is already doing equivalent tests and short-circuiting to nearest filter, so you might not need to check this anyway. This is also used for the gl renderer, so I don't think I can count on that short circuit there... Though bilinear vs nearest doesn't have anywhere near the same performance impact there. Yeah, we need to pick nearest vs. linear in the gl-renderer ourselves. Even if the choice did not affect the outcome, choosing nearest when we can might save memory bandwidth. I hope. And then we have the accuracy problems of GL, so picking
Re: PATCH] Fix new_id(n) arg parse from va list
While that looks correct, I'm sure it breaks something. As the person who wrote that code, that choice was very intentional. Even though the protocol type is uint32_t, new_id arguments are always passed into libwayland as either a wl_proxy or wl_resource and that's what this code is designed to handle. --Jason On Tue, Sep 30, 2014 at 8:45 AM, Jasper St. Pierre jstpie...@mecheye.net wrote: What is this trying to solve? On Mon, Aug 18, 2014 at 11:12 AM, Maks Naumov maksq...@ukr.net wrote: Signed-off-by: Maks Naumov maksq...@ukr.net --- src/connection.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/connection.c b/src/connection.c index f292853..13e3a3e 100644 --- a/src/connection.c +++ b/src/connection.c @@ -492,7 +492,7 @@ wl_argument_from_va_list(const char *signature, union wl_argument *args, args[i].o = va_arg(ap, struct wl_object *); break; case 'n': - args[i].o = va_arg(ap, struct wl_object *); + args[i].n = va_arg(ap, uint32_t); break; case 'a': args[i].a = va_arg(ap, struct wl_array *); -- 1.9.1 ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel -- Jasper ___ 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 1/4] connection: Fix sendmsg() on BSD systems
On Sep 28, 2014 6:54 PM, Bill Spitzak spit...@gmail.com wrote: On 09/28/2014 11:49 AM, Karsten Otto wrote: + msg.msg_control = NULL; + msg.msg_controllen = 0; msg.msg_flags = 0; + /* Only set msg_control when sending ancillary data */ + if (clen 0) { + msg.msg_controllen = clen; + msg.msg_control = cmsg; + } How about: msg.msg_controllen = clen; msg.msg_control = clen ? cmsg : NULL; msg.msg_flags = 0; Actually, why don't we just memset the entire structure but yes, Bill's suggestion is decent too. ___ 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 4/4] wayland-server: Abort if a read from a client gives 0 length
On Sep 28, 2014 11:49 AM, Karsten Otto karsten.o...@posteo.de wrote: From: Philip Withnall phi...@tecnocode.co.uk This happens if the socket has been gracefully closed. [KAO: It prevents a potential infinite loop when using a different event handling mechanism than epoll, if said mechanism cannot distinguish EOF from regular read (e.g. select).] --- src/wayland-server.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/wayland-server.c b/src/wayland-server.c index 674aeca..85741cb 100644 --- a/src/wayland-server.c +++ b/src/wayland-server.c @@ -260,7 +260,7 @@ wl_client_connection_data(int fd, uint32_t mask, void *data) len = 0; if (mask WL_EVENT_READABLE) { len = wl_connection_read(connection); - if (len 0 errno != EAGAIN) { + if (len == 0 || (len 0 errno != EAGAIN)) { wl_client_destroy(client); return 1; It seems to me as if we should push this up the pipe to wl_connection_read so we fix it server-side too. } -- 1.9.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: xdg shell status and gaps
On Sep 25, 2014 3:04 PM, Jasper St. Pierre jstpie...@mecheye.net wrote: On Thu, Sep 25, 2014 at 3:58 PM, Bill Spitzak spit...@gmail.com wrote: On 09/25/2014 01:57 PM, Jasper St. Pierre wrote: https://github.com/magcius/weston/commit/c1e5a846f4f57400bca1262111f9793e451c5b49 That patch has nothing to do with what is needed. You don't need a modal window type. This is trivial for a client to do by just pretending that whatever keystrokes it gets go to the modal window even if the compositor sends them to a different window. What is needed is a non-flickering and atomic method of creating that modal window atop the main one and keeping it there. That requires a parent, not a modal flag. (well actually it does not require a parent if instead there was a way to atomically map and rearrange a set of surfaces, but I think the parent will be much easier and matches what programmers are familiar with). You mean like the existing set_parent request that has been there since xdg-shell has landed? http://cgit.freedesktop.org/wayland/weston/tree/protocol/xdg-shell.xml#n140 If parenting isn't sufficient, it would be easy enough to add a mutter extension for modal windows in the form of another state enum. If it turns out to be a well-defined thing other compositors want, we can think about moving it to core. In any case, it should be trivial from a protocol point of view. -- Jasper ___ 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: xdg shell status and gaps
On Fri, Sep 26, 2014 at 7:38 AM, Jasper St. Pierre jstpie...@mecheye.net wrote: On Fri, Sep 26, 2014 at 4:07 AM, Carlos Garnacho carl...@gnome.org wrote: Hey, On Thu, Sep 25, 2014 at 7:30 PM, Matthias Clasen matthias.cla...@gmail.com wrote: On Thu, Sep 25, 2014 at 10:32 AM, Jasper St. Pierre jstpie...@mecheye.net wrote: Anyway, here's the list: snip 7) Root window drop When is this useful? One place where it is used is when dragging tabs out of a window to create a new window. gnome-terminal, gedit, nautilus, all do this. But looking closer, the way it is implemented is not actually as a root window drop, specifically, but just any failed drop - if you don't drop on a notebook in the same app that is hooked up to accept the tab, we just treat any failed drop as a change to create a new window. So, what we need is a signal that a drop failed. Not sure we get that, currently ? There isn't. IMO it'd be more generally useful notifying about the drag being finished. The client that started the drag should already know which mimetype was requested. It'd also help retrofit toolkits to wayland, how higher layers in GTK+ rely on getting the full press/motion.../release events in the drag source client can't be easily circumvented, having a consistent ending point for the operation would help lots. Although... if drag failed animations are to be performed by the compositor, I think we actually want the drag operation accepted in some form, or we'd confusingly get both things happening. For root window drops at least, maybe application/x-rootwindow-drop from XDND could be reused, and have the compositor peer on the wl_data_device so it accepts that mimetype. One of the other things we need, and was on the to-do list a long time ago, was to support move / copy DND emblems so that some more information can be transferred about the context of a drag. I'm not sure if this should be sent by the source, or can be done entirely by the destination client. How does this work under XDND? It would be good to talk to Giulio Camuffo (who I just added to the CC) about this. He was working on trying to get Qt's drag-and-drop support to work some time ago and I think he ran in to similar issues. --Jason ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH V2] wayland.xml: add enum, bitfield and is_bitfield attributes
On Tue, Sep 23, 2014 at 8:17 AM, Nils Chr. Brause nilschrbra...@gmail.com wrote: On Tue, Sep 23, 2014 at 8:35 AM, Pekka Paalanen ppaala...@gmail.com wrote: On Tue, 23 Sep 2014 13:59:39 +0800 Boyan Ding stu_...@126.com wrote: On Mon, 2014-09-22 at 11:27 -0700, Jason Ekstrand wrote: I'm a little unsure. I think trying to completely solve this problem in a way that will truly make strongly typed languages happy is insanity. That said, I'm cautiously ok with defining bitfields and enums as long as we are very careful in scoping what bitfield and enum mean. A bitfield should have only power of two values and the result should always be interpreted as an OR of those values. An enum should have every possible value enumerated. If anyone has a good example of something that validly doesn't fit into either of these, please speak up. Originally, the idea to distinct between enumerations and bitfields was that the user should be able to apply bitwise logic operations, where it might be useful (everything I marked with is_bitfield=true) and not be able to do this where this doesn't make sense (everywhere else). Therefore, everything should fall in either of these categories. Either it makes sense to use bitwise logic operations or it doesn't. In this case, what is the point of making the distinction? Why not just specify everything as a bitfield? What do you gain from knowing that two things will never be ORed together? The example of wl_output.transform is an enum because every possibility is enumerated. From C or a similar language, you can do fun stuff like if (transform WL_OUTPUT_TRANSFORM_FLIPPED) to determine if there is a flip. In a strongly typed language, you can't do this and we shouldn't bend over backwards to make it possible. If With the logic explained above, this would fall in my bitfield category. Having more than the power-of-two values listed isn't a problem in my opinion. I see these extra values as some sort of shortcuts for often used OR-combinations. The strongly-typed part here only means that you can't compare enums/bitfields of different type or pass type B, where type A is expected. Therefore this can very well be done in a strongly typed language. As an example have a look at my C++ bindings: https://github.com/NilsBrause/waylandpp Especially at this part of the example: https://github.com/NilsBrause/waylandpp/blob/master/example/test.cpp#L194 we try and come up with some convoluted system that makes this possible with typed languages, we're going to cause far more pain than it's worth. One other thing that we need to keep in mind here is the primary target audience of Wayland and its libraries. That audience is compositors and toolkits. Most of those are written in C and C++. What we don't want to do is to do a bunch of things for the sake of 1% of the target audience that makes the rest have to bend over backwards. When I said cautiously OK, I mean that I don't see that happening yet and I don't see a valid use for an enum that doesn't follow one of those two rules. However, if we have a plausible case where doing so would make everyone's lives easier, I'm going to not be a big fan. Please note that I'm not trying to insult Haskel or other functional or strongly typed languages or the people who use them. I'm simply trying to be pragmatic and recognize that people who want to write an app in haskel that manually bangs the Wayland protocol isn't the target audience. I understand, that the target audience of Wayland are high-level toolkits and compositors, but I don't agree, that this is a good idea. If you only want to create an EGL window and handle some input, there is no need for a huge toolkit like Qt, in my opinion. A small binding library for the programming language of your choice is much more useful and less wasteful in terms of memory and performance. --Jason Completely agree with Jason here. Actually the present enum are just mnemotic to 32-bit int or uint values (and that's why the type field in current protocol is int or uint instead of things like enum). Though developers of strongly-typed languages may not like it, it is versatile and makes sense. Any attempt to define what an enum or bitfield is will change the current semantics and introduce a lot of complexity. There may exist a way to make it work perfectly (and I'm okay with it if it really exists), but I doubt whether the effort worth it since it doesn't do any good to C or C++ wayland programmers, who are the main targeted audience of wayland. Probably not for C programmers, but it can very well be useful for C++ programmers. C++ has scoped enumerations since C++11, which turns an enumeration into a distinct type. My C++ bindings are using this feature for every enumeration that is not a bitfield
Re: [PATCH weston 1/3] Introduce pointer lock interface
Hi Matthieu, Could you please provide more explanation on what use-cases you are considering and why the current proposal fails to accomplish them? All I was able to get from your post was the example of a TV remote. It's all well and good to completely disagree with the proposed approach, but without a good reason as to why it doesn't work, it's kind of moot. It's also worth noting the scope of the proposal. The primary objective here is to provide a mechanism to take a device that could be providing relative events but which the compositor has turned into a pointer and get the relative events out of it. It's not intended to be a generic system for getting relative events. On that note, if you want to get a generic relative motion events, can't you just open the device and read them? Sure we could cobble together a specification for how to read a bunch of buttons and relative events and then create a cursor from them. I'm pretty sure it exists in the form of the USB HID spec. How does reimplementing that in Wayland help us? There was, at one point, a proposal for gamepads, but nothing has happened there in a while. On Tue, Sep 23, 2014 at 9:01 AM, Matthieu Gautier d...@mgautier.fr wrote: Hello, I'm pretty new into wayland and the discussion is relatively long, so I may have missed arguments/constraints. However I would like to share my point of view. It seems to me that we are taking the problem the wrong way. Relative motions exist as soon as there is a device generating them. wl_pointer is just a particular interpretation of those events. In fact, we may have a system where we have relative motion events but no wl_pointer. Think about a smart tv with a remote control with accelerator/gyroscope detectors. This remote may behave as a mouse, generating relative motion events. But the main interface of the TV may have no pointer. The interface should been a set of icons and user move between them with the remote buttons. If you're just pressing buttons to move between icons, then pointer is probably the wrong way to look at it anyway. It's more arrow keys than a pointer. In the same way, we may want that special applications still have access to motion events: - A web browser that will display itself the pointer (or activate wl_pointer in the compositor) - A video game - Any application that want make gesture recognition. In this context, wl_pointer is a special use-case of a shell and having a mouse device doesn't imply having a pointer. Relative motions should be always available (if there is a device) and wl_pointer should be created on top of relative motions. Trying to reduce the wl_pointer behavior to have the raw events seems to me the contrary of what we have to do. What I propose is : - Having a way to get relative input object (lets call it wl_relative for now) from wl_seat. - Having a way to get a wl_pointer from the wl_seat at it is already the case. Relative events a sent to client if it is active (It is to the compositor to decide this, as usual) whatever there is a wl_pointer or not. The pointer lock interface will become some kind of deactivate/configure wl_pointer. # Functionally : A combination of : - Hide the cursor (already available with wl_pointer.set_cursor) - Don't not update wl_pointer position from relative events. - Confine the pointer position into my wl_surface. - Set wl_pointer at this position. - A fps game will hide the cursor and deactivate update of wl_pointer position. - A strategy game will just confine the pointer. - A application with a 3D view that what to rotate it when user drag the mouse will just deactivate update of pointer position between button_down and button_up. - A application that just want relative motion events do nothing. At any time, relative motion events are sent to client through the wl_relative object. Regardless of the state of wl_pointer. It is up to the client to handle events from wl_pointer or wl_relative depending of which kind of information it wants. # Interface : The wl_pointer could gain two (four?) more requests : - set_mode(mode, callback) - reset_mode() ( - has_mode - get_mode ) The default mode is the mode we have for now (no special constraints) A client can change the mode of a wl_pointer. It gets a callback. When compositor stops the special mode (or refuse it) the done event of the callback is sent. When the client has finished with special mode, it sends the reset_mode request. The wl_pointer.leave event may or not be sent to client when the done event is sent (The pointer may still being inside the wl_surface when special mode ends) However a wl_pointer.leave event implies a done event. (We cannot have a special mode if we don't have the pointer focus) On the interface to get the wl_relative object from seat, it depends : Is there a possibility to have several cursor on one seat ? One cursor per seat. If there are
Re: [PATCH weston 1/3] Introduce pointer lock interface
Bill, That's an interesting idea, but there are a few problems (which may be solvable). I do kind of like the way it completely sidesteps the acceleration issue. On Tue, Sep 23, 2014 at 12:43 PM, Bill Spitzak spit...@gmail.com wrote: In any case this all sounds excessively complicated. I think this will work: - Client can create a pointer_lock object from an wl_pointer. This takes a serial of an event so the compositor can decide whether it should be allowed by the event. - Motion and push/release events still come in IDENTICAL to how they were before, except it acts as though the surface is infinite in size. This is to make it easier for toolkits to handle. There is no need for relative events as it is trivial for the toolkit to subtract the starting position. Acceleration should be unchanged. This won't quite work. There is no such thing as an infinite datatype with which to express these absolute positions. It's tempting to say just use float or double but those quickly loose precision as you go further off center. We could allow the wl_fixed values to wrap around and trust in subtraction, but I don't know how well that will work. Also, this still leaves the issue of how do you handle multiple pointer objects. This doesn't completely solve it at all. For instance, suppose I'm playing an FPS game and I look upper-right and click to fire. The pointer position being reported may be right over my close button. If the toolkit still thinks it's getting regular pointer events, then we have a problem. - The client will only get an enter event if it has gotten a leave event before pointer lock is granted (this is probably impossible on most compositors). There is no difference between this enter event and a normal one. - I don't think the client will get a leave event until pointer lock is cancelled, and only if the cursor ends up outside the surface. - Tablets may change to a relative mode. There's a race here. How does the client know that it got relative events and not absolute? - The cursor stops moving. A new api is added (maybe to the pointer_lock object) that allows the cursor to be positioned. The coordinate system is exactly the same one the mouse events are using. Are the events the client gets after it sets the pointer starting from the newly set position? If so, then how do we solve the inherent race of the client not knowing where in the pointer event stream that happened? If not, then how does the toolkit with another wl_pointer object (other than the relative one) handle it properly? - The client can change the cursor image (including making it invisible) using the same api as always. - The client cancels pointer lock by destroying the pointer lock object. Depending on the hardware this will either make the pointer move to the last cursor position, or move the cursor to the absolute pointer position. Either of these may cause leave, enter, and move events. - The compositor can break pointer lock by sending a special event. This may be the only event that the pointer_lock object can produce. When the client gets it it should destroy the pointer lock object. I don't think it should be called leave and it has nothing to do with enter/leave. Examples: Game where pointer position controls things: client requests pointer lock on activation, and possibly on enter events. A smarter client can wait until the mouse hovers a bit before grabbing it. Client then hides the cursor and uses the relative events until the game ends or pointer lock is lost. Restrict motion to the surface: client acts like the game, but does not hide the cursor. Instead on all motion it clamps the motion to the allowed area and places the cursor there, and acts like the motion is to that point. Slow scrollbar: on push client requests pointer lock. It then scales down the motion, moving it's scrollbar and the cursor the same so they still look locked together. On release it destroys the pointer lock. Eyedropper color picker: client requests pointer lock after user enters color picker mode. It then moves the cursor to match the pointer position. On push/release it uses a privledged api to pick the color off the screen at that location. Depending on the app it may drop pointer lock at this point, or stay in color picker mode. Tumble 3D controller: Client requests pointer lock on push of the shift key and destroys it on release of the shift key. It probably hides the cursor. It then computes tumble on all movements while the mouse is down. I don't see why the above proposal doesn't also solve these cases. --Jason ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH V2] wayland.xml: add enum, bitfield and is_bitfield attributes
On Mon, Sep 22, 2014 at 9:30 AM, Nils Chr. Brause nilschrbra...@gmail.com wrote: On Mon, Sep 22, 2014 at 11:21 AM, Pekka Paalanen ppaala...@gmail.com wrote: On Wed, 3 Sep 2014 19:44:04 +0200 Nils Chr. Brause nilschrbra...@gmail.com wrote: This replaces [PATCH wayland] Add enum attribute to arg elements. Previous concers have be incorporated and it is meant for 1.7. From 733fb18b163de93276f092a4be100c7e0c0c723f Mon Sep 17 00:00:00 2001 From: Nils Chr. Brause nilschrbra...@googlemail.com Date: Wed, 3 Sep 2014 19:18:28 +0200 Subject: [PATCH] wayland.xml: add enum, bitfield and is_bitfield attributes There are programming languages, that are more strongly typed than C. People creating Wayland bindings for these languages will want to make use of this strong type system by declaring each enumeration a distinct type and differentiating between enumerations and bit fields. For code generation to work with these languages, there needs to be information about which enumerations are actually bit fields and which enumerations or bit fields may be passed/received in which request/event arguments. This is accomplished by adding an is_bitfield attribute to enum elements, which are actually bitfields and by adding enum or bitfield attributes to the arg elements of requests and events. The values of the enum and bitfield attributes have the format $interface.$name, where $interface is the name of the interface, where the enumeration or bit field is declared and $name is the name of the enumeration or bit field, which is used in this argument. The scanner has been modified to enforce those rules. --- protocol/wayland.dtd | 3 ++ protocol/wayland.xml | 42 ++--- src/scanner.c| 101 ++- 3 files changed, 124 insertions(+), 22 deletions(-) diff --git a/protocol/wayland.dtd b/protocol/wayland.dtd index b8b1573..4506201 100644 --- a/protocol/wayland.dtd +++ b/protocol/wayland.dtd @@ -14,6 +14,7 @@ !ELEMENT enum (description?,entry*) !ATTLIST enum name CDATA #REQUIRED !ATTLIST enum since CDATA #IMPLIED + !ATTLIST enum is_bitfield CDATA #IMPLIED !ELEMENT entry (description?) !ATTLIST entry name CDATA #REQUIRED !ATTLIST entry value CDATA #REQUIRED @@ -25,5 +26,7 @@ !ATTLIST arg summary CDATA #IMPLIED !ATTLIST arg interface CDATA #IMPLIED !ATTLIST arg allow-null CDATA #IMPLIED + !ATTLIST arg enum CDATA #IMPLIED + !ATTLIST arg bitfield CDATA #IMPLIED !ELEMENT description (#PCDATA) !ATTLIST description summary CDATA #REQUIRED diff --git a/protocol/wayland.xml b/protocol/wayland.xml index bb457bc..878c347 100644 --- a/protocol/wayland.xml +++ b/protocol/wayland.xml @@ -229,7 +229,7 @@ arg name=width type=int/ arg name=height type=int/ arg name=stride type=int/ - arg name=format type=uint/ + arg name=format type=uint enum=wl_shm.format/ /request request name=destroy type=destructor @@ -367,7 +367,7 @@ can be used for buffers. Known formats include argb and xrgb. /description - arg name=format type=uint/ + arg name=format type=uint enum=wl_shm.format/ /event /interface @@ -723,7 +723,7 @@ arg name=serial type=uint summary=serial of the implicit grab on the pointer/ /request -enum name=resize +enum name=resize is_bitfield=true description summary=edge values for resizing These values are used to indicate which edge of a surface is being dragged in a resize operation. The server may @@ -751,7 +751,7 @@ /description arg name=seat type=object interface=wl_seat summary=the wl_seat whose pointer is used/ arg name=serial type=uint summary=serial of the implicit grab on the pointer/ - arg name=edges type=uint summary=which edge or corner is being dragged/ + arg name=edges type=uint summary=which edge or corner is being dragged bitfield=wl_shell_surface.resize/ /request request name=set_toplevel @@ -762,7 +762,7 @@ /description /request -enum name=transient +enum name=transient is_bitfield=true description summary=details of transient behaviour These flags specify details of the expected behaviour of transient surfaces. Used in the set_transient request. @@ -784,7 +784,7 @@ arg name=parent type=object interface=wl_surface/ arg name=x type=int/ arg name=y type=int/ - arg name=flags type=uint/ + arg name=flags type=uint bitfield=wl_shell_surface.transient/ /request enum name=fullscreen_method @@ -835,7 +835,7 @@ with the dimensions for the
Re: [PATCH weston 1/3] Introduce pointer lock interface
On Sun, Sep 21, 2014 at 11:43 PM, Pekka Paalanen ppaala...@gmail.com wrote: On Sat, 20 Sep 2014 11:43:22 -0700 Jason Ekstrand ja...@jlekstrand.net wrote: On Sat, Sep 20, 2014 at 1:29 AM, Pekka Paalanen ppaala...@gmail.com wrote: On Fri, 19 Sep 2014 20:20:53 +0200 Jonas Ådahl jad...@gmail.com wrote: On Fri, Sep 19, 2014 at 11:00:21AM -0600, Jasper St. Pierre wrote: On Fri, Sep 19, 2014 at 2:54 AM, Pekka Paalanen ppaala...@gmail.com wrote: non-sensible requests (set_cursor) on the additional wl_pointer, and potentially more as wl_pointer is extended. The alternatives that has come up in IRC discussions and backlog reading are: 1) Create a wl_relative_pointer that would replace wl_pointer, i.e. have its own button, axis, etc events. This would solve all problems related to versioning and wl_pointer being extended, but brings a bunch of other problems for example we'd have to extend wl_relative_pointer each time wl_pointer is extended to keep up, as well as the issue where a wl_pointer is used in a interface or request. Either you use wl_pointer and get extensions automatically even if you don't want to, or you use a new interface that you have to extend explicitly if you want the same as in wl_pointer. Two sides of a coin. As concluded in IRC, wl_pointer is not used as argument in any messages. But it is by wl_pointer_lock.lock_pointer, and who says it'll be the only place its used as an argument? I think wl_seat might be a better argument. After all, all wl_pointer objects created from the same wl_seat in the same way will be identical. I think we should keep it that way, too. That's a decent idea. We could simply have wl_pointer_lock.get_relative_pointer(seat) which creates a relative pointer. At that point, all the absolute pointers get a leave. The client, if it wants, can create multiple relative pointers. Once all of them are destroyed, the absolute pointers get an enter again, and off we go. Mind, you are saying that simply a client creating the wl_relative_pointer will never get normal wl_pointer events as long as a wl_relative_pointer object exists. Is this a deliberate design choice? You seem to contradict it right below here with the leave events. One note though: I don't think we want to try too hard to make multiple pointers per seat seamless. That's going to be an awkward case no matter what we do. We do need to make it well-defined. I think that having all the absolute pointers get a leave is a reasonable way to do that. If the compositor has to break the lock, all the relative pointers should get a leave and absolute motion resumes. (It probably won't get an absolute enter right away. The use probably went to an expose or alt-tab view so it probably would have gotten an absolute leave anyway.) Yes, that is a logical consequence of the all wl_* input device type objects for the same wl_seat and created the same way are identical. :-) It's actually a pretty logical implication from the globals: binding several times to a specific global name creates identical objects that all refer to the same thing, and act the same way at the same time. This might be a good design rule to set, don't you think? Hmm. Maybe we need to lock pointer given a seat instead. It'd change the mode of the underlying pointer to be locked in its position, i.e. not send any wl_pointer.motion events. Locking could then be ref counted, so that as long as more than one wl_relative_pointer is alive, it's in relative mode. Maybe thats unnecessarily complex though. We are distinguishing wl_relative_pointer exists from pointer-lock is active, right? A question is: how does the client know when pointer-lock is active? I think we'd need something like wl_relative_pointer.enter/leave. I'm still a fan of destroying relative pointers when not in use. It's a bit more protocol churn, but it makes it clear when they are/are not active. Maybe enter/leave will work, but I'm not convinced it's the right way forward. I'll have to give that a bit more thought before I could give a real solid argument though. Yes, of course. It's more a matter of how does the client know to destroy the wl_relative_pointer, when the compositor chooses to deactivate the pointer-lock. If there is a leave event for that, good. I think with this, it could actually make sense what you said above: no normal wl_pointer events as long as any wl_relative_pointer exists. This would strongly push app devs to destroy the wl_relative_pointer at the right time, I believe. And then we need a backup plan for clients that do not destroy it, for protocol definition completeness. If the server sends a leave
Re: [PATCH V2] wayland.xml: add enum, bitfield and is_bitfield attributes
On Mon, Sep 22, 2014 at 12:48 PM, Jasper St. Pierre jstpie...@mecheye.net wrote: On Mon, Sep 22, 2014 at 12:27 PM, Jason Ekstrand ja...@jlekstrand.net wrote: On Mon, Sep 22, 2014 at 9:30 AM, Nils Chr. Brause nilschrbra...@gmail.com wrote: On Mon, Sep 22, 2014 at 11:21 AM, Pekka Paalanen ppaala...@gmail.com wrote: On Wed, 3 Sep 2014 19:44:04 +0200 Nils Chr. Brause nilschrbra...@gmail.com wrote: This replaces [PATCH wayland] Add enum attribute to arg elements. Previous concers have be incorporated and it is meant for 1.7. From 733fb18b163de93276f092a4be100c7e0c0c723f Mon Sep 17 00:00:00 2001 From: Nils Chr. Brause nilschrbra...@googlemail.com Date: Wed, 3 Sep 2014 19:18:28 +0200 Subject: [PATCH] wayland.xml: add enum, bitfield and is_bitfield attributes There are programming languages, that are more strongly typed than C. People creating Wayland bindings for these languages will want to make use of this strong type system by declaring each enumeration a distinct type and differentiating between enumerations and bit fields. For code generation to work with these languages, there needs to be information about which enumerations are actually bit fields and which enumerations or bit fields may be passed/received in which request/event arguments. This is accomplished by adding an is_bitfield attribute to enum elements, which are actually bitfields and by adding enum or bitfield attributes to the arg elements of requests and events. The values of the enum and bitfield attributes have the format $interface.$name, where $interface is the name of the interface, where the enumeration or bit field is declared and $name is the name of the enumeration or bit field, which is used in this argument. The scanner has been modified to enforce those rules. --- protocol/wayland.dtd | 3 ++ protocol/wayland.xml | 42 ++--- src/scanner.c| 101 ++- 3 files changed, 124 insertions(+), 22 deletions(-) diff --git a/protocol/wayland.dtd b/protocol/wayland.dtd index b8b1573..4506201 100644 --- a/protocol/wayland.dtd +++ b/protocol/wayland.dtd @@ -14,6 +14,7 @@ !ELEMENT enum (description?,entry*) !ATTLIST enum name CDATA #REQUIRED !ATTLIST enum since CDATA #IMPLIED + !ATTLIST enum is_bitfield CDATA #IMPLIED !ELEMENT entry (description?) !ATTLIST entry name CDATA #REQUIRED !ATTLIST entry value CDATA #REQUIRED @@ -25,5 +26,7 @@ !ATTLIST arg summary CDATA #IMPLIED !ATTLIST arg interface CDATA #IMPLIED !ATTLIST arg allow-null CDATA #IMPLIED + !ATTLIST arg enum CDATA #IMPLIED + !ATTLIST arg bitfield CDATA #IMPLIED !ELEMENT description (#PCDATA) !ATTLIST description summary CDATA #REQUIRED diff --git a/protocol/wayland.xml b/protocol/wayland.xml index bb457bc..878c347 100644 --- a/protocol/wayland.xml +++ b/protocol/wayland.xml @@ -229,7 +229,7 @@ arg name=width type=int/ arg name=height type=int/ arg name=stride type=int/ - arg name=format type=uint/ + arg name=format type=uint enum=wl_shm.format/ /request request name=destroy type=destructor @@ -367,7 +367,7 @@ can be used for buffers. Known formats include argb and xrgb. /description - arg name=format type=uint/ + arg name=format type=uint enum=wl_shm.format/ /event /interface @@ -723,7 +723,7 @@ arg name=serial type=uint summary=serial of the implicit grab on the pointer/ /request -enum name=resize +enum name=resize is_bitfield=true description summary=edge values for resizing These values are used to indicate which edge of a surface is being dragged in a resize operation. The server may @@ -751,7 +751,7 @@ /description arg name=seat type=object interface=wl_seat summary=the wl_seat whose pointer is used/ arg name=serial type=uint summary=serial of the implicit grab on the pointer/ - arg name=edges type=uint summary=which edge or corner is being dragged/ + arg name=edges type=uint summary=which edge or corner is being dragged bitfield=wl_shell_surface.resize/ /request request name=set_toplevel @@ -762,7 +762,7 @@ /description /request -enum name=transient +enum name=transient is_bitfield=true description summary=details of transient behaviour These flags specify details of the expected behaviour of transient surfaces. Used in the set_transient request. @@ -784,7 +784,7 @@ arg name=parent type=object interface=wl_surface/ arg name=x type=int/ arg name=y type=int/ - arg name=flags type=uint/ + arg
Re: [PATCH weston 1/3] Introduce pointer lock interface
On Sat, Sep 20, 2014 at 12:58 AM, Pekka Paalanen ppaala...@gmail.com wrote: On Fri, 19 Sep 2014 12:46:20 -0700 Jason Ekstrand ja...@jlekstrand.net wrote: On Fri, Sep 19, 2014 at 11:20 AM, Jonas Ådahl jad...@gmail.com wrote: On Fri, Sep 19, 2014 at 11:00:21AM -0600, Jasper St. Pierre wrote: On Fri, Sep 19, 2014 at 2:54 AM, Pekka Paalanen ppaala...@gmail.com wrote: Or what if an app manually starts pointer-lock because the toolkit does not support it? This is not a use case to design for at all, but if it happens to work, it might be a nice bonus. If we make it so that locking simply locks the cursor at its position, it'd will just work, as the pointer cursor wouldn't move. No unexpected wierd wl_pointer.motion events. I don't see any problem with locking only one of the available pointers. The absolute ones simply get no motion until it's unlocked. If we add a set_position request to wl_relative_pointer, then I guess that would trigger motion on the absolute pointers. I think it's probably fine. Note, that it won't be motion. It'll be a leave/enter pair, because the change in position does not come from an input device, but from software (a client). Seems ugly to me. Ok, I think I see what you mean. More details in the next email (trying to slow down the thread explosion) I think that the best would be to simply not change the position or state of the cursor at all. Locking would mean that while locked, the wl_pointer has focus, meaning the client can change the cursor, or hide it. For example if pointer-locking would be used to click-drag to select items from a list (where dragging changes highlighted element), it would be bad if the cursor suddenly jumps to the middle of the surface when done. Staying where it was at the point of locking also works the same way as pointer locking does in GLFW. Haven't checked SDL yet. What would a set_cursor_position argument do? It sounds like that could be more or less used to implement actual warping by just locking, set position then unlocking. It'd probably be more useful to have a set_cursor_position request in wl_relative_pointer that can be used when implementing pointer warping in Xwayland and then possibly even SDL and GLFW. Hmm. I'm not 100% sure what you're getting at here. However, I think we do want to have cases where the client is drawing a warped pointer and then the the pointer-lock breaks. One example would be a VM that uses pointer_lock to give relative events to the client machine. In that case, when the you move the pointer to the edge of the window, it sets the pointer position and breaks the lock. This way it looks like the pointer is simply moving through the window. RTS games will probably want to do similar things. I don't think just lock it where it is is always going to work. Yup, this was the reason for the set-position for cursor in pointer-locked mode. Without such request, the position should stay put, not warp to middle of window or anything. Thanks, pq ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH weston 1/3] Introduce pointer lock interface
On Sat, Sep 20, 2014 at 1:29 AM, Pekka Paalanen ppaala...@gmail.com wrote: On Fri, 19 Sep 2014 20:20:53 +0200 Jonas Ådahl jad...@gmail.com wrote: On Fri, Sep 19, 2014 at 11:00:21AM -0600, Jasper St. Pierre wrote: On Fri, Sep 19, 2014 at 2:54 AM, Pekka Paalanen ppaala...@gmail.com wrote: On Fri, 19 Sep 2014 08:33:13 +0200 Jonas Ådahl jad...@gmail.com wrote: On Thu, Sep 18, 2014 at 10:43:47AM -0700, Jason Ekstrand wrote: On Thu, Sep 18, 2014 at 2:26 AM, Pekka Paalanen ppaala...@gmail.com wrote: On Wed, 17 Sep 2014 22:35:40 +0200 Jonas Ådahl jad...@gmail.com wrote: You could create a wl_pointer, if you do it from interface whose ancestor is wl_seat, or directly a request in wl_seat. But it does have downsides like not being able to let the extension mature in Weston before moving it to Wayland core. I think its a good practice to have a isolateable kind of way of extending protocols (ala wl_relative_pointer, wl_scaler, etc) so that core protocol objects don't grow too large with non-removable request and events. Indeed. Maybe we need to define modes of operation that would be similar to roles (e.g. xdg_surface), so that wl_relative_pointer changes the mode of wl_pointer to wl_relative_pointer. This would allow us to add a wl_pointer_lock.confine_pointer that doesn't make sense to enable at the same time as the pointer is in relative motion mode. Roles are not really modes. Some roles do affect the wl_surface state somewhat, but they do not seriously alter the behaviour of wl_surface requests or events. Having relative vs. absolute modes on a wl_pointer would, though. I'd rather avoid modes where we can, especially on protocol objects. The underlying thing may have modes where it chooses which protocol object it targets, which is a bit less confusing than switching what a protocol object does. Agreed. Every time one object modifies another it's a lot of documentation chasing every time you want to understand it, and it's really easy to get completely wrong. non-sensible requests (set_cursor) on the additional wl_pointer, and potentially more as wl_pointer is extended. The alternatives that has come up in IRC discussions and backlog reading are: 1) Create a wl_relative_pointer that would replace wl_pointer, i.e. have its own button, axis, etc events. This would solve all problems related to versioning and wl_pointer being extended, but brings a bunch of other problems for example we'd have to extend wl_relative_pointer each time wl_pointer is extended to keep up, as well as the issue where a wl_pointer is used in a interface or request. Either you use wl_pointer and get extensions automatically even if you don't want to, or you use a new interface that you have to extend explicitly if you want the same as in wl_pointer. Two sides of a coin. As concluded in IRC, wl_pointer is not used as argument in any messages. But it is by wl_pointer_lock.lock_pointer, and who says it'll be the only place its used as an argument? I think wl_seat might be a better argument. After all, all wl_pointer objects created from the same wl_seat in the same way will be identical. I think we should keep it that way, too. That's a decent idea. We could simply have wl_pointer_lock.get_relative_pointer(seat) which creates a relative pointer. At that point, all the absolute pointers get a leave. The client, if it wants, can create multiple relative pointers. Once all of them are destroyed, the absolute pointers get an enter again, and off we go. One note though: I don't think we want to try too hard to make multiple pointers per seat seamless. That's going to be an awkward case no matter what we do. We do need to make it well-defined. I think that having all the absolute pointers get a leave is a reasonable way to do that. If the compositor has to break the lock, all the relative pointers should get a leave and absolute motion resumes. (It probably won't get an absolute enter right away. The use probably went to an expose or alt-tab view so it probably would have gotten an absolute leave anyway.) 3) Extend wl_pointer similar to how wl_viewport extends wl_surface with a wl_relative_pointer (different from in (1)). Such a wl_relative_pointer would, to begin with, only have one request (release) and one event (motion). It wouldn't change anything to the wl_pointer object, it'd still receive button, axis etc events, but motion events are turned into relative motion events via wl_relative_pointer, not affecting the position of the cursor, i.e. not resulting in wl_pointer.motion events. This is an interesting idea. I wonder how it works if the client has two parts (say, libraries or toolkits
Re: [PATCH weston 1/3] Introduce pointer lock interface
On Fri, Sep 19, 2014 at 11:20 AM, Jonas Ådahl jad...@gmail.com wrote: On Fri, Sep 19, 2014 at 11:00:21AM -0600, Jasper St. Pierre wrote: On Fri, Sep 19, 2014 at 2:54 AM, Pekka Paalanen ppaala...@gmail.com wrote: On Fri, 19 Sep 2014 08:33:13 +0200 Jonas Ådahl jad...@gmail.com wrote: On Thu, Sep 18, 2014 at 10:43:47AM -0700, Jason Ekstrand wrote: On Thu, Sep 18, 2014 at 2:26 AM, Pekka Paalanen ppaala...@gmail.com wrote: On Wed, 17 Sep 2014 22:35:40 +0200 Jonas Ådahl jad...@gmail.com wrote: On Wed, Sep 17, 2014 at 11:16:06PM +0300, Giulio Camuffo wrote: 2014-09-17 23:11 GMT+03:00 Jonas Ådahl jad...@gmail.com: On Wed, Sep 17, 2014 at 10:56:13PM +0300, Giulio Camuffo wrote: I haven't looked at the implementation yet, just at the protocol, but isn't _wl_pointer_lock.lock_pointer() returning a new wl_pointer a problem? Objects should have a unique factory interface, or else the version of the interface can't be determined. Is it really? In the implementation below, the wl_pointer object gets the same version as the wl_pointer object that is locked. It is also specified in the last paragraph of _wl_pointer_lock.lock_pointer. Mmh, then maybe it is fine. I'm not convinced actually, but I'm too tired now. :) No, it's not fine. You cannot define exceptions to the versioning rules in a protocol spec. We have common code in libwayland handling all runtime versioning, and you just cannot implement any exceptions. Technically, those haven't been merged yet... So, technically, we could have it inherit the version from the wl_pointer. That said, I think we'll regret if we do. Even if we did do that, it would cause problems, not because we couldn't update wl_pointer, but because we really couldn't update wl_pointer_lock. All in all, it's a bad plan. So, from what I've heard here now is it's clear we don't want to create a new wl_pointer object. Other than the versioning issue, we'd have You could create a wl_pointer, if you do it from interface whose ancestor is wl_seat, or directly a request in wl_seat. But it does have downsides like not being able to let the extension mature in Weston before moving it to Wayland core. I think its a good practice to have a isolateable kind of way of extending protocols (ala wl_relative_pointer, wl_scaler, etc) so that core protocol objects don't grow too large with non-removable request and events. Maybe we need to define modes of operation that would be similar to roles (e.g. xdg_surface), so that wl_relative_pointer changes the mode of wl_pointer to wl_relative_pointer. This would allow us to add a wl_pointer_lock.confine_pointer that doesn't make sense to enable at the same time as the pointer is in relative motion mode. Maybe, but that sounds like a problem to solve with the second extension of wl_pointer, not the first. Right now, we don't have any other extensions flying around, so we don't know how it will interact with these other hypothetical extensions. non-sensible requests (set_cursor) on the additional wl_pointer, and potentially more as wl_pointer is extended. The alternatives that has come up in IRC discussions and backlog reading are: 1) Create a wl_relative_pointer that would replace wl_pointer, i.e. have its own button, axis, etc events. This would solve all problems related to versioning and wl_pointer being extended, but brings a bunch of other problems for example we'd have to extend wl_relative_pointer each time wl_pointer is extended to keep up, as well as the issue where a wl_pointer is used in a interface or request. Either you use wl_pointer and get extensions automatically even if you don't want to, or you use a new interface that you have to extend explicitly if you want the same as in wl_pointer. Two sides of a coin. As concluded in IRC, wl_pointer is not used as argument in any messages. But it is by wl_pointer_lock.lock_pointer, and who says it'll be the only place its used as an argument? 2) Add begin and end events to wl_pointer_lock that changes the state of an existing wl_pointer. After a wl_pointer_lock.begin is sent, wl_pointer.motion sends relative motion events. After wl_pointer_lock.end it'd revert back to sending absolute motion events. This might be confusing if a client creates several wl_pointer objects for the same seat underlying a wl_seat. Apart from being confusing just by changing the meaning of an event via a mode. 3) Extend wl_pointer similar to how wl_viewport extends wl_surface with a wl_relative_pointer (different from in (1
Re: [PATCH 3/3] client: cancel read in wl_display_read_events() when last_error is set
I took a look at it too and it looks good to me --Jason On Thu, Sep 11, 2014 at 1:46 AM, Pekka Paalanen ppaala...@gmail.com wrote: On Wed, 10 Sep 2014 12:47:14 +0200 Marek Chalupa mchqwe...@gmail.com wrote: Calling wl_display_read_events() after an error should be equivalent to wl_display_cancel_read(), so that display state is consistent. Thanks to Pekka Paalanen pekka.paala...@collabora.co.uk for pointing that out. Signed-off-by: Marek Chalupa mchqwe...@gmail.com --- src/wayland-client.c | 13 ++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/src/wayland-client.c b/src/wayland-client.c index 1b7a046..b0f77b9 100644 --- a/src/wayland-client.c +++ b/src/wayland-client.c @@ -1192,6 +1192,14 @@ read_events(struct wl_display *display) return 0; } +static void +cancel_read(struct wl_display *display) +{ + display-reader_count--; + if (display-reader_count == 0) + display_wakeup_threads(display); +} + /** Read events from display file descriptor * * \param display The display context object @@ -1219,6 +1227,7 @@ wl_display_read_events(struct wl_display *display) pthread_mutex_lock(display-mutex); if (display-last_error) { + cancel_read(display); pthread_mutex_unlock(display-mutex); errno = display-last_error; @@ -1365,9 +1374,7 @@ wl_display_cancel_read(struct wl_display *display) { pthread_mutex_lock(display-mutex); - display-reader_count--; - if (display-reader_count == 0) - display_wakeup_threads(display); + cancel_read(display); pthread_mutex_unlock(display-mutex); } All three pushed. 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: [PATCH] pixman-backend: add support for zooming
On Mon, Sep 1, 2014 at 2:37 AM, Pekka Paalanen ppaala...@gmail.com wrote: On Fri, 29 Aug 2014 23:35:37 -0700 Jason Ekstrand ja...@jlekstrand.net wrote: On Aug 29, 2014 6:25 PM, Derek Foreman der...@osg.samsung.com wrote: On 29/08/14 06:42 PM, Jason Ekstrand wrote: Derek, I haven't had a chance to look that hard at or play with this patch yet. Hopefully I can look at it before too long. One quick question though: Have you verified that this works with all of the output transforms? I'm sorry if this seems premature but a) pixman renderer stuff is hard to get right and b) people are constantly breaking the pixman renderer with respect to output transforms. So I thought the question was worth asking even if I haven't had time to review yet. Ouch - I am now guilty of 'b' That's OK. I, for one, can never get pixman changes right on the first try. That's why I asked. ;-) We'd really need to hook pixman renderer into the headless backend, let the test extension do screenshots, and write some tests to test all the output transforms... it's a big task, but would be hugely useful and allow to write more tests that can check real on-screen results. I've been dreaming about that for a while. Transforms do indeed break - the zoom translation is being applied in the wrong direction for some, and the clip region needs to be rotated for others. FWIW, I have a series on my github that accomplishes the same thing only with substantially deeper changes to Weston: https://github.com/jekstrand/weston/commits/wip/transforms Interesting... I think I can re-work my patch to handle rotations/flips pretty easily, though the code will bulk up a little. Is that worth the time, or would you prefer to move forward with yours? I think mine is ultimately a better way forward. The big advantage is that it provides a surface to/from buffer coordinates matrix and gets rid of all of the transform logic from the pixman renderer. Part of the reason the pixman renderer is always breaking is that it's a separate render/transform path from the go renderer and the two can get out of sync. Also, pixman does things more-or-less backwards from GL so it's hard to get right. My patches shouldn't need much of a rebase. The reason I never pushed them was that fixing zoom in the pixman renderer was never the end objective and I never considered the series really finished. However, if else want zoom+pixman, they are probably good to go. Oh yeah, finishing Jason's transformations revolution would be very good. I should probably take a quick look if someone wants to pick it up, since I don't recall if/what we agreed on the basic principles like weston_matrix vs. pixman matrix etc. Pixman is not the only rendering path that has to manually unravel and redo everything, rpi-renderer is the other one. Currently Weston's core has been written to support only the GL-renderer with its funny normalized GL coordinate spaces, which means the common transformation code (like output zoom handling) is practically useless for any other renderer that might (*gasp*) want to deal with pixel coordinates. You should look at my patches. They fix some of that. :-) Don't quite fix the rpi stuff though. I got a fair ways with those patches but blocked on a few things and then didn't have time to finish. Here's roughly what needs to be done: 1) Add region transform code that transforms a region based on a matrix. 2) Use said region transform code to do region transforms instead of the current ints+enum version 3) Write a function to detect if a matrix is a combination of integer translation, integer scale, 90-degree rotation, and possible flip. If it is, return the data needed to reconstruct it as such. 4) Use said function for a bunch of things; a) Inside of the GL renderer to determine whether GL_LINEAR or GL_NEAREST should be used b) Inside of the DRM backend to determine if something can go in a plane c) Maybe in the RPI renderer for some things? That's a rough sketch for what I had intended. Then there was stuff about damage coordinates but that's blocking on getting at least 2) if not 3). --Jason The current code also makes using hardware overlays in e.g. DRM-backend more of a hassle than it needs to, because there too you would like to have the end-to-end pixels-to-pixels transformation to program the overlays. So roughly the big idea would be that Weston core provides coordinates and transformations end-to-end from buffer to output in raw pixels, and if the renderer has something strange like GL, it should do the adaptation in its own code. I don't recall complaints about zoom on pixman renderer, but maybe that's because I've known it is broken. IMHO, working towards that testing thing or pushing Jason coordinate revolution forward would be time better used
Re: [PATCH] pixman-backend: add support for zooming
On Aug 29, 2014 6:25 PM, Derek Foreman der...@osg.samsung.com wrote: On 29/08/14 06:42 PM, Jason Ekstrand wrote: Derek, I haven't had a chance to look that hard at or play with this patch yet. Hopefully I can look at it before too long. One quick question though: Have you verified that this works with all of the output transforms? I'm sorry if this seems premature but a) pixman renderer stuff is hard to get right and b) people are constantly breaking the pixman renderer with respect to output transforms. So I thought the question was worth asking even if I haven't had time to review yet. Ouch - I am now guilty of 'b' That's OK. I, for one, can never get pixman changes right on the first try. That's why I asked. ;-) Transforms do indeed break - the zoom translation is being applied in the wrong direction for some, and the clip region needs to be rotated for others. FWIW, I have a series on my github that accomplishes the same thing only with substantially deeper changes to Weston: https://github.com/jekstrand/weston/commits/wip/transforms Interesting... I think I can re-work my patch to handle rotations/flips pretty easily, though the code will bulk up a little. Is that worth the time, or would you prefer to move forward with yours? I think mine is ultimately a better way forward. The big advantage is that it provides a surface to/from buffer coordinates matrix and gets rid of all of the transform logic from the pixman renderer. Part of the reason the pixman renderer is always breaking is that it's a separate render/transform path from the go renderer and the two can get out of sync. Also, pixman does things more-or-less backwards from GL so it's hard to get right. My patches shouldn't need much of a rebase. The reason I never pushed them was that fixing zoom in the pixman renderer was never the end objective and I never considered the series really finished. However, if else want zoom+pixman, they are probably good to go. --Jason Ekstrand On Aug 29, 2014 7:56 AM, Derek Foreman der...@osg.samsung.com wrote: Currently if you try to zoom with mod+scrollwheel the pixman backend will stop rendering anything at all and will continuously log pixman renderer does not support zoom, giving the impression that the server is hung. Instead, this patch adds the missing zoom functionality. This should close BUG 80258 --- src/pixman-renderer.c | 65 ++- 1 file changed, 59 insertions(+), 6 deletions(-) diff --git a/src/pixman-renderer.c b/src/pixman-renderer.c index 4fdcb05..55e8824 100644 --- a/src/pixman-renderer.c +++ b/src/pixman-renderer.c @@ -168,6 +168,37 @@ transform_apply_viewport(pixman_transform_t *transform, } static void +region_apply_zoom(struct weston_output *output, + pixman_region32_t *region) +{ + pixman_box32_t *rects, *zoomed_rects; + int nrects, i; + double mag, tx, ty, zoomw, zoomh; + + mag = 1.0 / (1.0 - output-zoom.spring_z.current); + zoomw = mag * output-width; + zoomh = mag * output-height; + tx = output-zoom.trans_x * output-width/2.0; + ty = output-zoom.trans_y * output-height/2.0; + rects = pixman_region32_rectangles(region, nrects); + zoomed_rects = calloc(nrects, sizeof(pixman_box32_t)); + + for (i = 0; i nrects; i++) { + zoomed_rects[i].x1 = (output-width / 2.0) + + mag * (rects[i].x1 - tx) - zoomw / 2.0; + zoomed_rects[i].x2 = (output-width / 2.0) + + mag * (rects[i].x2 - tx) - zoomw / 2.0; + zoomed_rects[i].y1 = (output-height / 2.0) + + mag * (rects[i].y1 - ty) - zoomh / 2.0; + zoomed_rects[i].y2 = (output-height / 2.0) + + mag * (rects[i].y2 - ty) - zoomh / 2.0; + } + pixman_region32_clear(region); + pixman_region32_init_rects(region, zoomed_rects, nrects); + free(zoomed_rects); +} + +static void repaint_region(struct weston_view *ev, struct weston_output *output, pixman_region32_t *region, pixman_region32_t *surf_region, pixman_op_t pixman_op) @@ -211,6 +242,10 @@ repaint_region(struct weston_view *ev, struct weston_output *output, /* Convert from global to output coord */ region_global_to_output(output, final_region); + /* Apply zoom if applicable */ + if (output-zoom.active) + region_apply_zoom(output, final_region); + /* And clip to it */ pixman_image_set_clip_region32 (po-shadow_image, final_region); @@ -218,6 +253,25 @@ repaint_region(struct weston_view *ev, struct weston_output *output, position, the output position/transform/scale
Re: [PATCH] pixman-backend: add support for zooming
Derek, I haven't had a chance to look that hard at or play with this patch yet. Hopefully I can look at it before too long. One quick question though: Have you verified that this works with all of the output transforms? I'm sorry if this seems premature but a) pixman renderer stuff is hard to get right and b) people are constantly breaking the pixman renderer with respect to output transforms. So I thought the question was worth asking even if I haven't had time to review yet. FWIW, I have a series on my github that accomplishes the same thing only with substantially deeper changes to Weston: https://github.com/jekstrand/weston/commits/wip/transforms --Jason Ekstrand On Aug 29, 2014 7:56 AM, Derek Foreman der...@osg.samsung.com wrote: Currently if you try to zoom with mod+scrollwheel the pixman backend will stop rendering anything at all and will continuously log pixman renderer does not support zoom, giving the impression that the server is hung. Instead, this patch adds the missing zoom functionality. This should close BUG 80258 --- src/pixman-renderer.c | 65 ++- 1 file changed, 59 insertions(+), 6 deletions(-) diff --git a/src/pixman-renderer.c b/src/pixman-renderer.c index 4fdcb05..55e8824 100644 --- a/src/pixman-renderer.c +++ b/src/pixman-renderer.c @@ -168,6 +168,37 @@ transform_apply_viewport(pixman_transform_t *transform, } static void +region_apply_zoom(struct weston_output *output, + pixman_region32_t *region) +{ + pixman_box32_t *rects, *zoomed_rects; + int nrects, i; + double mag, tx, ty, zoomw, zoomh; + + mag = 1.0 / (1.0 - output-zoom.spring_z.current); + zoomw = mag * output-width; + zoomh = mag * output-height; + tx = output-zoom.trans_x * output-width/2.0; + ty = output-zoom.trans_y * output-height/2.0; + rects = pixman_region32_rectangles(region, nrects); + zoomed_rects = calloc(nrects, sizeof(pixman_box32_t)); + + for (i = 0; i nrects; i++) { + zoomed_rects[i].x1 = (output-width / 2.0) + + mag * (rects[i].x1 - tx) - zoomw / 2.0; + zoomed_rects[i].x2 = (output-width / 2.0) + + mag * (rects[i].x2 - tx) - zoomw / 2.0; + zoomed_rects[i].y1 = (output-height / 2.0) + + mag * (rects[i].y1 - ty) - zoomh / 2.0; + zoomed_rects[i].y2 = (output-height / 2.0) + + mag * (rects[i].y2 - ty) - zoomh / 2.0; + } + pixman_region32_clear(region); + pixman_region32_init_rects(region, zoomed_rects, nrects); + free(zoomed_rects); +} + +static void repaint_region(struct weston_view *ev, struct weston_output *output, pixman_region32_t *region, pixman_region32_t *surf_region, pixman_op_t pixman_op) @@ -211,6 +242,10 @@ repaint_region(struct weston_view *ev, struct weston_output *output, /* Convert from global to output coord */ region_global_to_output(output, final_region); + /* Apply zoom if applicable */ + if (output-zoom.active) + region_apply_zoom(output, final_region); + /* And clip to it */ pixman_image_set_clip_region32 (po-shadow_image, final_region); @@ -218,6 +253,25 @@ repaint_region(struct weston_view *ev, struct weston_output *output, position, the output position/transform/scale and the client specified buffer transform/scale */ pixman_transform_init_identity(transform); + + if (output-zoom.active) { + double mag, zoomw, zoomh, tx, ty; + + mag = 1.0f - output-zoom.spring_z.current; + zoomw = mag * output-width; + zoomh = mag * output-height; + tx = output-zoom.trans_x * output-width/2.0 - (zoomw - output-width) / 2.0; + ty = output-zoom.trans_y * output-height/2.0 - (zoomh - output-height) / 2.0; + + pixman_transform_scale(transform, NULL, + pixman_double_to_fixed (mag), + pixman_double_to_fixed (mag)); + + pixman_transform_translate(transform, NULL, + pixman_double_to_fixed(tx), + pixman_double_to_fixed(ty)); + } + pixman_transform_scale(transform, NULL, pixman_double_to_fixed ((double)1.0/output-current_scale), pixman_double_to_fixed ((double)1.0/output-current_scale)); @@ -334,7 +388,8 @@ repaint_region(struct weston_view *ev, struct weston_output *output, pixman_image_set_transform(ps-image, transform); - if (ev-transform.enabled || output-current_scale != vp
Re: Self introduction
Welcome Matthieu, On Fri, Aug 29, 2014 at 8:40 AM, Matthieu Gautier d...@mgautier.fr wrote: Hello All, As usual, a small self introduction... I don't know that it's really usual on this list, but meh. My name is Matthieu Gautier, French developer of 30 years old. I'm a developer for years now and I've worked on a lot of projects related to embedded and graphics (video game, scientific visualization and digital TV). My last job was to work on a embedded platform allowing user to have several applications running simultaneously on its TV on top of the live video (pretty relative to Wayland isn't it ? :) ). I'm also a long term user of Free software and I'm Fedora ambassador for almost as long as I'm developer. And my irc nickname is starmad. Well, my small introduction made, let's speak about Wayland: I'm really interested in the wayland project and now than I've more free time, I would like to stop looking at the project and start being involve in it. Where am I? I've made the minimal : read the doc, downloaded, compiled and run wayland/weston. Played a little with everything and dived in the source code. Glad to hear you've got things up and running. Where can I go ? In a lot of directions. Where will I go ? Well.. Learn more about Wayland and Weston. To do so, I plan to search on bugzilla few easy fix (If you know some, don't hesitate), the best way to enter in a project. And probably ask you a lot of questions :) Well, if you want to jump right in, the testing guys at Intel just sent out an e-mail with a bunch of bugs. Pick one and try to squash it! If you're feeling more adventurous, I've got a project or two that I wouldn't mind someone picking up. (I haven't had time lately for much weston hacking.) --Jason Ekstrand ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH weston] shell: quit weston, if weston-desktop-shell dies early
Really, do we need a whole 30 seconds? You can easily do enough interaction to crash something in 30 seconds. Other than that, this looks fine. Reviewed-by: Jason Ekstrand jason.ekstr...@intel.com On Thu, Aug 28, 2014 at 1:48 AM, Pekka Paalanen ppaala...@gmail.com wrote: From: Pekka Paalanen pekka.paala...@collabora.co.uk If weston-desktop-shell dies soon after launch, or maybe cannot be executed at all, let weston exit rather than letting the user stare at a black screen. But, do not exit weston, if weston-desktop-shell dies later, as the user may already have apps open, and those apps would likely still function correctly. This gives the user the opportunity to save his work and close the apps properly. This should make one class of I see only black screen failures obvious. Signed-off-by: Pekka Paalanen pekka.paala...@collabora.co.uk --- desktop-shell/shell.c | 32 +++- desktop-shell/shell.h | 3 +++ 2 files changed, 34 insertions(+), 1 deletion(-) diff --git a/desktop-shell/shell.c b/desktop-shell/shell.c index c21d364..26f13cc 100644 --- a/desktop-shell/shell.c +++ b/desktop-shell/shell.c @@ -5298,6 +5298,32 @@ shell_surface_configure(struct weston_surface *es, int32_t sx, int32_t sy) } } +static bool +check_desktop_shell_crash_too_early(struct desktop_shell *shell) +{ + struct timespec now; + + if (clock_gettime(CLOCK_MONOTONIC, now) 0) + return false; + + /* +* If the shell helper client dies before the session has been +* up for roughly 30 seconds, better just make Weston shut down, +* because the user likely has no way to interact with the desktop +* anyway. +*/ + if (now.tv_sec - shell-startup_time.tv_sec 30) { + weston_log(Error: %s apparently cannot run at all.\n, + shell-client); + weston_log_continue(STAMP_SPACE Quitting...); + wl_display_terminate(shell-compositor-wl_display); + + return true; + } + + return false; +} + static void launch_desktop_shell_process(void *data); static void @@ -5340,7 +5366,9 @@ desktop_shell_client_destroy(struct wl_listener *listener, void *data) * returning. */ - respawn_desktop_shell_process(shell); + if (!check_desktop_shell_crash_too_early(shell)) + respawn_desktop_shell_process(shell); + shell_fade_startup(shell); } @@ -6407,5 +6435,7 @@ module_init(struct weston_compositor *ec, shell_fade_init(shell); + clock_gettime(CLOCK_MONOTONIC, shell-startup_time); + return 0; } diff --git a/desktop-shell/shell.h b/desktop-shell/shell.h index 67c5f50..2cfd1d6 100644 --- a/desktop-shell/shell.h +++ b/desktop-shell/shell.h @@ -23,6 +23,7 @@ */ #include stdbool.h +#include time.h #include compositor.h @@ -209,6 +210,8 @@ struct desktop_shell { enum desktop_shell_panel_position panel_position; char *client; + + struct timespec startup_time; }; struct weston_output * -- 1.8.5.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 weston] xdg-shell: Make stable
Jasper, I looked through your xdg-shell-rewrite branch and, other than some things that need better docs/rewording, I think it looks OK. In particular, I think we're probably OK removing set_unstable_version in 1.6. Things that need attention (some of this may be a repeat of what Pekka said): - The wording in the xdg_surface description about destruction is contradictory: Either it gets destroyed by the destroy request or by destroying the surface. Pick one (preferably the former). - Incorporate the discussion on maximized. I think we're pretty settled on how that should work now. - In set_parent, you say the parent is raised when the child is raised. I think you want that the other way around. Also, a little more description in general would be good. In particular, a note about destruction order (which I think you want) is better than vague wording about mappedness. Also, what are the symantics of reparenting? Is this tied to the commit in any way? - move: who is responsible for the cursor? Does the surface get a leave event and then the compositor set it? - resize: same question about cursors. Also, how does the client know the event has been ignored? Does it just not get a configure? Is this going to look to look terrible if the client and server are using different cursor themes? TBH, I don't know how solvable that last one is. - The wording on configure and ack_configure needs work. I think you need to be more explicit that when the client receives the configure it needs to repaint with the adjusted state and send an back along with the commit. Also be explicit that the ack_configure is double-buffered like other surface state. - Geometry: First, I think configure needs a see-also pointing to the set_geometry event. Also, this needs to be explicitly stated to be double-buffered with commit. I think popups look OK but I don't have much experience there. --Jason Ekstrand ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH weston] xdg-shell: Make stable
On Wed, Aug 27, 2014 at 1:52 AM, Giulio Camuffo giuliocamu...@gmail.com wrote: 2014-08-27 11:38 GMT+03:00 Pekka Paalanen ppaala...@gmail.com: On Wed, 27 Aug 2014 11:03:04 +0300 Giulio Camuffo giuliocamu...@gmail.com wrote: 2014-08-27 10:32 GMT+03:00 Pekka Paalanen ppaala...@gmail.com: On Tue, 26 Aug 2014 17:50:47 +0300 Giulio Camuffo giuliocamu...@gmail.com wrote: 2014-08-26 17:39 GMT+03:00 Jason Ekstrand ja...@jlekstrand.net: On Aug 26, 2014 1:01 AM, Giulio Camuffo giuliocamu...@gmail.com wrote: 2014-08-26 10:24 GMT+03:00 Pekka Paalanen ppaala...@gmail.com: On Mon, 25 Aug 2014 21:51:57 -0700 Jason Ekstrand ja...@jlekstrand.net wrote: On Aug 22, 2014 1:48 PM, Jasper St. Pierre jstpie...@mecheye.net wrote: On Wed, Aug 6, 2014 at 9:39 AM, Pekka Paalanen ppaala...@gmail.com wrote: On Thu, 17 Jul 2014 17:57:45 -0400 Jasper St. Pierre jstpie...@mecheye.net wrote: entry name=fullscreen value=2 summary=the surface is fullscreen The surface is fullscreen. The window geometry specified in the configure event must be obeyed by the client. Really? So, will we rely on wl_viewport for scaling low-res apps to fullscreen? No provision for automatic black borders in aspect ratio or size mismatch, even if the display hardware would be able to generate those for free while scanning out the client buffer, bypassing compositing? Since we have a big space for these states, I suppose we could do those mismatch cases in separate and explicit state variants of fullscreen, could we not? I explicitly removed this feature from the first draft of the patch simply to make my life easier as a compositor writer. We could add additional states for this, or break fullscreen into multiple states: fullscreen + size_strict or fullscreen + size_loose or something. I am not familiar enough with the sixteen different configurations of the old fullscreen system to make an informed decision about what most clients want. Your help and experience is very appreciated. I'm not keen to add back the matrix of configuration options. Yeah, not sure what to do here. I like the idea of the compositor doing it for the client. Sure, you could use wl_viewport, subsurfaces, and a couple black surfaces for letterboxing. However that is going to be far more difficult for the compositor to translate into overlays/planes than just the one surface and some scaling instructions. I don't think it would be that hard for a compositor to use overlays even then. Have one surface with a 1x1 wl_buffer, scaled with wl_viewport to fill the screen, and then have another surface on top with the video wl_buffers being fed in, scaled with wl_viewport to keep aspect ratio. A compositor can easily put the video on an overlay, and if the CRTC hardware supports, it might even eliminate the black surface and replace it with a background color. What is not clear to me is what advantages does the wl_subsurface+wl_viewport approach has compared to the compositor just scaling the surface and putting a black surface behind it. Is it just to remove the hint in the wl_fullscreen request? This seems a lazy reason to me, implementing that hint in the compositor is not hard, and in turn it means increasing the complexity of every client that would ever want to go fullscreen. There is also one use case for which the wl_subsurface+wl_viewport cannot work. Having the same surface fullscreen on two differently sized outputs (think of presentations), like this: http://im9.eu/picture/phx647 Sure, the compositor can send the configure event with one output's size and scale the surface to fit the other one, but then what is the purpose of wl_viewport again here, if the compositor must scale the surface anyway? Yeah, I think I still have to go with Guilio here. It really it's probably less work for the compositor to implement fullscreen modes than to implement viewport+subsurface correctly and it's certainly less work for clients. In my previous reply, I concluded that wl_viewport+wl_subsurface would be enough (I suprised myself), and we would not really need yet another way from xdg_surface. Obviously, I forgot something, that the IRC discussion of last night brought back to my mind. The wl_shell fullscreening has three different cases for dealing with size mismatch between the output and the window: - aspect-correct scaling - centered, no scaling - please, I would really like a mode switch