tensorflow / tfjs

A WebGL accelerated JavaScript library for training and deploying ML models.
https://js.tensorflow.org
Apache License 2.0
18.35k stars 1.92k forks source link

drawBokehEffect less blurred on safari #8331

Closed groupboard closed 1 month ago

groupboard commented 2 months ago

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.

groupboard commented 2 months ago

Problem occurs with both body-segmentation and body-pix models (it looks like the cpuBlur code is used in both for Safari).

gaikwadrahul8 commented 1 month ago

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.

github-actions[bot] commented 1 month ago

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.

github-actions[bot] commented 1 month ago

This issue was closed due to lack of activity after being marked stale for past 7 days.

google-ml-butler[bot] commented 1 month ago

Are you satisfied with the resolution of your issue? Yes No