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

Stabilize tray module #2689

Closed
11 tasks done
patrick96 opened this issue Apr 15, 2022 · 14 comments
Closed
11 tasks done

Stabilize tray module #2689

patrick96 opened this issue Apr 15, 2022 · 14 comments
Milestone

Comments

@patrick96
Copy link
Member

patrick96 commented Apr 15, 2022

With #2595 being merged, there are a few steps to make the tray module stable.
The goal is to completely deprecate positioning the tray without a module (and all tray-* settings in the bar section).

  • Automatically hide/show the tray window depending on whether a tray position (%{Pt} tag) is set in the renderer. This is needed to deal with hiding modules.
  • Remove tray-position = adaptive again and just activate the tray when a tray module exists (or one of the other positions is set). The existence of a tray module should override any value for tray-position. In particular, if a tray module exists and tray-position is set, a warning or error should be printed.
  • Fix the tray window being pushed off the bar if there is too much stuff on the bar. The tray window should always be fully within the bar window.
  • Rework all tray settings in module (see below)
  • Write module documentation on RTD
    • Write migration docs on RTD.
  • Emit deprecation warnings for tray-position and all other tray settings in the bar section. Deprecate legacy tray options #3002

It's probably easiest to implement each of these in a separate PR to keep complexity low. The order shouldn't matter except that the last one should be done last.

If we don't make the tray stable before the 3.7 release, polybar should also emit a warning whenever the tray module is used so that people are aware that the tray module may break again in the future.


For those wanting to test it out right now, I'll also add a minimal example for how to add the tray module. All of this is experimental and any config options may change before the tray becomes stable.

[bar/example]
modules-left = tray

[module/tray]
type = internal/tray
format = <tray>

We could now also completely change the tray behavior if the module is used without it being a breaking change. In the tray_manager we could have a flag in_module that can be used to differentiate between behavior for the old tray or the module.
We should consider taking advantage of this, especially for formatting.

  • There should be tray-spacing, which determines the spacing between the icons (tray-padding kind of does this already but the padding is also added at the beginning and end of the tray window.
  • tray-padding should now be applied to the left and right of each individual icon (just as what happens for regular labels). Only supports points and pixels
  • tray-maxsize and tray-scale should be merged into a single option. They both affect client_height, while tray-maxsize also increases spacing. This can be made cleaner. There should probably be just a single tray-size option (percentage with offset relative to bar height, defaults to 66%) and the final icon height and width is min(bar_heigh, tray-size).
  • Use of tray-background should be discouraged. Make it clear that it will only affect tha background of individual icons, not the spacing in between.

This would help fix #1207 I think.

The following tray settings will not have any equivalent setting in the module:

  • tray-detached: Not really sure what the use-case for this ever was
  • tray-transparent: Already deprecated and has no effect
  • tray-offset-x: Can be achieved with format offsets and offset tags.
  • tray-offset-y: Due to the tray being subwindows now, it cannot be moved outside of the bar area anyway. If people want to vertically position icons within the bar, we could introduce a tray-alignmnent setting.

The new tray module would also be a workaround for #651, as long as there is enough padding before or after the tray module.


Linked Issues & PRs

@patrick96 patrick96 added this to the 3.7.0 milestone Apr 15, 2022
@raffael0
Copy link
Contributor

Since we now have experimental support in the feature branch we should probably add some documentation on how to use it. I think adding a new module in the wiki sidebar would probably be the best solution with a big experimental warning on top.
Another approach would be to wait with the documentation until the feature is stable.
How would you like to handle this?

@patrick96
Copy link
Member Author

I don't think documentation is crucial at the moment. Especially since any settings inside the tray module may well change again. We should probably hold off writing documentation until at least the config settings are stable.

@tiagovla
Copy link

Could you still provide a short example of how to use it? I would like to try it.

@patrick96
Copy link
Member Author

@tiagovla Added it to the post.

@raffael0
Copy link
Contributor

I thought a bit about the remaining issues and I have some questions/comments.

  1. Hiding the tray window. Was this already implemented? I'm not really sure which position you mean. The one we set in the context or the global position(left,center...)? When are the modules hidden?
  2. tray_postition = adaptive: Is there a way to check in the tray_manager if the tray module was instantiated? I added adaptive because I didn't know how to figure out when no tray module was instantiated and therefor didn't know when not to set left, right, center.
  3. pushed off bar: Has there been some prior work into something similar like this(Maybe with text)? Otherwise I imagine that this is going to be really difficult to implement since we never know in the renderer how much content will be added before or after the tray and how wide the tray is. I can't think of a way to approach this.
  4. How do you want to manage the configs? I can think of two approaches. First would be to keep the config handling in the tray_manager and just read the config from the tray module instead of the bar. This is a bit messy. The other option would be to move the config handling to the tray module. This is a much cleaner solution but then it's quite hard to get the config settings to the tray_manager(Maybe with a signal?) since they are completely separate right now.

Right now a lot of logic is split between the tray_manager and the tray module with no way of them interacting(Besides signals). This complicates a lot but I can't think of a way to simplify this short of moving the tray_manager completely into the tray module. I'm unsure how "clean" we should implement these things since some(like the tray_position) will be removed later which cleans up a lot of code.

@patrick96
Copy link
Member Author

Hiding the tray window. Was this already implemented? I'm not really sure which position you mean. The one we set in the context or the global position(left,center...)? When are the modules hidden?

I didn't formulate that clearly, I meant whether the dispatch called apply_tray_position with a valid position. Tray hiding wasn't implemented before but since modules can be hidden, the tray should probably support it as well.
What currently happens, is that if the module is hidden, no %{Pt} tag is emitted, so the tray position isn't updated and the tray just stays wherever it was in the previous render cycle.

Modules can be hidden using module actions (for example using polybar-msg).

tray_postition = adaptive: Is there a way to check in the tray_manager if the tray module was instantiated? I added adaptive because I didn't know how to figure out when no tray module was instantiated and therefor didn't know when not to set left, right, center.

The tray can't really check this. But what we can do, is pass this information from the controller to bar::start which then passes it to tray_manager::setup. The controller can check this in the loop where it creates the modules.

pushed off bar: Has there been some prior work into something similar like this(Maybe with text)? Otherwise I imagine that this is going to be really difficult to implement since we never know in the renderer how much content will be added before or after the tray and how wide the tray is. I can't think of a way to approach this.

For text that doesn't fit onto the bar, we render a fading out gradient at the end of the bar when the left, right, and center blocks are placed on the bar:

if (!fits) {
// Paint falloff gradient at the end of the visible block
// to indicate that the content expands past the canvas
/*
* How many pixels are hidden
*/
double overflow = xw - m_rect.width;
double visible_width = w - overflow;
/*
* Width of the falloff gradient. Depends on how much of the block is hidden
*/
double fsize = std::max(5.0, std::min(std::abs(overflow), 30.0));
m_log.trace("renderer: Drawing falloff (pos=%g, size=%g, overflow=%g)", visible_width - fsize, fsize, overflow);
m_context->save();
*m_context << cairo::translate{(double)m_rect.x, (double)m_rect.y};
*m_context << cairo::abspos{0.0, 0.0};
*m_context << cairo::rect{x + visible_width - fsize, y, fsize, h};
m_context->clip(true);
*m_context << cairo::linear_gradient{
x + visible_width - fsize, y, x + visible_width, y, {rgba{0x00000000}, rgba{0xFF000000}}};
m_context->paint(0.25);
m_context->restore();
}

However, for the tray, my idea was to not have this logic in the renderer but in the tray itself.
The tray manager knows how wide the tray and the bar are, so when it receives the tray_pos_change signal, it can calculate whether the new position will place the tray window (partially) outside of the bar. And instead of hiding parts of the tray window when it goes outside of the bar, we keep it at the right end of the bar instead of letting it slide off. The calculation for the tray position would look something like this:

tray_pos = bar.x + max(0, min(tray_pos, bar.w - tray_width))

How do you want to manage the configs? I can think of two approaches. First would be to keep the config handling in the tray_manager and just read the config from the tray module instead of the bar. This is a bit messy. The other option would be to move the config handling to the tray module. This is a much cleaner solution but then it's quite hard to get the config settings to the tray_manager(Maybe with a signal?) since they are completely separate right now.

I think we should go with the first option, the tray manager should be responsible for loading its own configuration options. This could be achieved by passing the name of the tray module to tray_manager::setup.


If we didn't have to still support tray-position = left|center|right, we would create the tray manager instace inside the tray module and would not have any of these issues about communicating information between components. But even if it is going to be removed at some point, it will still be with us for some time, so we shouldn't just be sloppy here.

@raffael0
Copy link
Contributor

raffael0 commented Jun 2, 2022

In the tray_manager we could have a flag in_module

I decided to just store the tray module name since we need that anyways for the formatting options

@ruscur
Copy link

ruscur commented Jun 8, 2022

With the tray becoming a module, is it possible that true transparency could be added (as per #2086 (comment))?

Not to impose more features on this issue, just curious as to the possibility/complexity as someone without much understanding of X/WMs.

@raffael0
Copy link
Contributor

raffael0 commented Jun 8, 2022

I also barely know anything about X and window managers 😅 so I'm probably not really qualified to answer that.

The changes we added in these PRs are basically one big hack. As you might know the tray right now is a separate window overlayed over the bar window. Instead of removing this window and incorporating the tray into the bar, we decided to internally create blank a module with the same size as the tray window. The tray window is then just moved over the blank window in the bar. Therefore, the tray now behaves like a normal module but is still a separate window.

I think that since we only change the window position the same problem still remains with true transparency, although @patrick96 probably knows a lot more about it.

I hope this helps!

@patrick96
Copy link
Member Author

Sorry for the radio silence, I have been swamped the past few weeks. I appreciate your work on this 😃

I should have some time to start reviewing stuff again next week, but I will only be around sporadically for at least a month or two.


I decided to just store the tray module name since we need that anyways for the formatting options

👍

With the tray becoming a module, is it possible that true transparency could be added (as per #2086 (comment))?

True transparency remains as difficult as it has before, this "new" tray module didn't really change how the tray is rendered, just where it is placed on the bar. I am considering adding back true transparency in #2609, but no promises.

As you might know the tray right now is a separate window overlayed over the bar window. Instead of removing this window and incorporating the tray into the bar, we decided to internally create blank a module with the same size as the tray window.

I don't think we will ever get away from doing it like this. Since each tray icon is a separate window, we have to somehow overlay them over the bar window.

@raffael0
Copy link
Contributor

I'll look over/reorganize my existing PRs till next week and try to finish the settings one.

Since the plan is to release this feature with 3.7. What's your timetable for that release?

@patrick96
Copy link
Member Author

There is no fixed timeline. It is mainly tied to which features I want in it. In the 3.6 blog post I said that I wanted to have a fixed tray, new font renderer, and some config syntax improvements in 3.7. But it is starting to look like that will be take too much time for a single release (I really want to release more often than every 12 months).

So I think we should release 3.7 once the tray module is stable and I am finished with #2609. I do want to finish #2609 before 3.7 because it will result in some breaking tray changes (basically if you had the tray anywhere outside of the bar window, this will no longer work) and if people should migrate to the module anyway that won't be a big problem.

patrick96 pushed a commit that referenced this issue Jun 15, 2022
The tray is automatically started if there is a tray module. In addition, `tray-position` and the tray module conflict.

Ref #2689
@quantenzitrone
Copy link
Contributor

I have started building polybar from source and using the unstable tray module, and I must say, it works quite well until now.
One problem tho:
The tray background has pseudo transparency, which is stated in the wiki, yes, but it looks off if the tray position changes (depending on the size of the other modules), because then the tray background is not the actual background.
So to fully integrate the tray as a module, I think we need compositor transparency support.

@patrick96
Copy link
Member Author

@quantenzitrone I have also noticed this and I think I no longer see this in #2609. However certain applications still have that issue (dropbox for me) and I have not yet found a way to make them behave (I am not even sure if a way exists).

I have also decided to not implement true transparency for the tray icons because we would have to do the compositing ourselves and couldn't use a compositor like picom and I don't have the time to do that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants