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.

Sign up for GitHub

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

Jump to bottom

-l opacity does not work as documented #281

Closed
mimuki opened this issue May 3, 2023 · 6 comments · Fixed by #296
Closed

-l opacity does not work as documented #281

mimuki opened this issue May 3, 2023 · 6 comments · Fixed by #296
Labels
bug Something isn't working

Comments

@mimuki
Copy link

mimuki commented May 3, 2023

According to the man page, running a command like scrot -s -l width=8,color="#ff79c6",opacity=255,mode=edge should result in an opaque pink border, and should be not transparent at all.

Instead, opacity=255 is slightly see through, but opacity=100 is completely opaque.

I'm not familiar with C and might be misunderstanding this line, but it looks like this isn't a bug, just the documentation being wrong.

scrot/src/selection_edge.c

Line 79 in 5ef8a82

unsigned long opacity = opt.lineOpacity * ((unsigned)-1 / 100);

@N-R-K N-R-K added the bug Something isn't working label May 3, 2023
@N-R-K
Copy link
Collaborator

N-R-K commented May 6, 2023

I can't really find any documentation on _NET_WM_WINDOW_OPACITY - it's not described in EWMH and seems to be something non-standard. Only thing I found was this comment in Tk source code which suggest the atom works by mapping [0x0, 0xFFFFFFFF] to [0%, 100%] transparency.

If that's the case, then the bug I think is in the source code. Not the documentation.

@N-R-K
Copy link
Collaborator

N-R-K commented May 6, 2023

This will be hard for me to fix because I don't use a compositor. If you change that line to the following:

	unsigned long opacity = opt.lineOpacity * (0xFFFFFFFFu / 255);

does that work as expected?

If my understanding of _NET_WM_WINDOW_OPACITY is correct, then it should work. But I'll need help from people who can actually test it out and report back :)

@N-R-K N-R-K changed the title man page has incorrect documentation for -l opacity -l opacity does not work as documented May 6, 2023
N-R-K added a commit to N-R-K/scrot that referenced this issue May 18, 2023
@N-R-K
fix: incorrect line opacity mapping
5419aa6 
current code treats opacity as [0, 100] even though the manpage
documents it as being [0, 255].

Fixes: resurrecting-open-source-projects#281
@N-R-K N-R-K mentioned this issue May 18, 2023
Merged
@daltomi
Copy link
Collaborator

daltomi commented May 25, 2023

The opacity values for the 'edge' selection mode is between 10% and 100%
At some point the function call was removed:

int const lineOpacity = optionsParseRequireRange(opt.lineOpacity, 10, 100);

@N-R-K
Copy link
Collaborator

N-R-K commented May 25, 2023

It was:

    const int lineOpacity = optionsParseRequireRange(opt.lineOpacity,
            SELECTION_EDGE_OPACITY_MIN, SELECTION_OPACITY_MAX);

The SELECTION_EDGE_OPACITY_{MIN,MAX} constants got removed in ff88b0b for some reason.

@N-R-K
Copy link
Collaborator

N-R-K commented May 26, 2023 •
edited

The opacity values for the 'edge' selection mode is between 10% and 100%

@daltomi What do you think should be done now?

  1. Change the code back to using [10, 100] for edge and fix the manpage.
  2. Change the code to use [0, 255] as documented in the manpage.

@daltomi
Copy link
Collaborator

daltomi commented May 26, 2023 via email

N-R-K added a commit that referenced this issue May 27, 2024
@N-R-K
fix: incorrect line opacity mapping
95350f6 
current code treats opacity as [0, 100] even though the manpage
documents it as being [0, 255].

Fixes: #281
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug Something isn't working
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

fix: incorrect line opacity mapping N-R-K/scrot
3 participants
@mimuki @daltomi @N-R-K