bakkeby / dmenu-flexipatch

A dmenu build with preprocessor directives to decide which patches to include during build time
MIT License
183 stars 78 forks source link

Looks like ALPHA_PATCH's `-o` option does nothing #4

Closed PRESFIL closed 1 month ago

PRESFIL commented 3 years ago

I tried to find the use of the ALPHA_PATCH's variable opacity after reading its value from argv, but I couldn't find anything. Is it dangling?

Also, as far as I understand, this is not the same patch from suckless (although it does not know how to read the value from argv at all).

I ask you to fix this because transparency in st forces the use of transparency in dmenu to be used when using with EXTERNALPIPE_PATCH. In total, everything works, but transparent embedded dmenu with transparent st look about weird...

image

PRESFIL commented 3 years ago

I found a solution in disabling transparency by using:

--- config.def.h
+++ config.h
@@ -48,7 +48,7 @@
 #endif // SYMBOLS_PATCH

 #if ALPHA_PATCH
-static const unsigned int baralpha = 0xd0;
+static const unsigned int baralpha = OPAQUE;
 static const unsigned int borderalpha = OPAQUE;
 static const unsigned int alphas[][3]      = {
        /*               fg      bg        border     */

But still wondering about dangling switch /variable / parameter.

bakkeby commented 3 years ago

Also, as far as I understand, this is not the same patch from suckless (although it does not know how to read the value from argv at all).

Yes this was based on Baitinq's dmenu and was added back in May 2020, wasn't aware that an alpha patch had been added on the suckless page. Looks like it was added in June 2021.

Maybe it is worth removing the alpha patch that I have in favour of that.

I ask you to fix this because transparency in st forces the use of transparency in dmenu to be used when using with EXTERNALPIPE_PATCH.

Do you know of this also happens with a bare dmenu that is just patched with the externalpipe patch?

I see that the opacity command line argument originates from Baitinq's dmenu, but that logic was later removed in favour of something more similar to what the alpha patch on the suckless page has. I must have forgotten to remove those lines when I was working out what was needed to get the alpha patch working back then. As-is it doesn't do anything so I should remove that, but it would make sense to move to the new patch on the suckless page as that is more of a reference.

I suspect that moving to the new patch won't actually resolve your issue though. An intermediate workaround could be to have a second dmenu binary that isn't compiled with the alpha patch, and use that in relation to st with the externalpipe patch.

Another intermediate solution is to add this change:

diff --git a/dmenu.c b/dmenu.c
index 70d6483..ebb454a 100644
--- a/dmenu.c
+++ b/dmenu.c
@@ -1312,7 +1312,7 @@ xinitvisual()

        XFree(infos);

-       if (! visual) {
+       if (! visual || !opacity) {
                visual = DefaultVisual(dpy, screen);
                depth = DefaultDepth(dpy, screen);
                cmap = DefaultColormap(dpy, screen);

The opacity value can then at least be used to disable alpha altogether by passing -o 0.

All add that for now, not entirely sure when I'll find time to look into changing the patch itself.

PRESFIL commented 3 years ago

Maybe it is worth removing the alpha patch that I have in favour of that.

I dont know. It seems very strange that transparency cannot be overridden in the current patch from suckless. Perhaps it is better to leave it as it is for now, or even to ask them to look for a solution.

I'd like to use your last :point_up: commit 1d200d1 and add the appropriate changes to st-flexypatch:

```diff --- config.def.h +++ config.h @@ -375,7 +375,7 @@ #if EXTERNALPIPE_PATCH // example command static char *openurlcmd[] = { "/bin/sh", "-c", - "xurls | dmenu -l 10 -w $WINDOWID | xargs -r open", + "xurls | dmenu -l 10 -w $WINDOWID -o 0 | xargs -r open", "externalpipe", NULL }; #endif // EXTERNALPIPE_PATCH ```

Do you know of this also happens with a bare dmenu that is just patched with the externalpipe patch?

I don’t know, but I’ll try. Your project greatly facilitates the process and without doing it manually it will be very sad :disappointed: .

but that logic was later removed in favour of something more similar to what the alpha patch on the suckless page has.

I didn't quite understand what kind of logic they have there now.

bakkeby commented 3 years ago

but that logic was later removed in favour of something more similar to what the alpha patch on the suckless page has.

I didn't quite understand what kind of logic they have there now.

Baitinq implemented the alpha patch for dmenu himself, and I suspect that his first attempt was based on the clientopacity patch for dwm and later he managed to get it to work with the alpha patch. The clientopacity patch sets a property on the window so that the compositor can set the opacity accordingly, but the text will also be transparent so it is not ideal.

When I was trying to mimic what he did in his build I must have taken some of the remnants of the clientopacity patch in the process which did not actually do anything in the end.

bakkeby commented 1 year ago

Revisiting this (after nearly two years) I find that the -o option is merely intended to enable or disable alpha depending on what opacity is set to in the config.h file.

More so this works as-is if one enables the alpha patch in patches.h.

But, if the alpha patch is combined with the xresources patch then the X resources are read before command line arguments are processed - this because command line options that affect colours or font should still be allowed to override whatever is set as defaults within dmenu and what is loaded via X resources.

Before xresources are loaded the drawable needs to be created, which means that the alpha visuals need to be initialised as well, and given that this happens before the command line arguments are parsed the opacity option is not overridden.

As such this is merely a compatibility issue between the alpha and xresources patches. Will push a fix for this.