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

WebGLRenderer - Implemented setSortOverride. #24809

Conversation

SeanPeplinski
Copy link

@SeanPeplinski SeanPeplinski commented Oct 17, 2022

Related issue: #24811

Description

There are many sorting use-cases that aren't possible to implement with only a custom Javascript sort comparison function (via WebGLRenderer.setTransparentSort and WebGLRenderer.setOpaqueSort). This PR allows the entire sort method in WebGLRenderList to be overridden with a callback that accepts parameters for all object lists and the sorting methods (either the default methods, or the custom sorting methods specified in WebGLRenderer).

A quick example use-case is logical grouping. Perhaps objects should be grouped by some condition and then sorted independently as a group by a certain sorting algorithm. Then, all groups should be sorted against each other by some other algorithm, like z-distance to the camera.

@Mugen87
Copy link
Collaborator

Mugen87 commented Oct 18, 2022

Sorry, but I'm not in favor of this change. This makes it possible for rendering transparent and opaque objects in a single render list which is no good idea, IMO. It opens up the door for many glitches that we have to support and explain.

I think it's better if you modify the renderer by yourself.

@SeanPeplinski
Copy link
Author

I understand that. Is there any interest in an alternative solution like modifying setTransparentSort and setOpaqueSort to accept a callback instead of a sort compare function?

So, for example: renderer.setTransparentSort((objects, sortCompareFunction) => { ... })), where 'sortCompareFunction' is reversePainterSortStable and can be ignored if sorting with a custom algorithm.

I think it would benefit the API to not limit sorting to a Javascript 'sort' call.

@Mugen87
Copy link
Collaborator

Mugen87 commented Oct 18, 2022

I don't think it's worth to break the current usage of both setTransparentSort() and setOpaqueSort() for this specific use case. TBH, I also have a hard time to understand why a custom compare function for Array.sort() isn't sufficient.

@SeanPeplinski
Copy link
Author

As for my specific use case, I am designing support for 2D content, similar to DIVs in HTML.

Objects in each 2D content container need to be sorted independently based on a zOrder value that I have populated (pretty much DOM order of the scene tree unless a zIndex value is specified). 2D objects within the same container absolutely cannot be ordered using z-distance to the camera because it results in random ordering depending on camera rotation. However, each 2D container as a whole, does indeed need to be sorted by z-distance with other 2D containers and other 3D objects in the scene. The depth buffer is not an issue because I use polygon offset values, but transparent objects still need to be sorted correctly.

However, what I'm describing is a general problem that can't be solved with a sort algorithm. Here is a very simple JSFiddle: https://jsfiddle.net/6ecrzgyL/1/

@Mugen87
Copy link
Collaborator

Mugen87 commented Oct 19, 2022

Thanks for the fiddle. I understand your use case now but I'm still the opinion that this scenario is too specific which means it's not appropriate to change the renderer in the suggested way. I suggest you maintain the render list on app level and then define a renderOrder for each 3D object. If this does not work for a specific reason, I suggest you use a custom version of the renderer.

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.

2 participants