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

609 channel picker with shader #624

Open
wants to merge 10 commits into
base: master
Choose a base branch
from

Conversation

B-LechCode
Copy link
Collaborator

No description provided.

@B-LechCode B-LechCode linked an issue Jan 20, 2025 that may be closed by this pull request
@B-LechCode B-LechCode marked this pull request as ready for review January 20, 2025 22:30
@Stoppedpuma
Copy link
Collaborator

Colours don't seem to be accurate:

colour.webm

This is confirmed by both oculante as well as hyprpicker.

@B-LechCode
Copy link
Collaborator Author

B-LechCode commented Jan 21, 2025

Colours don't seem to be accurate:
colour.webm

This is confirmed by both oculante as well as hyprpicker.

Thank you! I see what I missed here.
Edit: I was too fast - I don't see the issue. I watched the RGBA values and the red channel is constant 1.0. Seems to be ok? The Red channel is between ff and fe there, your screenshot replicates these values to all channels?

Left: build from master, Right: build from my PR - no difference visible for me :)
image

@Stoppedpuma
Copy link
Collaborator

Colours don't seem to be accurate:
colour.webm
This is confirmed by both oculante as well as hyprpicker.

Thank you! I see what I missed here. Edit: I was too fast - I don't see the issue. I watched the RGBA values and the red channel is constant 1.0. Seems to be ok? The Red channel is between ff and fe there, your screenshot replicates these values to all channels?

Left: build from master, Right: build from my PR - no difference visible for me :) image

Apologies, this was a late night review. I think I was confused by the hex value and other colour values being wrong. I was under the assumption that this would change the colour to update as well to that channel. Thinking about this now it wouldn't really make sense since it's just greyscale.

@Stoppedpuma
Copy link
Collaborator

The alpha channel seems to break the black border of the zoom window.

picker.webm

@B-LechCode
Copy link
Collaborator Author

B-LechCode commented Jan 21, 2025

The alpha channel seems to break the black border of the zoom window.
picker.webm

Thank you for this interesing bug. That's indeed a Problem, cause the shader doesn't do a distinction between border and normal texture.
I need to think about a possible fix and will come back then :)

@Stoppedpuma fixed it :)

Copy link
Collaborator

@Stoppedpuma Stoppedpuma left a comment

Choose a reason for hiding this comment

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

Functionally, this works fine. I'll leave actual code review to @woelper.

Copy link
Collaborator

@Stoppedpuma Stoppedpuma left a comment

Choose a reason for hiding this comment

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

Few obvious things as well as a clippy recommendation (clippy is being enforced going forwards).

src/ui/top_bar.rs Outdated Show resolved Hide resolved
src/texture_wrapper.rs Outdated Show resolved Hide resolved
src/texture_wrapper.rs Outdated Show resolved Hide resolved
@B-LechCode
Copy link
Collaborator Author

@woelper is it ok to merge, or do you want to have a look, too?

@woelper
Copy link
Owner

woelper commented Jan 23, 2025

This is great work! I hope I have some time to review tonight!

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.

Use shader for channel picker
3 participants