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

add shadowCameraAutomatic option #449

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

add shadowCameraAutomatic option #449

wants to merge 2 commits into from

Conversation

Algorush
Copy link
Collaborator

@Algorush Algorush commented Jan 8, 2024

  • add shadowCameraAutomatic option with #default-street element for automatic shadow camera sizing
  • delete shadowCamera parameters

add shadowCameraAutomatic option
delete shadowCamera parameters
Copy link

netlify bot commented Jan 8, 2024

Deploy Preview for 3dstreet-core-builds ready!

Name Link
🔨 Latest commit c995f2f
🔍 Latest deploy log https://app.netlify.com/sites/3dstreet-core-builds/deploys/659f4e3ed79e6a000859a6ce
😎 Deploy Preview https://deploy-preview-449--3dstreet-core-builds.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@Algorush
Copy link
Collaborator Author

Algorush commented Jan 8, 2024

I'm still testing this option for automatic shadow camera sizing. It is good for determining the size of the shadow area. But I want to test whether the performance has changed

@Algorush
Copy link
Collaborator Author

Algorush commented Jan 11, 2024

I noticed that if use the shadowCameraAutomatic option for the light component, the scene slows down noticeably. This happens after turning on the editor and going to the viewer again. Perhaps because with this option the size of the object passed to shadowCameraAutomatic is calculated every tick, but not sure.
I sized the shadow camera according to the length of the street and added 10 meters to each side since the camera is aimed at an angle. For all enviroment modes, shadows from all objects are displayed on the scene. Tested with different street length

@kfarr
Copy link
Collaborator

kfarr commented Jan 15, 2024

A couple thoughts:

  • I'm not sure "default-street" length is the source of truth for how to position camera shadow in a scene -- there may be multiple streets, intersections or other user-defined geometry in the scene. This element id may not always be present.
  • What is the best way to test this? What is the goal here? Is it to have shadow on any object that is in the camera field of view? Is that possible with A-Frame?
  • You mentioned performance issues with shadowCameraAutomatic. Is shadowCameraAutomatic supposed to automate the above? Does it work for this use case ignoring the performance issue? If so maybe it's worth digging a bit more to fix the root cause?

@Algorush
Copy link
Collaborator Author

Algorush commented Jan 15, 2024

I'm not sure "default-street" length is the source of truth for how to position camera shadow in a scene -- there may be multiple streets, intersections or other user-defined geometry in the scene. This element id may not always be present.

Yes, I thought about that too. Maybe it's worth storing length in a STREET object?

What is the best way to test this? What is the goal here? Is it to have shadow on any object that is in the camera field of view? Is that possible with A-Frame?

Yes, the goal here is to have shadows on all objects. I came from here:
#439
Yes, it's possible in AFrame, why not? Just need to adjust the light.shadow.camera area. The problem is that this area is adjusted to the light source, not the viewer's camera. It would be possible to simply specify very large values ​​for the shadow.camera area, but then the shadow becomes pixelated in small scenes

Is shadowCameraAutomatic supposed to automate the above? Does it work for this use case ignoring the performance issue? If so maybe it's worth digging a bit more to fix the root cause?

Yes, this works if we specify in shadowCameraAutomatic a scene element in the scene that the light and shadow should cover. But what the root cause you mean? Not quite understand
On my computer, after adding this setting, there are quite severe performance leaks. After turning the editor on and off

@Algorush
Copy link
Collaborator Author

Algorush commented Jan 16, 2024

I'm not sure "default-street" length is the source of truth for how to position camera shadow in a scene -- there may be multiple streets, intersections or other user-defined geometry in the scene. This element id may not always be present.

Maybe we just can use #street-container instead of #default-street? And calculate #street-container size to use them for shadow.camera sizes

@kfarr
Copy link
Collaborator

kfarr commented Jan 16, 2024

@Algorush yes one level higher? So in theory shadowCameraAutomatic automatically adjusts for the combination of the camera frustum and the bounding box of the target entity?

looks like yes

@Algorush
Copy link
Collaborator Author

Algorush commented Jan 24, 2024

I looked at the code of the aframe light component and it turns out that the size of the element, or rather the array of elements passed to shadowCameraAutomatic, is calculated every tick, if this option is enabled:
https://github.com/aframevr/aframe/blob/770f95a0cda0ad083c6d6f8d6ce4399804f238d5/src/components/light.js#L155
Therefore, this led to severe performance losses when using it. I can take a function from the light component that determines the exact dimensions of the shadowCamera, but I think that it is very large and in our case it is enough to simply specify the maximum side of the vector with the size of streetContainer. And add +/-10 meters to take into account that the camera light is at an angle. I'll test with different light angles again

@Algorush
Copy link
Collaborator Author

Algorush commented Jan 28, 2024

calculating the dimensions of a street-container is a little more difficult than it seems. We can't use the element's loaded event since we are loading the content via JS. I tried different options, the option worked well when I added shadowCameraAutomatic with the street-container element, and then turned off this option through setTimeout, after few seconds (5 is enough). Works good. But on this scene (https://streetmix.net/kfarr/3/3dstreet-demo-street) an error occurs. It looks like the problem is that one of the segments did not load and has NaN values in one of properties

@Algorush
Copy link
Collaborator Author

Algorush commented Jan 28, 2024

I'll try to do something about it next time. For now I'll deal with other issues.
I also suggest adding a check to aframe-streetmix-parser.js: if the chosen segment type is not supported, don't load it or load the sidewalk segment instead or use the neighboring segment type? Will make new issue for this.
And if we add such a check, then we can use the option with shadowCameraAutomatic and setTimeout for the first time

@Algorush Algorush marked this pull request as draft February 5, 2024 19:40
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.

None yet

2 participants