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

Add transparency support #96

Merged
merged 7 commits into from
Mar 16, 2020
Merged

Add transparency support #96

merged 7 commits into from
Mar 16, 2020

Conversation

vilhalmer
Copy link
Contributor

This was a much smaller diff than anticipated! Colors are now accepted with or without a fourth pair of hex values for alpha. Without alpha specified, it defaults to FF as you would expect.

@vilhalmer
Copy link
Contributor Author

This fixes #71.

@Cloudef
Copy link
Owner

Cloudef commented Feb 27, 2020

Very nice! Did you test this on which platform? X11 might need RGBA bits set on window creation. Wayland i think would work without any changes.

@vilhalmer
Copy link
Contributor Author

Just fixed up clearing the cairo surface, missed pushing that up. I've only tested on wayland, I can take a look at X tomorrow morning.

@Cloudef
Copy link
Owner

Cloudef commented Feb 27, 2020

If you are willing to do X support as well. I'll wait until that before merging this. Looks good.

@Cloudef
Copy link
Owner

Cloudef commented Feb 27, 2020

Also note that X11 most likely won't work without a compositor. Fake transparency could be done for non compositor support. But that could as very well be another feature, another pr.

if (sscanf(nhex,"#%2x%2x%2x", &r, &b, &g) != 3)
unsigned int r, g, b, a = 255;
int matched = sscanf(nhex, "#%2x%2x%2x%2x", &r, &b, &g, &a);
if (matched != 3 && matched != 4)
Copy link

@panaman67 panaman67 Feb 27, 2020

Choose a reason for hiding this comment

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

Suggested change
if (matched != 3 && matched != 4)
if (matched < 3)

Copy link
Owner

@Cloudef Cloudef Feb 27, 2020

Choose a reason for hiding this comment

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

That would make it accept color codes with more than 4 components, so not sure if that's good idea 🤔

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm fine with either way, but only checking < 3 will allow extra characters to slip by.

@vilhalmer
Copy link
Contributor Author

If you are willing to do X support as well

I have no experience with Xlib, so if you'd rather handle that I will defer to you. But I'm also willing to learn a bit. :)

@Cloudef
Copy link
Owner

Cloudef commented Feb 27, 2020

https://gist.github.com/je-so/903479/834dfd78705b16ec5f7bbd10925980ace4049e17#file-testprogram-c-L53-L55
Basically these are the only bits you should need in bm_x11_window_create. Fallback to defaultvisual and defaultdepth if creating window with 32 bit visual and depth fails.

@Cloudef
Copy link
Owner

Cloudef commented Feb 27, 2020

I think if you are testing in XWayland, that should already be composed (by the wayland composer), so RGBA should work fine there.

@vilhalmer
Copy link
Contributor Author

I've pushed what I believe to be the correct solution. However, I'm unable to test properly because even current master refuses to start for me in Xwayland with BEMENU_BACKEND=x11. :(

I was able to test that it still runs correctly under native X, but I don't have an X compositor installed to see if it really becomes transparent.

@Cloudef
Copy link
Owner

Cloudef commented Feb 28, 2020

Compton is something you could use to test the X11 side quickly. https://github.com/chjj/compton

@Earnestly
Copy link
Contributor

Now developed in this fork: https://github.com/yshui/picom

@Cloudef
Copy link
Owner

Cloudef commented Feb 28, 2020

Tested it! Seems to work.

@Cloudef
Copy link
Owner

Cloudef commented Feb 28, 2020

if I do bemenu-run -l 15 -nb #00000000 and go down the list, it acts bit strangely. I guess there are no clears before redrawing?

@Cloudef
Copy link
Owner

Cloudef commented Feb 28, 2020

Also, the transparency seems to work even without a compositor. I guess either cairo or x11 does the fake transparency for you.

@vilhalmer
Copy link
Contributor Author

I guess there are no clears before redrawing?

Hmm, I thought this was fixed in ffd773f. Maybe cairo behaves differently when using the X backend?

@vilhalmer
Copy link
Contributor Author

I'm not sure that dc33df1 fixes the redrawing issue on X, but it makes more sense to use CAIRO_OPERATOR_CLEAR to clear either way. :)

@vilhalmer
Copy link
Contributor Author

The good news is I got transparency to work in X, not sure what I was doing wrong before. The bad news is that I've been bashing my head against bm_x11_window_render for a while and I have no idea what is going wrong. It seems like the cairo group is being used instead of a real double buffer, which I'm not sure works with transparency? Any change I make to the function either does nothing or eliminates the transparency.

@Cloudef
Copy link
Owner

Cloudef commented Feb 29, 2020

Why do you need to edit bm_x11_window_render?

@Cloudef
Copy link
Owner

Cloudef commented Feb 29, 2020

Ah, I just tested your new code. I see the issue now. Lemme send you a patch that should fix it in a moment.

@Cloudef
Copy link
Owner

Cloudef commented Feb 29, 2020

diff --git a/lib/renderers/x11/window.c b/lib/renderers/x11/window.c
index bca1003..daf129a 100644
--- a/lib/renderers/x11/window.c
+++ b/lib/renderers/x11/window.c
@@ -94,8 +94,11 @@ bm_x11_window_render(struct window *window, const struct bm_menu *menu)
     }
 
     if (buffer->created) {
+        cairo_save(buffer->cairo.cr);
+        cairo_set_operator(buffer->cairo.cr, CAIRO_OPERATOR_SOURCE);
         cairo_paint(buffer->cairo.cr);
         cairo_surface_flush(buffer->cairo.surface);
+        cairo_restore(buffer->cairo.cr);
     }
 }

This should do it. It will only work with compositor. Doing pseudo-transparency would involve using XCopyArea or other tricks. But that would be separate thing.

@Cloudef
Copy link
Owner

Cloudef commented Mar 2, 2020

Note with latest diff. Not sure if the clearing and such is needed in cairo render, but maybe they are still for wayland?

@vilhalmer
Copy link
Contributor Author

(Sorry, have been busy with other things. Will get back to this soon!)

@vilhalmer
Copy link
Contributor Author

Alright! With your patch and picom running, looks good on X. I think this is ready to go. Do you want me to squash things into a nicer set of commits, or will you handle that when merging?

@Cloudef
Copy link
Owner

Cloudef commented Mar 16, 2020

Lets see how github's "squash and merge" works

@Cloudef Cloudef merged commit 6350a40 into Cloudef:master Mar 16, 2020
@Cloudef
Copy link
Owner

Cloudef commented Mar 16, 2020

Merged, thank you!

@vilhalmer
Copy link
Contributor Author

Thanks for your help!

@vilhalmer vilhalmer deleted the transparency branch March 16, 2020 05:08
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

4 participants