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

🎉 Focus #456

Closed
wants to merge 0 commits into from
Closed

🎉 Focus #456

wants to merge 0 commits into from

Conversation

jgarza9788
Copy link
Contributor

this is the first time i'm contributing (pull request) to any repo outside of my 9-5.

  • so please go easy 😅

this blurs and alphas the window to appear and disappears.
similar to the way the PIP window does on IOS.

focus

later the performance can be improved by adjusting line 52 of focus.frag.
this could allow the offset to be widen a bit ... so we are looping a little less.
however this comes at a cost of the quality of the blur.

@Schneegans
Copy link
Owner

Schneegans commented Oct 23, 2024

Hey thanks so much for this contribution! This is really a cool idea. I think we will need to tweak some things, but I am sure that we can include this.

Here are some initial observations:

  • You can fix the failing CI check by running scripts/clang-format.sh.
  • I think we have to improve the performance. The default setting makes (50*2)^2 = 10000 texture look-ups per pixel which is way too much for most hardware. I get a smooth animation on Full-HD windows if I lower the radius to 10 which still results in 400 texture look-ups per pixel. Of course, something like this should be implemented in two passes, but this is not possible here. I think an efficient, rather good looking strategy for single-pass blurs is to use radial look-ups. Like this:
    vec4 blur(vec2 uv, float radius) {
      vec4 color = vec4(0.0);
    
      const float tau        = 6.28318530718;
      const float directions = 15.0;
      const float samples    = 5.0;
    
      for (float d = 0.0; d < tau; d += tau / directions) {
        for (float s = 0.0; s < 1.0; s += 1.0 / samples) {
          vec2 offset = vec2(cos(d), sin(d)) * radius * (1.0 - s) / uSize;
          color += getInputColor(uv + offset);
        }
      }
    
      return color / samples / directions;
    }
    Here, the radius does not affect the number of samples taken. The settings above will result in 75 texture look-ups (which is still much) but it produces acceptable results even with a radius of 100px.
  • It would be cool if you added a quality slider! If you use the above implementation, I think you could simply multiply both, the directions and the samples with the quality (maybe in the range [0.1 ... 1.0]) calling ceil() on both to ensure integer numbers.

What do you think? Would you like to add these changes?

Thank you again for the initiative! 💖

@jgarza9788
Copy link
Contributor Author

thanks for the suggestion.
i need to change the way i have my set up... since i'm copy and pasting from a VM.
since i didn't want to keep logging out of my real machine.

i'll try to resubmit soon :)

@jgarza9788
Copy link
Contributor Author

at 100 blur amount
the blur is a little patchy... but well worth the performance
and who's gonna pick 100 blur anyways ?
https://github.com/user-attachments/assets/496c3cbb-9fe9-4230-a999-3362bfa88883

@jgarza9788
Copy link
Contributor Author

ok i fixed the focus and ran the clang-format.sh

@Schneegans
Copy link
Owner

Well, if you ran clang-format.sh, you certainly did not commit the changes it made 😄

the blur is a little patchy... but well worth the performance

Yeah, that's why I called it "acceptable". But maybe you can allow the quality slider to move between [0.1 ... 2] with 1 being the default. So if anybody wants to, the patchyness could be resolved.

Also, you could experiment with the two factors directions and samples. Maybe it is sufficient to let the quality slider affect only one of them? Then the performance impact would be linear instead of quadratic...

@Schneegans
Copy link
Owner

I am using this since a couple of hours and I really like the effect 😄

@Schneegans
Copy link
Owner

Do you want to add the quality slider? Afterwards I would have a deeper look at the code and then we could merge this, I think!

@jgarza9788
Copy link
Contributor Author

yes, i think i'll do that later today :)

@jgarza9788
Copy link
Contributor Author

testing video
https://github.com/user-attachments/assets/e65390dd-9ed4-4a2d-b4f3-cb2838818d8f

even the lowest quality doesn't look bad

@jgarza9788
Copy link
Contributor Author

😅 I forgot the clang-format again

Copy link
Owner

@Schneegans Schneegans left a comment

Choose a reason for hiding this comment

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

Thank you so much! This really looks great now. I added a few minor remarks to the code. You should again try to run the clang-format.sh script as there are many empty lines added in various places which should be removed by the script.

Anyways, once these remaining issues are resolved, I am happy to merge this cool new effect!

I think I'll try to create a bit smoother and more compressed gif for the effect.

But yeah, thanks again!

README.md Outdated
@@ -50,6 +50,7 @@ This extension is not only more useless than the cube, but it is also much more
| **TV Effect** <br> This is a very simple effect to demonstrate that this extension could also be used in a more professional environment. | <img src ="docs/pics/tv.gif" /> |
| **TV Glitch** <br> This effect combines the Glitch and the TV Effect. | <img src ="docs/pics/tv-glitch.gif" /> |
| **Wisps** <br> Let your windows be carried away to the realm of dreams by these little fairies! | <img src ="docs/pics/wisps.gif" /> |
| **Focus** <br> Focus Dude, Focus | <img src ="docs/pics/focus.gif" /> |
Copy link
Owner

Choose a reason for hiding this comment

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

You could move this to the alphabetically correct position.

Comment on lines 34 to 42
// Ease-in-out cubic for alpha
float easeInOutCubic(float x) {
return x < 0.5 ? 4.0 * x * x * x : 1.0 - pow(-2.0 * x + 2.0, 3.0) / 2.0;
}

// Ease-in-out sine for blur
float easeInOutSine(float x) {
return -(cos(3.14159265 * x) - 1.0) / 2.0;
}
Copy link
Owner

Choose a reason for hiding this comment

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

These could be moved to common.glsl.

Comment on lines 49 to 51
const float directions = 15.0;
// const float samples = quality;

Copy link
Owner

Choose a reason for hiding this comment

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

I guess this could be removed.

Suggested change
const float directions = 15.0;
// const float samples = quality;
const float directions = 15.0;

Copy link
Owner

Choose a reason for hiding this comment

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

I am not sure what you changed here, but I think it is not related to this pull request, or is it? If not, I think you could undo all the changes you did here to keep the history clean.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ok, i can make a new branch and keep a clean history

Comment on lines 27 to 28
// This effect ... //
// <- Please add a description of your effect here -> //
Copy link
Owner

Choose a reason for hiding this comment

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

It would be cool if you added a short description here 😄

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes, i can

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.

2 participants