-
Notifications
You must be signed in to change notification settings - Fork 1.9k
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
drawBokehEffect less blurred on safari #8331
Comments
Problem occurs with both body-segmentation and body-pix models (it looks like the cpuBlur code is used in both for Safari). |
Hi, @groupboard We apologize for the delay in our response. Thank you for raising this issue concerning the bokehEffect appearing less blurred on Safari compared to Google Chrome. We have replicated this behavior using the demo:https://storage.googleapis.com/tfjs-models/demos/body-segmentation/index.html?model=selfie_segmentation on both Safari browser (Version 17.5 (19618.2.12.11.6) and Google chrome browser (Version 126.0.6478.127 (Official Build) (arm64)) and there is bokehEffect less blurred on Safari at the moment If you have a workaround to address this inconsistency, we welcome pull request (PR) from your end. Our team will thoroughly review your PR and integrate it if it adheres to our codebase standards. Thank you for your cooperation and patience. |
This issue has been marked stale because it has no recent activity since 7 days. It will be closed if no further activity occurs. Thank you. |
This issue was closed due to lack of activity after being marked stale for past 7 days. |
Describe the current behavior
When drawBokehEffect is called with blurAmount=3, it doesn't blur enough on Safari
Describe the expected behavior
Should blur the same amount on safari as on chrome and firefox.
Standalone code to reproduce the issue
Go to: https://storage.googleapis.com/tfjs-models/demos/body-segmentation/index.html?model=selfie_segmentation and choose bokehEffect as the visualization.
Other info / logs
The bug is in this line of cpuBlur:
const step = blur < 3 ? 1 : 2;
It should really just set step to 1 all the time, I think. It looks like this code was taken from a codepen, and it is perhaps a hack to make it work a bit faster (but with incorrect results) for blur values > 2. Certainly it seems to work differently from how the blur is implemented in the browser.
Also: do we still need to use cpuBlur on Safari? It would be worth checking to see if that code can be ditched now. The code (from 6 years ago) doesn't say why cpuBlur is used on safari. Presumably due to a bug or performance issue, which might have since been fixed.
The text was updated successfully, but these errors were encountered: