Skip to content
This repository has been archived by the owner on Nov 1, 2021. It is now read-only.

Add screencopy protocol #17

Merged
merged 9 commits into from Jul 28, 2018
Merged

Add screencopy protocol #17

merged 9 commits into from Jul 28, 2018

Conversation

emersion
Copy link
Member

@emersion emersion commented May 23, 2018

  • Capture outputs
  • Copy a region instead of the whole output
  • Capture views (waiting to know A DMA-BUF export protocol #11's approach) not yet
  • Capture other stuff (e.g. sway workspaces/containers)

Fixes #16

@emersion emersion changed the title Add first draft of the screencopy protocol Add screencopy protocol May 23, 2018
If the frame is successfully copied, a ready event is sent. Otherwise,
an abort event is sent.
</description>
<arg name="buffer" type="object" interface="wl_buffer"/>
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure we want to use a wl_buffer for this - maybe a FD would be more appropriate.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

wl_buffer seems fine to me

@ddevault
Copy link
Member

Copy a region instead of the whole output

I would like to see this get in, and I would like to see a timestamp added telling you the time the frame you got captured was presented.

@emersion emersion changed the title Add screencopy protocol [WIP] Add screencopy protocol Jun 23, 2018
</description>
<arg name="frame" type="new_id" interface="zwlr_screencopy_frame_v1"/>
<arg name="overlay_cursor" type="int"
summary="include custom client hardware cursor on top of the frame"/>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think we need to get so specific here. summary="composite cursor onto the frame"

@emersion emersion changed the title [WIP] Add screencopy protocol Add screencopy protocol Jun 23, 2018
@emersion
Copy link
Member Author

@ascent12 @atomnuker Comments welcome!

I'm not sure if the format and stride should be enforced by the compositor or should just be preferred.

@ddevault
Copy link
Member

I can't imagine a situation where I'm going to be willing to do format conversions in the compositor for the sake of a picky client.


The region is given in output logical coordinates, see
xdg_output.logical_size. Trying to capture a region spanning outside the
output extents is a protocol error.
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is racy against anything that changes logical size, resulting in a protocol error sounds sub-optimal to me.
The frame event has it's own size either way, is there a reason you don't want to clip it?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, I just wanted to avoid frames with no content. But you're right, it's a bad idea.

@emersion
Copy link
Member Author

I can't imagine a situation where I'm going to be willing to do format conversions in the compositor for the sake of a picky client.

That's true, but in the case of OpenGL, the real format used in the GPU buffer is opaque and driver-specific, so we need to do format conversions anyway. It does make more sense to force the client to use the format we pick though.

@emersion
Copy link
Member Author

Ping?

@ddevault
Copy link
Member

👍

Copy link
Contributor

@agx agx left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why are ready and flags two events? I couldn't figure that out from the commit message.

If there's a output transform in place would you return the transformed or untransformed output. Untransformed is IMHO o.k. since the client can get the transform from the output but it should be probably be noted in the protocol.

<description summary="manager to inform clients and begin capturing">
This object is a manager which offers requests to start capturing from a
source.
</description>

This comment was marked as resolved.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we differentiate this it would probably be at the export-dmabuf level, since the latter is more specific. As for screenshooter I think we can consider screenshooter obsoleted by this protocol.

@ghost
Copy link

ghost commented Jun 29, 2018

I don't get why you want clients to supply their own buffer. Just leave it to clients to get an allocated buffer and refcount it themselves if they have to.

@emersion
Copy link
Member Author

Just leave it to clients to get an allocated buffer

What do you mean?

Why are ready and flags two events? I couldn't figure that out from the commit message.

I wanted not to pollute the ready event with flags.

Untransformed is IMHO o.k. since the client can get the transform from the output but it should be probably be noted in the protocol

Yeah, that's how clients are supposed to do. Same for scale.

@agx
Copy link
Contributor

agx commented Jun 29, 2018 via email

@ghost
Copy link

ghost commented Jun 29, 2018

Can't clients directly get a wl_buffer without the request?

@emersion
Copy link
Member Author

Can't clients directly get a wl_buffer without the request?

No, they can't read a compositor-supplied wl_buffer. wl_buffer is designed to be client-side allocated.

@ddevault ddevault merged commit c99e4dd into swaywm:master Jul 28, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants