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

A-Frame sort order flexibillity can be improved #5332

Closed
diarmidmackenzie opened this issue Jul 8, 2023 · 15 comments
Closed

A-Frame sort order flexibillity can be improved #5332

diarmidmackenzie opened this issue Jul 8, 2023 · 15 comments

Comments

@diarmidmackenzie
Copy link
Contributor

diarmidmackenzie commented Jul 8, 2023

Description:

  • A-Frame Version: 1.4.2
  • Platform / Device: All
  • Reproducible Code Snippet or URL: None

I've been doing a deep dive into A-Frame / Three.js sortObjects, prompted by some difficulties I had getting a "hider material" to work for a Mixed Reality scene I was working on.

A-Frame sortObjects is described here as follows:

Sorting is used to attempt to properly render objects that have some degree of transparency. Due to various limitations, proper transparency often requires some amount of careful setup. By default, objects are not sorted, and the order of elements in the DOM determines order of rendering. Re-ordering DOM elements provides one way of forcing a consistent behavior, whereas use of renderer="sortObjects: true" may cause unwanted changes as the camera moves.

However this description fails to mention some important details about how the THREE.js sortObjects option actually works.

When sortObjects is enabled, sorting of objets happens according to the following logic (see code here)

  • 1st factor is any renderOrder value set on any Group the Object3D is a member of.
  • 2nd factor is any renderOrder value set on the Object3D itself
  • 3rd factor is the id of the Object3D's material (this is done for performance reasons - see this checkin)
  • 4th factor (only if ordering can't be decided by any of the above) is z-distance of the object from the camera (the object's center, of course, which may or may not be useful - e.g it's very much not useful for a sky...)

The A-Frame sortObjects option + docs were added under #3424, following discussion under #666.

#666 includes various reports of the render order changing as the camera moves around the scene (e.g. this) - which explains the warning about sortOrder that was added to the docs.

However this is only true for objects that use the same material. For obects using different materials, sort order is determined by material id. That's been true since THREE.js r69.

So in fact, enabling sortObjects enables a bunch of different things, not just the z-distance sort mentioned in the docs:

  1. The ability to control sort order directly using Object3D.renderOrder
  2. A performance optimization to render objects in material order (comments in the THREE.js checkin suggest this could be pretty significant)
  3. z-distance from camera

Ideally, it would be great to be able to control enabling / disabling of each of these individually. THREE.js doesn't offer a native way to do that, but it does allow specification of a custom sort function, which could be used to do a pre-render sort that can be configurable using properties on the renderer component.

In terms of default settings, I agree with the discussion in #666 that says that stable ordering is preferable. But of the above, both 1 & 2 are stable - only 3 results in position-based instability.

So I'd propose that by default:

  • Object3D renderOrder settings should be considered
  • Material sorting (which may boost performance) should be enabled
  • z-distance from camera should be disabled.

And then there should be options to:

  • enable z-distance sorting (existing function, which we can retain)
  • disable material sorting (particularly useful for the case where z-distance sorting should take priority)
  • and maybe an option to disable sorting completely - helps ensure back compatibility, and may be useful in cases where the sorting itself causes a performance hit.

I'm happy to do a PR on this, but keen to gather input first on what others might want / need here.

For my original use case (a "hider material") I need to ensure this is rendered before all other objects. There are several ways to achieve this, but the simplest is probably to use a renderOrder of -1 on the object. I can do that with current A-Frame, as long as I enable renderer="sortObjects:true". That does have the side effect of also enabling z-distance sorting for same-material objects, which I don't actually need, but doesn't seem like it will cause too many problems...

@diarmidmackenzie
Copy link
Contributor Author

diarmidmackenzie commented Jul 8, 2023

I just noticed that reversePainterSortStable (used for sorting transparent objects) doesn't sort based on material id.

So my comment that render order instability only occurs for same-material objects is incorrect. This instability can occur for same-material objects or for transparent objects. It's probably the latter case that was being described in #666.

@DougReeder
Copy link
Contributor

Whatever code changes we make, we should write a guide outlining different strategies to solve sort order issues.

@diarmidmackenzie
Copy link
Contributor Author

diarmidmackenzie commented Jul 8, 2023

Created a glitch that demonstrates a performance boost from setting sortObjects: true.
https://glitch.com/edit/#!/sort-objects-performance-boost?path=index.html%3A52%3A6
(you'll need to renix this and set sortObjects: true) to see the difference. On my laptop I see a ~20% perf gain from ~35fps to ~42fps.

However the example is a little contrived:

  • there's no appreciable difference unless the different materials run different "programs". I forced this by using different shaders (Standard, Basic, Phong, Lambert). There are probably other ways to force this that may be more realistic, but just using multiple colors is not enough to trigger multiple programs.
  • in order for sorting to work correctly, you have to explicitly use the same Material. If you use the regular material component, A-Frame creates a new material for each entity, and sorting by material id doesn't end up with objects rendered in an optimized order.

I suspect with most A-Frame scenes there will be little to no discernable performance benefit - you need to carefully set up your materials in a non-standard way to benefit from the optimization.

We could perhaps make enhancements to A-Frame material component to better leverage the potential for optimization here, but that's something I'd want to handle as a separate enhancement.

As things stand, I don't think enabling the material sorting that THREE.js does is actually going to have any positive benefit for A-Frame...

@dmarcos
Copy link
Member

dmarcos commented Jul 10, 2023

Thanks for the write up! Open to revisit the sort order. What was exactly your problem with the "hider material"? It's easier for me to understand and have a conversation over specific use cases.

@diarmidmackenzie
Copy link
Contributor Author

Ok, my use case is for passthrough on Quest with XR planes for room layout.

When you set up a room layout, you define the walls, ceiling, floor + furniture like desks etc., and then a WebXR experience can query a set of XRPlanes.

Various things are needed for these to be incorporated "realistically" into a WebXR experience.

One aspect is physics, so that when an object lands on a desk, it stops there, rolls off etc., rather than falling through.

Another is occlusion, so when a ball rolls under a desk, you want the desk to occlude the ball.

I'm doing this occlusion using a "hider material", which is just a material with colorWrite set to "false".

That works fine, but you need it to be rendered before any other objects in the scene.

One way to do this is to use A-Frame default ordering and make it the first object in the DOM. However because of the way the WebXR planes are reported (I'm using Meta's Reality Accelerator Toolkit), that wasn't a very natural thing to do.

In the end the solution was simple:

  • set renderOrder = -1 on the Mesh for the hider material
  • set renderer=sortObjects:true on the a-scene

However, it took me a while to understand what was going on....

I first tried enabling sortObjects: true as the docs made me think this would give z-distanced based ordering, which would have been enough. But what I actually got was objects rendered in material index order (which was not described in any documentation anywhere,.so I had no idea what was going on, it just wasn't working).

Then I got rid of sortObjects:true and tried renderOrder = -1 on the Mesh, which also didn't work, as renderOrder is ignored when sortObjects is false....

Had to read through the Three.js code to figure out what was going on, at which point it finally all made sense...!

Minimum requirement here is better documentation.

However #666 correctly points out that z-distance ordering is undesirable in some cases.

I think it would be very useful to be enable to activate renderOrder configuration, and get the perf benefits of material-index ordering, but without also enabling z-distance ordering.

Best interface is probably 3 flags to independently toggle each of the 3 types of ordering, and IMO it would be better for the 1st two types to be enabled by default.

@dmarcos
Copy link
Member

dmarcos commented Jul 10, 2023

thanks. small tangent. should we do anything to improve support for XR planes?

@diarmidmackenzie
Copy link
Contributor Author

I have built this...

https://github.com/diarmidmackenzie/aframe-components/tree/main/components/xr-room-physics

which rolls together physics, shadows and occlusion for XR Planes.

It's built on Meta's Reality Accelerator Toolkit, though actually the abstractions provided on their APIs weren't very helpful and I ended up having to work at a lower level for much of what I wanted to do.

I'm planning to offer some suggestions for improvements to ratk, and maybe a couple of PR.

I don't know what all the various use cases for XR Planes are, and without a clear view of that it might be hard to get the right presentation of the API.

A thin A-Frame wrapper on top of ratk might be nice, but if the APIs aren't quite right, we'd risk making things even less accessible by putting another layer in between the application developer and what they actually need.

My inclination would be to see wait and see what happens with community components, for now.

For more experienced people there are no real barriers to working with XR Planes, and less experienced Devs are more likely to want all-in solutions that just make e.g. physics work with XR Planes, meaning they may not need an explicit A-Frame presentation of the XR Plane itself.

@dmarcos
Copy link
Member

dmarcos commented Jul 11, 2023

I've heard (but not sure) planes now return some semantic info? (table, wall...)? Might be cool / useful to have an A-Frame component able to render those? <a-entity xr-plane="type: wall">

@dmarcos
Copy link
Member

dmarcos commented Jul 11, 2023

A PR for this we can discuss would be cool. thanks!

So I'd propose that by default:

Object3D renderOrder settings should be considered
Material sorting (which may boost performance) should be enabled
z-distance from camera should be disabled.
And then there should be options to:

enable z-distance sorting (existing function, which we can retain)
disable material sorting (particularly useful for the case where z-distance sorting should take priority)
and maybe an option to disable sorting completely - helps ensure back compatibility, and may be useful in cases where the sorting itself causes a performance hit.
I'm happy to do a PR on this, but keen to gather input first on what others might want / need here.

@mrxz
Copy link
Contributor

mrxz commented Jul 12, 2023

I'm not entirely convinced that there's much benefit in the material sorting in the case of A-Frame. Since most uses of A-Frame probably target mobile GPUs you'd not only want to limit calls, but especially try to avoid (excessive) overdraw (z-sorting).

Since materials in A-Frame get unique ids the material sorting will not result in a clear improvement, but it will thwart the z-sorting from being effective. And as you've noticed, the materials would have to use different underlying programs to really result in the worst case. Or in other words, if users reduce the amount of different (types of) materials in their scenes it would already benefit performance (regardless of sorting, excluding worst-case alternating patterns).

Personally I think it would be fine to have two modes provided out of the box:

  • no-sorting: The current default, this allows users to control the render order by tree-order. Limiting material types and placing objects front to back in the DOM can be done to increase performance. Easy to understand, backwards compatible. Mostly suited for stationary (XR) or single viewpoint (non-immersive) experiences.
  • z-sorting: Sorting based on groupOrder, renderOrder and z distance. Limiting material types is still advisable for performance reasons. Suitable for scenes with a lot of motion by either the user or the elements in the scene.

It might be worth considering making sorting the default approach, as I feel that the manual order, while easy to understand, isn't intuitive per se (though that's a separate discussion).

@dmarcos
Copy link
Member

dmarcos commented Jul 13, 2023

good point about overdraw. thoughts? Should we default to z-sorting then?

@diarmidmackenzie
Copy link
Contributor Author

diarmidmackenzie commented Jul 14, 2023

Thanks for your thoughts @mrxz .

I can think of various cases where more flexibility is desirable:

  • material sorting will be useful in some cases. A-Frame material component is pretty limited, so it's common to set up THREE.js materials by hand, meaning shared materials are not uncommon

  • Resolve alpha depth issues automatically #666 presented cases where the instability of z-ordering is undesirable for transparent objects. So it would be helpful to be able to respect groupOrder and renderOrder settings, without enabling z-ordering.

  • what's being referred to as "DOM order" isn't really DOM order - it's three.js tree order. That's different when objects get reparented, which is common for e.g. grabbing entities with controllers, and can lead to weird effects where transparency suddenly breaks when an entity is grabbed.

I'm leaning towards an approach where we build an external component that is highly flexible, and for A-Frame core we focus on making the documentation of the current behaviour much better, with signposting to an external component for greater flexibility.

That said, if we were to reconsider the default for A-Frame, and maybe a little more flexibility, I think I'd advocate...

  • for transparent objects:

    • respect renderOrder & groupOrder (users who don't want to use them, can simply not set them)
    • option to sort in reverse z-order (far to near), or three.js tree order (which is same as DOM order, aside from Object3D re-parentings). Leave default as three.js tree order (as today) for back compatibility.
  • for opaque objects

    • respect renderOrder & groupOrder (users who don't want to use them, can simply not set them)
    • a;ways sort in z-order (near-to-far) as this should be GPU-optimal. Not concerned about instability of this ordering, as this primarily affects performance, not appearance.

In terms of change from current implementation, that amounts to:

  • the "sortOrder" parameter now only affects transparent objects, not opaque objects (opaque objects are always sorted)
  • renderOrder & groupOrder are always respected
  • zOrder peferred to material order for opaque objects (as more likely to be an appropriate optimization for A-Frame)
  • much better documentation about behaviour in this area & alternatives.

Further flexibility can be provided by external components, which can be referenced by the documentation.

I can do a PR for this + create an external component with more flexibility in the next ~week if there's no strong disagreement on this approach.

@mrxz
Copy link
Contributor

mrxz commented Jul 14, 2023

@diarmidmackenzie totally agree on the point that there are situations where more flexibility is useful. Delegating that to a dedicated external component is a good idea. That way the core implementation can provide a sane default with a small API surface.

I'm well aware that "DOM order" isn't really DOM order, but conceptually it holds pretty well and I think for users of A-Frame that use it more as a low-/no-code solution it's an easy way to resolve issues with layered transparency in their scenes. Though the documentation of this sorting behaviour and implications could definitely be improved.

That being said, I wonder if it might be better to actually drop this behaviour (or at least, not default to it). Since there's no single 'correct' way to sort transparent objects, I'm not surprised that there are cases where a scene authored for one approach fails when switching and vice versa. So if we ever want to default to it being reverse z-sorted, it will require some people to make changes when upgrading (i.e. specifying render order, or opting out of transparency sorting), but at the same time:

  • Will ensure there's one unambiguous way to manually specify the order (renderOrder and groupOrder)
  • Aligns closely with the default Three.js behaviour
  • Handles the case of transparent objects switching order or being viewed from different directions

In other words, a wider range of transparency issues can be tackled, which is a superset of the ones the "DOM order" allows for. Though it comes at the cost of breaking backwards compatibility for scenes relying on the current implicit behaviour.

Either way, the changes as you've outlined seem fine to me. Perhaps renaming sortObjects to sortTransparentObjects and printing a deprecation warning when people use sortObjects

@dmarcos
Copy link
Member

dmarcos commented Nov 16, 2023

Is there any additional work to be done here?

@diarmidmackenzie
Copy link
Contributor Author

Yes, I don't think there's anything outstanding here. I'll close this. Thanks for your support in getting these changes in.

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

No branches or pull requests

4 participants