search

ReactVision / viro

ViroReact: The AR and VR library for React Native 📳💙💛🤍💚
MIT License
1.4k stars 164 forks source link

Chroma key filtering is useless #76

Open ramicaza opened 2 years ago

ramicaza commented 2 years ago

Environment

Please provide the following information about your environment:

  1. Development OS: Mac
  2. Device OS & Version: iOS any useable version
  3. Version: ViroReact version 2.21.1, RN Version 0.64.2
  4. Device(s): physical iPhone

Description

The chroma key filtering is useless, the range of colors it filters is far far too wide. Yes I've tried green, blue AND red screens. I've dug through the code of ViroCore to find the issue and it's in line 902 of VROShaderFactory.cpp.

chroma_threshold_sensitivity is set to a hardcoded value of 0.4 and ideally this is variable from the JS API but at the minimum this should be set to a more reasonable default.

This issue is compounded by the fact the documentation on how to build the whole library is minimal and out of date. I'm sure this project would get an influx of community support if there were some simple dev build instructions!

Update: I've built the library from scratch and confirmed that changing the aforementioned line to a lower value (0.1) does the trick. Issue still stands.

Reproducible Demo

Insert a green/blue/red screen video into AR using ViroReact and observe that too many colors are filtered out.

robertjcolley commented 2 years ago

@ramicaza is there a corresponding issue in virocore you could link?

ramicaza commented 2 years ago

Actually, in PR 63 (not yet merged) I changed the chroma_threshold_sensitivity to a more reasonable default. Would you like me to open an issue over there to address the fact that the value is not settable from the JS API?

robertjcolley commented 2 years ago

@ramicaza I think so. The ViroCore API should expose getters and setters for that. The JS layer is entirely in ViroReact, so there is no need to include any mention of JS in ViroCore.

What will need to happen is:

  1. ViroCore needs to add getters and setters
  2. ViroReact implements (with ViroCore and bridging functions) the getters and setters in ViroReact.
robertjcolley commented 2 years ago

@ramicaza PR #63 was merged - should this be closed? or should this be updated to reflect the Feature Request to add a react means of updating this value?

SamiChab commented 2 years ago

@ramicaza thanks for your contribution to this project ! Can you please tell us what to do with this ?

@ramicaza PR #63 was merged - should this be closed? or should this be updated to reflect the Feature Request to add a react means of updating this value?

gokugen commented 2 years ago

Hello is there a way to change the value chroma_threshold_sensitivity through ViroReact?

I need to play with it in order to remove the border color around the character.

andresteves commented 1 year ago

@ramicaza does your PR solve the situation that @gokugen mentioned? I happen to have the same issue.