Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Expand X Render extension to be able to composite windows onto one another #11

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

MadLittleMods
Copy link
Contributor

@MadLittleMods MadLittleMods commented Sep 25, 2023

Expand X Render extension to be able to composite windows (or any "picture") onto one another.

Adds to the X Render extension xrender.zig:

  • query_pict_formats: Find matching picture formats in order to create a picture
  • create_picture: Create a picture from any drawable (like a window or pixmap) for use with the X Render extension
  • composite: Composite those created pictures together with a variety of operations

Testing strategy

$ zig run testexample.zig

Notice a small 100x100 snapshot of the root window is copied to our window

Why use composite over copy_area?

When dealing with a normal default situation where our window just inherits 24-bit color depth from the root window, you could also just use the copy_area command for this kind of functionality. But copy_area does not work when there is a depth mismatch, like when you change your window to 32-bit depth for transparency and try to copy from the 24-bit root window. For an example of this, see MadLittleMods/fps-aim-analyzer#5

I'll also briefly explain the motivation for using Xrender to access the window contents. It is perfectly possible to create a GC for a window and use XCopyArea() to copy the contents of the window if you want to use the core protocol, but since the Composite extension exposes new visuals (ones with alpha channels e.g.), there's no guarantee that the format of the source drawable will match that of the destination. With the core protocol that situation will result in a match error, something that won't happen with the Xrender extension.

In addition the core protocol has no understanding of alpha channels, which means that it can't composite windows that use the new ARGB visual. When the source and destination have the same format, there's also no performance advantage to using the core protocol as of X11R6.8. That release is also the first to support the new Composite extension.

So in conclusion there are no drawbacks, and only advantages to choosing Xrender over the core protocol for these operations.

-- https://www.talisman.org/~erlkonig/misc/x11-composite-tutorial/ (original location: http://ktown.kde.org/~fredrik/composite_howto.html)

Dev notes

@@ -184,15 +222,17 @@ pub fn main() !u8 {
}

const double_buf = try x.DoubleBuffer.init(
std.mem.alignForward(usize, 1000, std.mem.page_size),
std.mem.alignForward(usize, 8000, std.mem.page_size),
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Bumped to 8000 because the query_pict_formats response is big (4888 bytes on my system)

@MadLittleMods MadLittleMods marked this pull request as ready for review September 25, 2023 23:08
Copy link
Owner

@marler8997 marler8997 left a comment

Choose a reason for hiding this comment

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

Had a few comments but overall the changes look reasonable and really cool functionality you've added!


const font_dims: FontDims = blk: {
_ = try x.readOneMsg(conn.reader(), @alignCast(buf.nextReadBuffer()));
const message_length = try x.readOneMsg(conn.reader(), @alignCast(buf.nextReadBuffer()));
try checkMessageLengthFitsInBuffer(message_length, buffer_limit);
Copy link
Owner

Choose a reason for hiding this comment

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

I'm a little confused by this. Doesn't nextReadBuffer already limit the buffer to the maximum length? If not maybe that's a footgun that should be fixed? If we have to add an extra check after reading into a buffer in order to use it correctly, seems like there's a bigger underlying issue that needs addressed.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Here is what the doc comment suggests:

zigx/x.zig

Lines 2541 to 2545 in 74ffbcb

/// The caller must check whether the length returned is larger than the provided `buf`.
/// If it is, then only the first 32-bytes have been read. The caller can allocate a new
/// buffer large enough to accomodate and finish reading the message by copying the first
/// 32 bytes to the new buffer then calling `readOneMsgFinish`.
pub fn readOneMsg(reader: anytype, buf: []align(4) u8) !u32 {

testexample.zig Outdated Show resolved Hide resolved
MadLittleMods added a commit to MadLittleMods/zigx that referenced this pull request Dec 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants