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

x11: Add support for custom cursors #1801

Merged
merged 8 commits into from
Jun 15, 2021
Merged

Conversation

psychon
Copy link
Contributor

@psychon psychon commented May 22, 2021

This adds custom cursor support to the X11 backend, fixing an open todo from #1755.

Half of the added code is "RENDER initialisation" (is the necessary RENDER version supported? What is the id of the standard ARGB32 format?). The other half of the code "upload this image to the X11 server". I guess the later could be simplified somehow, but I was lazy and did it "manually". This could also be made faster, but I don't think creating cursors is performance critical...

Oh and: Nothing ever deletes custom cursors, so they stay around forever, eating memory in the X11 server. I wonder if one can make the X11 server go out-of-memory with cursors...

This code is completely untested since I can't build druid and thus cannot run the cursor example due to a too old Rust compiler.

Signed-off-by: Uli Schlachter <psychon@znc.in>
Signed-off-by: Uli Schlachter <psychon@znc.in>
@psychon
Copy link
Contributor Author

psychon commented May 22, 2021

Hm, I think the x11rb-image dependency is not actually needed here. That code doesn't actually do anything useful here. Future-me should double-check that and possibly remove the dependency. (Note to self)

@maan2003
Copy link
Collaborator

looks like channels are wrong. Also it is less clear
cursor example x11:
x11-cursor
cursor example gtk:
cursor-gtk

) -> Result<Cursor, ReplyOrIdError> {
// BEGIN: Lots of code just to get the image into a RENDER Picture

// No idea how to sanely get the pixel values, so I'll go with 'insane':
Copy link
Collaborator

Choose a reason for hiding this comment

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

ImageBuf::raw_pixels should work

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, but then I need to handle all the different possible ImageFormat values. The GTK backend does not do that (I guess it only handles two of the four possible values properly), so this seems complicated/untested.

And with raw_pixels, RgbaSeparate still needs to be converted to premultiplied "by hand". The simplest way for that could be the already existing code, I guess.

@psychon
Copy link
Contributor Author

psychon commented May 30, 2021

Updated. Here is a screenshot. On the left is the actual cursor and on the right is a screenshot of gimp showing the same image. I added some more colors to the image for testing:

Unbenannt

Seems like I got the component order completely wrong. And treating unpremultiplied pixels as premultiplied looks worse than I expected, so I also implemented that todo. The big-endian code is completely untested, but might work. Or not...

I have no idea which component order are required. xdpyinfo -ext RENDER here has nine picture formats with depth: 32. Some of them have no alpha component, some have depth 10 per color component. There are only two with 8 bits for each of r/g/b/a: argb and bgra. No idea if both of them are required, the spec only requires:

The server must support a Direct PictFormat with 8 bits each of red, green,
blue and alpha as well as a Direct PictFormat with 8 bits of red, green and
blue and 0 bits of alpha. The server must also support Direct PictFormats
with 1, 4 and 8 bits of alpha and 0 bits of r, g and b.

Also: Thanks a lot for the hint on how to get druid to work with my ancient Rust compiler.

While piet provides pixels in rgba order, the X11 server wants an u32 in
its native endianness with order argb. I even called a variable
argb32_format, but I still did not handle this difference.

This commit premultiplies the color component with the alpha, handles
the different component order and also converts to the server's
endianness.

Signed-off-by: Uli Schlachter <psychon@znc.in>
The code only used x11rb's image feature for splitting a large PutImage
request into smaller pieces. Since X11 servers usually have a maximum
request size of 16 MiB these days, it is unlikely that people create
cursors large enough to go above this limit. You'd need a 2k x 2k cursor
for that!

Signed-off-by: Uli Schlachter <psychon@znc.in>
Copy link
Collaborator

@maan2003 maan2003 left a comment

Choose a reason for hiding this comment

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

Looks good! the cursor example is working correctly

@psychon
Copy link
Contributor Author

psychon commented Jun 13, 2021

It just occurred to me that one could let piet-cairo and cairo handle the "complicated" part of uploading stuff to the X11 server in the right format. I briefly followed down this road, but when I figured out I'd need to call XCBSurface::create_with_xrender_format, I gave up. This requires more fun like the already existing xcb_visualtype_t hack that exists in the code: Basically providing a Rust translation of two more xcb C structs. Perhaps the existing code is good enough.

@maan2003
Copy link
Collaborator

I am in favor of "existing code is good enough"

@maan2003
Copy link
Collaborator

😔 conflict resolution failed

@psychon
Copy link
Contributor Author

psychon commented Jun 15, 2021

Here is my attempt at merging with master. I also added the use for TryInto in the merge commit (a886766 removed it).

I didn't test anything besides "it builds".

@maan2003 maan2003 merged commit f19ddf1 into linebender:master Jun 15, 2021
@psychon psychon deleted the x11-cursors branch June 15, 2021 16:25
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.

None yet

2 participants