Comments (6)
Sorry, I should have better documented the fix. The bug manifested when calculating bounding boxes here:
https://github.com/aframevr/aframe/blob/master/src/components/obb-collider.js#L66
The geometry component was not initialized when the hasLoaded
flag was true and the bounding box initialized to zero. Notice the obb-collider
component doesn't depend on the geometry component directly (or any others that might set a mesh). An entity with an obb-collider could have a gltf-model
component and not a geometry one. Just want to know that all components have initialized before calculating the bounding box.
I myself always assumed that components would be ready after the loaded
event fires (and corresponding hasLoaded
flag set) as stated in docs
Whether the entity has attached and initialized all of its components
What would it mean that an entity has loaded if components have not also initialized?
from aframe.
We have to fix all the kinks
from aframe.
If can generate some tests where the new code fails I'm happy to look at it.
from aframe.
The geometry component was not initialized after the 'loaded' event fired and the bounding box size was zero. Notice the component doesn't depend on the geometry directly (or any others that might set a mesh). Just need to know the components have initialized before performing the calculation.
That does indeed illustrate the reason for the change, but isn't it now using the hasLoaded
and loaded
event as a proxy to know when the geometry component initialized? In fact, all it should really care about is updating the bounding box during initialization and upon each change to the underlying mesh(es).
Listening for object3dset
and object3dremove
should probably do the trick. This eliminates the need to specifically listen to the model-loaded
event, handles cases when the geometry component is added or changed post-initialization, and where child entities change their mesh(es).
What would it mean that an entity has loaded if components have not also initialized?
The guarantee that the loaded state of an entity currently gives pertains to its children. An entity is loaded iff assets have loaded and child entities have emitted the loaded
event (components initialized). This has the neat property that there's exactly one state transition, after which the entity is ready to initialize and/or update its components. This is regardless of when/where these component inits/updates come from, whether they were already set on the entity pre-load or dynamically set afterwards.
By changing this semantic, there are now three states: not loaded, initializing, loaded. That is also where the mentioned regression stems from, the code doesn't properly distinguish the first two states in all cases, leading to components initializing while the entity is in effect not loaded and not yet initializing its components.
If a component should robustly handle initialization ordering issues, the updated behaviour of hasLoaded
/loaded event is of limited use, as it doesn't provide a solution in case the component is dynamically added post-load. In practice it's generally possible to find event-driven approaches that work regardless if its the initial initialization of the entity or not.
That being said, the documentation differing from how it works and is documented in code is far from ideal. And the fact that the hasLoaded
flag can be observed true before the loaded
event (which also provides additional guarantees) is a potential pitfall. I'm just not sure that the added complexity and change in behaviours is worth it. In fact, I don't think components should ever care whether they are initialized as part of the "initial entity initialization" or "post loaded".
from aframe.
Listening for object3dset and object3dremove should probably do the trick. This eliminates the need to specifically listen to the model-loaded event, handles cases when the geometry component is added or changed post-initialization, and where child entities change their mesh(es)
It's not enough. there are many other factors that affect the size of a bounding box. e.g adding children entities. I want to run the bounding box calculation after all components have loaded because cannot anticipate all scenarios before hand. I covered model-loaded
as well since loading a model is async and loading a model common. I can make more granular and robust the logic over time but making sure the bounding box calculation runs after components have loaded will catch a lot of scenarios without ad-hoc logic.
Definitively my intent with the loaded
event and that the hasLoaded
flag is to indicate that the entity and components have loaded.
If you produce some simple examples or tests that fail with the new logic and not the old one I'll be happy to fix.
from aframe.
It's not enough. there are many other factors that affect the size of a bounding box. e.g adding children entities. I want to run the bounding box calculation after all components have loaded because cannot anticipate all scenarios before hand.
I was under the impression that object3dset
and object3dremove
did a bit more heavy lifting, given its usage in the raycaster. While they do properly report child entities being added and models being loaded, you're right that they have severe shortcomings when it comes to removal of child entities or geometry changes.
However, it still feels like an XY problem in the context of this component. IMHO the component shouldn't care about entity initialization, but simple compute the bounding box and recompute upon each change (possible batched using a dirty flag). This way there is only one scenario to think about, nothing ad-hoc needed. The fact that there is no combination of events to easily achieve this, is potentially worth addressing. But that's beside the point of this issue.
Definitively my intent with the loaded event and that the hasLoaded flag is to indicate that the entity and components have loaded.
Having hasLoaded
and the loaded
event in agreement is good. With the entity loading state becoming a tri-state, modelling it as such will likely make the code easier to read and debug.
If you produce some simple examples or tests that fail with the new logic and not the old one I'll be happy to fix.
Here's a simple example where the resulting plane should be square (as it is in 1.5.0 and prior), but ends up elongated on master: https://glitch.com/edit/#!/puddle-awesome-coal?path=index.html
Here's an example where the removal of a mixin no longer takes effect: https://glitch.com/edit/#!/petal-absorbing-viscountess?path=index.html
Here's an example where an early initialization (triggered by .setAttribute
) results in an additional update
, which doesn't occur with 1.5.0: https://glitch.com/edit/#!/mixed-orange-raven?path=index.html
And of course the example with aframe-environment-component in the opening post. Although with #5426 merged, the erroneous behaviour now no longer results in an error. Though I believe it has the same cause as the first example.
from aframe.
Related Issues (20)
- 2w- HOT 1
- xr-mode-ui: Add explicit full screen mode HOT 8
- Video playback stops when entering immersive mode in Vision Pro HOT 21
- <a-scene> accessing my webcam HOT 4
- Hiding VR mode UI not working in 1.5.0 HOT 2
- a-videosphere: 180 degree support and SBS support. HOT 2
- Component/attribute inheritance and precedence for primitives HOT 5
- ar hit test can only place once,but i need it can place many times HOT 1
- Updating reflection not possible when using postprocessing HOT 2
- Sudden increase in GPU load when using postprocessing HOT 3
- Utils device isIOS and isMobile checks fail on iPad Pro 12.9 Safari HOT 5
- oob-collider not setting el.object3D as default HOT 16
- Add documentation for before/after API
- Duplicate pinch events on the Quest Pro HOT 3
- OBB isn't centered (includes proposed fix) HOT 6
- Support for transient input on Vision Pro HOT 1
- a-scene Chrome non-passive violation warnings for touchmove and touchstart event handlers HOT 5
- GLB frosted glass material only marginally affect primitives, doens't affect <a-text> or <a-image> HOT 3
- hand-tracking-controls component detaches children (+fix & ideas) HOT 2
- Loading Insta360 Videos HOT 1
Recommend Projects
-
React
A declarative, efficient, and flexible JavaScript library for building user interfaces.
-
Vue.js
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
-
Typescript
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
-
TensorFlow
An Open Source Machine Learning Framework for Everyone
-
Django
The Web framework for perfectionists with deadlines.
-
Laravel
A PHP framework for web artisans
-
D3
Bring data to life with SVG, Canvas and HTML. 📊📈🎉
-
Recommend Topics
-
javascript
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
-
web
Some thing interesting about web. New door for the world.
-
server
A server is a program made to process requests and deliver data to clients.
-
Machine learning
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
-
Visualization
Some thing interesting about visualization, use data art
-
Game
Some thing interesting about game, make everyone happy.
Recommend Org
-
Facebook
We are working to build community through open source technology. NB: members must have two-factor auth.
-
Microsoft
Open source projects and samples from Microsoft.
-
Google
Google ❤️ Open Source for everyone.
-
Alibaba
Alibaba Open Source for everyone
-
D3
Data-Driven Documents codes.
-
Tencent
China tencent open source team.
from aframe.