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

Rounding issue in setupInternalCanvasMatrix() #128

Closed
rabross opened this issue Nov 6, 2020 · 7 comments
Closed

Rounding issue in setupInternalCanvasMatrix() #128

rabross opened this issue Nov 6, 2020 · 7 comments
Labels

Comments

@rabross
Copy link

rabross commented Nov 6, 2020

Version 1.6.5

There's a problem in setupInternalCanvasMatrix() where a rounding issue can occur causing internalCanvas and rootView sizes to not match. This leads to some blurring issues at the bottom of BlurView.

This is down to using roundingScaleFactor during internalCanvas.scale() for height instead of the 'real' height scale factor.

I've fixed on my side by producing:
float nonRoundingScaleFactorHeight = (float) rootView.getHeight() / internalCanvas.getHeight();
and using that for scaling:
internalCanvas.scale(1 / scaleFactor, 1 / nonRoundingScaleFactorHeight);

It might be worth looking into.

@Dimezis
Copy link
Owner

Dimezis commented Nov 6, 2020

This might result in a slightly vertically stretched/squeezed image depending on the nonRoundingScaleFactorHeight.
Can you post a screenshot of what you're seeing? Also the size of the BlurView in that setup.

@rabross
Copy link
Author

rabross commented Nov 6, 2020

Thanks for getting back to me.
The (slightly hacky) solution above seems to be working well across devices.

Some screenshots without the fix.

Root view capture with extra row of transparent pixels at bottom (i.e. root has no background and no frame clear drawable)
pre-blur-capture

The problem is apparent in the post blur. The transparency has blurred into the image
post-blur

blurview-height

rootview-height

canvas-height

Here you can see the issue. This value is being round down to 204 when drawn to canvas which doesn't fill the 205 height of the canvas.
rounding-issue

With the fix
Screenshot 2020-11-06 at 23 01 33

@rabross
Copy link
Author

rabross commented Nov 8, 2020

Also in the above case:

rootView.getWidth() is 1080
internalCanvas.getWidth() is 192
internalBitmap.getWidth() is 192

SizeScaler scaleFactor is default 8

This gives a Size(192, 205, 5.625)

Running canvas.scale(1/5.625, 1/5.625) results in:
width 1080*0.17777778 = 192
height 1149*0.17777778 = 204.26666
The calculated height causing the issue when it is rounded down to 204, when 205 is required.
This is what leads to the single pixel height missing from the image.

@Dimezis Dimezis added the bug label Jan 26, 2021
@Dimezis
Copy link
Owner

Dimezis commented Jan 26, 2021

I'm not sure what to do here TBH.
It's obviously a bug and looks bad, however your fix is potentially squeezing/stretching the resulting image on the vertical axis.
On the blurred version it's not that noticeable, so I might go for it

@Dimezis
Copy link
Owner

Dimezis commented Jan 26, 2021

Alternative approach would be to scale by nonRoundingScaleFactorHeight in both directions, so it's preserving the aspect ratio, but goes slightly behind the borders on X axis

@Mickey34
Copy link

Hi,
I've faced the same issue recently but suggested changes to setupInternalCanvasMatrix() haven't solved my problem. I had to add the same calculation into BlockingBlurController.draw() method to make it work:

float nonRoundingScaleFactorHeight = (float) blurView.getHeight() / internalCanvas.getHeight();
canvas.save();
canvas.scale(scaleFactor, nonRoundingScaleFactorHeight);
canvas.drawBitmap(internalBitmap, 0, 0, paint);
canvas.restore();

I also wasn't able to use the solution with calculating the real rounding factor from rootView.getHeight(). It caused problems when rootView height is much more than blurView height. For example, nonRoundingScaleFactorHeight was 32 while scaleFactor was near 4.5 (don't remember exact number) for the case where rootView height is 1152 but blurView height is 96 pixels. As a result, my item wasn't blurred at all.
When I tried blurView instead of rootView for calculating nonRoundingScaleFactorHeight all issues disappeared.

@Dimezis
Copy link
Owner

Dimezis commented Jan 27, 2021

@Mickey34 rootView height shouldn't be used, it's just that the OP has rootView and BlurView of the same size.
You should use BlurView's height to make that trick work

@Dimezis Dimezis closed this as completed in 58db1f4 Apr 3, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants