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

Transparency in X11 and Wayland #2513

Merged
merged 4 commits into from
Jul 4, 2021
Merged

Conversation

elParaguayo
Copy link
Member

@elParaguayo elParaguayo commented Jun 6, 2021

This PR adds 32-bit colour depth for X11 which allows transparent windows (e.g. the bar). This PR aims to address some existing issues (#900, #1320, #1722, #1934 and probably some others) but will need more work before it can be merged. I'm uploading it now in case people want to test it.

It does look pretty good though:
20210606_212211

There's a discussion about getting transparency to work here: #2508

This is based on the work of frostidaho from a number of years ago - so big kudos to them for doing most of the legwork.

I'm assuming the system tray will be an issue so I'll look at that next...

Wayland would also need to be looked at separately.

EDIT: systray may be ok:
20210607_075722

EDIT 2: To do list

  • Handle redrawing of semi-opaque drawers. Overlay mode results in increasing opacity.
  • X11 - check if 32 bit enabled and adjust transparency if not (getting some rendering issues with 24-bit depth)
  • Testing
    • Check systray widget
      • Confirm transparency for icons
      • Note: SysTray only works for fully opaque/fully transparent bar. Won't fix for semi-opaque.
    • Check more widgets (graphs, images etc.)
      • Fix groupbox widget
      • Check tasklist (this is the only widget that looks like it may have a similar issue to groupbox)
    • Check whether gradient fill still works
    • Add tests!
  • Wayland
    • Check that Wayland backend not broken by these changes
    • Get transparency working in Wayland too!

@elParaguayo elParaguayo force-pushed the transparency branch 2 times, most recently from 62308ce to d0f47dd Compare June 6, 2021 20:38
@ramnes
Copy link
Member

ramnes commented Jun 7, 2021

Nice. Can you please share the to do list? Maybe you could get some help. :)

@elParaguayo
Copy link
Member Author

Hopefully it's just a case of testing now. I thought that the systray may be a problematic because that seemed to be an issue when frostidaho originally worked on this. It may be working now by my sheer dumb luck!

@m-col
Copy link
Member

m-col commented Jun 7, 2021

The Wayland Windows are all rendered as 32bit so should work fine without much changing.

Do we need to make the depth configurable? Perhaps we could try to use 32bit but get_depth_and_visual could fallback to the root visual if a 32bit depth isn't available?

Could you elaborate on the custom colormap stuff and the issues with RecordingSurface? Just so I understand what's going on :)

@m-col
Copy link
Member

m-col commented Jun 7, 2021

Just seen #2508 (comment) which helped. What does cairocffi.OPERATOR_CLEAR do? Is it the case the RecordingSurface is incompatible with using alpha?

@elParaguayo
Copy link
Member Author

elParaguayo commented Jun 7, 2021

The Wayland Windows are all rendered as 32bit so should work fine without much changing.

Well, it didn't work when I tried it but I don't know if I've got Wayland working or not (got a solid magenta background when I started it).

Do we need to make the depth configurable? Perhaps we could try to use 32bit but get_depth_and_visual could fallback to the root visual if a 32bit depth isn't available?

Currently, you can set the depth for the bar but, other than that, the changes default to requesting 32 bit windows for internal windows. It should fallback to root visual if 32 bit isn't available.

Could you elaborate on the custom colormap stuff and the issues with RecordingSurface? Just so I understand what's going on :)

Custom colormap was shamelessly copied from frostidaho's work (#1059 (comment)).

As for the issue with the RecordingSurface, this post on our discussions page should explain the issue. In short, the clear operator is not applied to the XCBSurface. My "solution" was to write the clear direct to the XCBSurface.

if getattr(self, "_xcb_surface", None) is not None:
ctx = cairocffi.Context(self._xcb_surface)
ctx.save()
ctx.set_operator(cairocffi.OPERATOR_CLEAR)
ctx.paint()
ctx.restore()

@elParaguayo
Copy link
Member Author

elParaguayo commented Jun 7, 2021

Ah - our posts overlapped.

This page is good at explaining the operators but, in short, OPERATOR_CLEAR removes content from the area being painted. The alternative is to use OPERATOR_SOURCE which doesn't do any blending and just puts the new source onto the surface but that also didn't work. The default operator is OPERATOR_OVER which does blending and that's the behaviour that I was seeing.

I haven't tested more but it seems that RecordingSurfaces aren't handling changing operators correctly. However, if we wrote directly to an XCBSurface we wouldn't have this issue.

In case it's not obvious, there's no issue when using opaque surfaces because OPERATOR_OVER has no alpha to blend so you just get the updated content each time.

@m-col
Copy link
Member

m-col commented Jun 7, 2021

Well, it didn't work when I tried it but I don't know if I've got Wayland working or not (got a solid magenta background when I started it).

The magenta is actually currently the default "you have no background set" colour, which I was using to help with damage tracking because it's so high contrast :p I should probably change that to black like the X server.... As for Internal, they themselves are represented by a 32 bit texture to which pixels are copied but Drawer will need updating in the same way as the x11 Drawer, and if some parts overlap then they can be implemented in base.py as is currently the case.

Custom colormap was shamelessly copied from frostidaho's work (#1059 (comment)).

OK thanks, think I'm following now.

I haven't tested more but it seems that RecordingSurfaces aren't handling changing operators correctly. However, if we wrote directly to an XCBSurface we wouldn't have this issue.

IIRC RecordingSurfaces were introduced so that the underlying pixmaps can be resized (i.e. destroyed and then replaced with different one) while keeping the drawing operations around and usable across pixmaps. Maybe we could find an alternative solution for that, otherwise we'll have to figure out a way to accomplish using alpha while continuing to use RecordingSurfaces.

@elParaguayo
Copy link
Member Author

The magenta is actually currently the default "you have no background set" colour, which I was using to help with damage tracking because it's so high contrast :p I should probably change that to black like the X server.... As for Internal, they themselves are represented by a 32 bit texture to which pixels are copied but Drawer will need updating in the same way as the x11 Drawer, and if some parts overlap then they can be implemented in base.py as is currently the case.

Good to know. I would like to inlcude Wayland in this PR if I can (will need to rename it...) and it sounds like it might be easy to do so I'll add to the list at the top.

IIRC RecordingSurfaces were introduced so that the underlying pixmaps can be resized (i.e. destroyed and then replaced with different one) while keeping the drawing operations around and usable across pixmaps. Maybe we could find an alternative solution for that, otherwise we'll have to figure out a way to accomplish using alpha while continuing to use RecordingSurfaces.

As far as I can tell, it's only going to be an issue when we need to clear the background which should be solved by my updated clear code. If that's the case, then I don't see a need to necessarily move away from using a RecordingSurface. For the rest of the time, if you're drawing semi transparent content over the background in the Drawer then the RecordingSurface should be fine as it will be using OPERATOR_OVER which is exactly the behaviour we'd want at that point.

@elParaguayo

This comment has been minimized.

@ramnes
Copy link
Member

ramnes commented Jun 7, 2021

Hopefully it's just a case of testing now.

You probably want to take a look at @m-col's work on #1697 for this, the tests should be quite similar I think?

@elParaguayo
Copy link
Member Author

Will do. Although I was thinking of more "hands-on" tests and seeing if things still display properly. A test script won't necessarily do that.

@elParaguayo elParaguayo changed the title Add 32-bit windows for X11 Transparency in X11 and Wayland Jun 8, 2021
@elParaguayo
Copy link
Member Author

Good news is that transparency is working in wayland (much less effort than x11).

However, I'm getting some interesting differences between x11 and wayland with same config.
20210608_191922
wayland

More digging to do!

@m-col
Copy link
Member

m-col commented Jun 8, 2021

Very cool. What differences are you seeing?

@elParaguayo
Copy link
Member Author

elParaguayo commented Jun 8, 2021

If you look at the left-hand side you can see that the layout icon is transparent and shows the bar colour in x11 but, in wayland, you see the wallpaper instead. x11 is correct here.

However, also in x11, the group box has lines underneath the inactive groups whereas wayland doesn't. wayland is correct on this.

EDIT: solved the first one, just the second to go...

EDIT 2: second one is an issue with the widget itself. If the line colour isn't set for inactive groups then it draws the line with the background colour. However, where you've got semi-opaque backgrounds, drawing this colour on top of itself makes it darker because the draw blends in overlay mode.

X11:
20210608_214727

Wayland:
wayland2

@m-col
Copy link
Member

m-col commented Jun 9, 2021

Nice, nice. re to do list entry Check multiple systray widgets, what's the worry there? I always found that having multiple systrays was buggy so I put one only on my first screen.

@elParaguayo
Copy link
Member Author

To be very honest: that's a typo.

I was thinking about checking multiple widgets (as in I want to check whether transparency threw out any issues in individual widgets, like the GroupBox in my earlier post). I was also thinking about the system tray as that had been flagged earlier as not working with transparency. My brain just squashed those ideas together!

@m-col
Copy link
Member

m-col commented Jun 9, 2021

Lol fair. The systray is an interesting case because the pixels are provided by clients. Should transparency even apply to its icons?

@elParaguayo
Copy link
Member Author

Systray is one of the widgets I'm least familiar with but I plan to look at it later today as it's crashing on wayland at the moment but I suspect that's a quick fix. Once it's running I can then test the transparency point.

In terms of your question, I would hope that the icons are in 32 bit windows so transparency is available to them. That seems to be the case with one of my earlier screenshots showing vlc in the tray but I haven't dug in to the code to see how it all fits together.

@m-col
Copy link
Member

m-col commented Jun 9, 2021

Systray is one of the widgets I'm least familiar with but I plan to look at it later today as it's crashing on wayland at the moment but I suspect that's a quick fix.

I could be wrong, but my understanding of the systray is that it is inherently tied to X (https://specifications.freedesktop.org/systemtray-spec/systemtray-spec-0.2.html), and therefore will never work under Wayland. For this reason I haven't looked at it at all as part of the wayland work. However, that is not to say that a system tray cannot be used under Wayland, it would just use a different protocol, e.g. https://www.freedesktop.org/wiki/Specifications/StatusNotifierItem/ which is what Swaybar uses.

@elParaguayo
Copy link
Member Author

Good to know. So my quick fix will be to remove it from my wayland config!

Can look at making a wayland systray another time.

@m-col
Copy link
Member

m-col commented Jun 9, 2021 via email

@elParaguayo
Copy link
Member Author

Interesting bug in SysTray with these changes. First icon displays fine but subsequent icons copy the drawing from the first icon and so on.

On Master, it should look like this:
20210610_062622
But on this branch, it looks like this:
20210610_062518

Need to dig in to the SysTray code and see how the drawers work there.

@elParaguayo
Copy link
Member Author

elParaguayo commented Jun 11, 2021

I think the bug has something to do with this error in the logs:

2021-06-11 08:23:23,950 ERROR libqtile core.py:_xpoll():L329 Got an exception in poll loop
Traceback (most recent call last):
  File "/home/elp/Documents/scripts/github/qtile-dev/qtile/libqtile/backend/x11/core.py", line 282, in _xpoll
    event = self.conn.conn.poll_for_event()
  File "/home/elp/Documents/scripts/github/qtile-dev/qtile/venv/lib/python3.9/site-packages/xcffib/__init__.py", line 571, in wrapper
    return f(*args)
  File "/home/elp/Documents/scripts/github/qtile-dev/qtile/venv/lib/python3.9/site-packages/xcffib/__init__.py", line 612, in poll_for_event
    return self.hoist_event(e)
  File "/home/elp/Documents/scripts/github/qtile-dev/qtile/venv/lib/python3.9/site-packages/xcffib/__init__.py", line 686, in hoist_event
    return self._process_error(ffi.cast("xcb_generic_error_t *", e))
  File "/home/elp/Documents/scripts/github/qtile-dev/qtile/venv/lib/python3.9/site-packages/xcffib/__init__.py", line 649, in _process_error
    raise error(buf)
xcffib.xproto.MatchError

I don't get this when the bar's window depth is set to 24 bit so I'm assuming that there's some mismatch between the bar and the SysTray in terms of the depth or visual but I'm having difficulties tracing it down.

@m-col
Copy link
Member

m-col commented Jun 11, 2021

@elParaguayo
Copy link
Member Author

Certainly some interesting bits in there but there's also this:

TODO: 32-bit visual
_NET_SYSTEM_TRAY_VISUAL visual_id VISUALID/32

The property should be set by the tray manager to indicate the preferred visual for icon windows.

To avoid ambiguity about the colormap to use this visual must either be the default visual for the screen 
or it must be a TrueColor visual. If this property is set to a visual with an alpha channel, the tray manager 
must use the Composite extension to composite the icon against the background using PictOpOver.

@m-col
Copy link
Member

m-col commented Jun 11, 2021

How will that affect what you're doing?

@elParaguayo
Copy link
Member Author

elParaguayo commented Jun 19, 2021

Almost got the Mirror working as above. It works fine on wayland but has a weird bug on x11: The background is getting cleared but the contents is not immediately drawn. So, if you mirror a Clock the text appears after 1 second (i.e. after the first refresh) but if you mirror a static TextBox the text is never drawn.

Can't immediately see where I've gone wrong and why it's working on wayland...

@m-col would be good if you could take a look at some point.

EDIT: looks like I need to rebase onto master but will do this once we've fixed this bug.

@elParaguayo elParaguayo force-pushed the transparency branch 3 times, most recently from 624d934 to a169709 Compare June 19, 2021 09:12
@elParaguayo

This comment has been minimized.

@elParaguayo elParaguayo force-pushed the transparency branch 2 times, most recently from b2500ab to 0afe403 Compare June 19, 2021 13:05
In X11 backends, transparency will be disabled in a bar if the ``background``
color is fully opaque.


Copy link
Member

Choose a reason for hiding this comment

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

Maybe add a note about Systray?

Copy link
Member Author

Choose a reason for hiding this comment

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

I added a SysTray comment in its own docstring so it's visible on the Widgets page.

@m-col
Copy link
Member

m-col commented Jul 3, 2021

The only issue I have is that the very first draw of mirror widgets (or, at least, mirrors of the CPUGraph widget) appear as black and then subsequent draws show what they are meant to. It's a small issue, but fixing it would make this "complete" IMO. If fixing that is not straight forward, I'd rather this PR be in the next rlease as is than not at all.

@elParaguayo
Copy link
Member Author

elParaguayo commented Jul 3, 2021

Great - let me take a look at that.

EDIT: Interesting. It's Wayland only...

EDIT 2: Think I've fixed it. Just need to rebase.

This PR adds 32-bit colour depth for X11 which allows transparent
windows (e.g. the bar). This is based on the work of frostidaho
from a number of years ago.

The PR also makes some changes to the Wayland backend to enable
rendering of transparent windows.
This PR fixes rendering issues in widgets related to the use of
transparent backgrounds.

SysTray: Only works on fully transparent backgrounds.

GroupBox and TaskList: fixes issues when background is semi-transparent.
The SysTray widget will not display correctly with a solid background
in a 32-bit window. We therefore set the bar's window depth to 24-bit
if the user requests a fully opaque bar.
rect_changed = current_rect != self.previous_rect

# Check if draw has content (would be False for completely transparent drawer)
ink_changed = any(not math.isclose(0.0, i) for i in self.surface.ink_extents())
Copy link
Member

Choose a reason for hiding this comment

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

It's interesting that ink_extents doesn't take into account transparent painted things. I'm guessing you checked this before doing it this way? I'd have expected them to be handled in the same way as things without 100% transparency.

Copy link
Member Author

Choose a reason for hiding this comment

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

Correct. Anything with 100% transparency doesn't register with ink_extents(). However, I think that makes sense as, if it's fully transparent, there's nothing to draw.

Copy link
Member

@m-col m-col left a comment

Choose a reason for hiding this comment

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

Great! Everything looks good to me, so if there is nothing left you had in mind to change then I guess this is ready for merge 🎉

@elParaguayo
Copy link
Member Author

It's light on tests (just checking some additional utilities) but, other than that, there's nothing more that I think needs to be added here.

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.

3 participants