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

Fix: wrong activity background blurring #179

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

luongvo
Copy link

@luongvo luongvo commented May 17, 2022

TODO

final float minBlurRadius = 10f;
final float step = 4f;

//set background, if your root layout doesn't have one
final Drawable windowBackground = getWindow().getDecorView().getBackground();
final View windowBackground = getWindow().getDecorView();
Copy link
Owner

Choose a reason for hiding this comment

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

That's not what I meant :)
Please set it up like this. No need to change library's API

ViewGroup decorView = (ViewGroup) getWindow().getDecorView();
topBlurView.setupWith(decorView)
        .setBlurAlgorithm(algorithm)
        .setBlurRadius(radius);

bottomBlurView.setupWith(decorView)
        .setBlurAlgorithm(new RenderScriptBlur(this))
        .setBlurRadius(radius);

Copy link
Author

Choose a reason for hiding this comment

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

@Dimezis From your comment on setupWith

The lower amount of Views are in the root, the better for performance.

I agree with this point, so changing root view is not the thing I'm trying to do with the fix. I'm still would like to add a support for the lib's API to support getting decor background by getWindow().getDecorView() for blurring. This approach brings better performance, right?

Again, I think this logic could help https://github.com/mmin18/RealtimeBlurView/blob/master/library/src/com/github/mmin18/widget/RealtimeBlurView.java#L245-L261 🤔

Copy link
Owner

@Dimezis Dimezis May 17, 2022

Choose a reason for hiding this comment

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

Again, I think this logic could help

If you squint your eyes, you'll see that's exactly what that library does - it draws the whole DecorView on the Canvas, and that's exactly why it works.

I'm still would like to add a support for the lib's API to support getting decor background by getWindow().getDecorView() for blurring

I'm not sure what you're trying to achieve in this PR, the bug is still there and this code essentially works the same as before - it takes the background from the DecorView. But instead of passing it directly, you pass the DecorView and take its background later.

This approach brings better performance, right?

The code in this PR performs identically to the original code.

If you're asking about the performance of drawing the whole DecorView when it's passed as a root, it is indeed slightly worse in theory, but I don't think you'll see a measurable difference.

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