Skip to content

Conversation

@Mugen87
Copy link
Collaborator

@Mugen87 Mugen87 commented Jul 8, 2018

Fixes #14415

The PR introduces a new class SortingGroup which can be used to group objects within the render list. So it's somewhat similar to Unity's SortingGroup API.

@Mugen87 Mugen87 changed the title WebGLRenderer: Added .sortGroups WebGLRenderer: Added SortingGroup Jul 8, 2018
@WestLangley
Copy link
Collaborator

So, within a sorting group, object.renderOrder only applies relative to the objects within the sorting group. Is that your intention?

If so, I expect SortingGroup will have to have its renderOrder property honored so the sorting groups can be sorted.

@WestLangley
Copy link
Collaborator

And a sorting group should not contain both opaque and transparent renderable objects. I think that is OK, though.

@WestLangley
Copy link
Collaborator

@mrdoob I wonder if SVGLoader could make use of this feature to ensure the shapes are rendered in the proper order -- especially if there are multiple SVG objects, or other objects in the scene.

@Mugen87
Copy link
Collaborator Author

Mugen87 commented Jul 8, 2018

So, within a sorting group, object.renderOrder only applies relative to the objects within the sorting group. Is that your intention?

Yes. And if SortingGroup groups are not used at all, renderOrder has still the highest priority.

If so, I expect SortingGroup will have to have its renderOrder property honored so the sorting groups can be sorted.

Um, I'm not sure how to solve this. Objects of type SortingGroup are not added to the render list so it's not possible to sort them right now. I guess we need some additional logic for this use case...

@WestLangley
Copy link
Collaborator

Yes. You have to be able to sort the sorting groups themselves.

@Mugen87
Copy link
Collaborator Author

Mugen87 commented Jul 9, 2018

Sorting groups via renderOrder should be possible now. You can see the effect if you change the properties in this fiddle: https://jsfiddle.net/f2Lommf5/8890/

BTW: It's now necessary to set new property of WebGLRenderer (sortingGroupsEnabled) to true so SortingGroups can be used. The sorting of groups needs an additional traversal through the scene graph. The flag was introduced in order to avoid this overhead if you don't need SortingGroups in your app.

@navigator117
Copy link

Mesh without SortingGroup parent should be able to compare renderOrder with SortingGroup, as unity does this.

@Mugen87
Copy link
Collaborator Author

Mugen87 commented Jul 10, 2018

This should work now, too. The code is also slightly refactored so changes to projectObject() are not necessary anymore.

@Mugen87
Copy link
Collaborator Author

Mugen87 commented Jul 10, 2018

Nevertheless the holistic implementation of this approach feels hacky. Besides, the current way WebGLSortingGroups and WebGLRenderList are working together is no good. I'm closing the PR for now and try to think of a different approach.

@mrdoob
Copy link
Owner

mrdoob commented Dec 28, 2018

What about this implementation:

diff --git a/src/renderers/WebGLRenderer.js b/src/renderers/WebGLRenderer.js
index 4f61704e7..f684902b8 100644
--- a/src/renderers/WebGLRenderer.js
+++ b/src/renderers/WebGLRenderer.js
@@ -1073,7 +1073,7 @@ function WebGLRenderer( parameters ) {
 		currentRenderList = renderLists.get( scene, camera );
 		currentRenderList.init();
 
-		projectObject( scene, camera, _this.sortObjects );
+		projectObject( scene, camera, 0, _this.sortObjects );
 
 		if ( _this.sortObjects === true ) {
 
@@ -1164,7 +1164,7 @@ function WebGLRenderer( parameters ) {
 
 	};
 
-	function projectObject( object, camera, sortObjects ) {
+	function projectObject( object, camera, groupOrder, sortObjects ) {
 
 		if ( object.visible === false ) return;
 
@@ -1172,7 +1172,11 @@ function WebGLRenderer( parameters ) {
 
 		if ( visible ) {
 
-			if ( object.isLight ) {
+			if ( object.isSortingGroup ) {
+
+				groupOrder = object.renderOrder;
+
+			} else if ( object.isLight ) {
 
 				currentRenderState.pushLight( object );
 
@@ -1196,7 +1200,7 @@ function WebGLRenderer( parameters ) {
 					var geometry = objects.update( object );
 					var material = object.material;
 
-					currentRenderList.push( object, geometry, material, _vector3.z, null );
+					currentRenderList.push( object, geometry, material, groupOrder, _vector3.z, null );
 
 				}
 
@@ -1209,7 +1213,7 @@ function WebGLRenderer( parameters ) {
 
 				}
 
-				currentRenderList.push( object, null, object.material, _vector3.z, null );
+				currentRenderList.push( object, null, object.material, groupOrder, _vector3.z, null );
 
 			} else if ( object.isMesh || object.isLine || object.isPoints ) {
 
@@ -1242,7 +1246,7 @@ function WebGLRenderer( parameters ) {
 
 							if ( groupMaterial && groupMaterial.visible ) {
 
-								currentRenderList.push( object, geometry, groupMaterial, _vector3.z, group );
+								currentRenderList.push( object, geometry, groupMaterial, groupOrder, _vector3.z, group );
 
 							}
 
@@ -1250,7 +1254,7 @@ function WebGLRenderer( parameters ) {
 
 					} else if ( material.visible ) {
 
-						currentRenderList.push( object, geometry, material, _vector3.z, null );
+						currentRenderList.push( object, geometry, material, groupOrder, _vector3.z, null );
 
 					}
 
@@ -1264,7 +1268,7 @@ function WebGLRenderer( parameters ) {
 
 		for ( var i = 0, l = children.length; i < l; i ++ ) {
 
-			projectObject( children[ i ], camera, sortObjects );
+			projectObject( children[ i ], camera, groupOrder, sortObjects );
 
 		}
 
diff --git a/src/renderers/webgl/WebGLRenderLists.js b/src/renderers/webgl/WebGLRenderLists.js
index 234de8d5f..0150d2518 100644
--- a/src/renderers/webgl/WebGLRenderLists.js
+++ b/src/renderers/webgl/WebGLRenderLists.js
@@ -4,7 +4,11 @@
 
 function painterSortStable( a, b ) {
 
-	if ( a.renderOrder !== b.renderOrder ) {
+	if ( a.groupOrder !== b.groupOrder ) {
+
+		return a.groupOrder - b.groupOrder;
+
+	} else if ( a.renderOrder !== b.renderOrder ) {
 
 		return a.renderOrder - b.renderOrder;
 
@@ -30,7 +34,11 @@ function painterSortStable( a, b ) {
 
 function reversePainterSortStable( a, b ) {
 
-	if ( a.renderOrder !== b.renderOrder ) {
+	if ( a.groupOrder !== b.groupOrder ) {
+
+		return a.groupOrder - b.groupOrder;
+
+	} else if ( a.renderOrder !== b.renderOrder ) {
 
 		return a.renderOrder - b.renderOrder;
 
@@ -64,7 +72,7 @@ function WebGLRenderList() {
 
 	}
 
-	function getNextRenderItem( object, geometry, material, z, group ) {
+	function getNextRenderItem( object, geometry, material, groupOrder, z, group ) {
 
 		var renderItem = renderItems[ renderItemsIndex ];
 
@@ -76,6 +84,7 @@ function WebGLRenderList() {
 				geometry: geometry,
 				material: material,
 				program: material.program,
+				groupOrder: groupOrder,
 				renderOrder: object.renderOrder,
 				z: z,
 				group: group
@@ -90,6 +99,7 @@ function WebGLRenderList() {
 			renderItem.geometry = geometry;
 			renderItem.material = material;
 			renderItem.program = material.program;
+			renderItem.groupOrder = object.groupOrder;
 			renderItem.renderOrder = object.renderOrder;
 			renderItem.z = z;
 			renderItem.group = group;

@mrdoob
Copy link
Owner

mrdoob commented Dec 28, 2018

Actually, we could just do this with the current Group directly.
I can't imagine anyone's current code breaking as setting group.renderOrder had no use previously.

diff --git a/src/renderers/WebGLRenderer.js b/src/renderers/WebGLRenderer.js
index 4f61704e7..f684902b8 100644
--- a/src/renderers/WebGLRenderer.js
+++ b/src/renderers/WebGLRenderer.js
@@ -1073,7 +1073,7 @@ function WebGLRenderer( parameters ) {
 		currentRenderList = renderLists.get( scene, camera );
 		currentRenderList.init();
 
-		projectObject( scene, camera, _this.sortObjects );
+		projectObject( scene, camera, 0, _this.sortObjects );
 
 		if ( _this.sortObjects === true ) {
 
@@ -1164,7 +1164,7 @@ function WebGLRenderer( parameters ) {
 
 	};
 
-	function projectObject( object, camera, sortObjects ) {
+	function projectObject( object, camera, groupOrder, sortObjects ) {
 
 		if ( object.visible === false ) return;
 
@@ -1172,7 +1172,11 @@ function WebGLRenderer( parameters ) {
 
 		if ( visible ) {
 
-			if ( object.isLight ) {
+			if ( object.isGroup ) {
+
+				groupOrder = object.renderOrder;
+
+			} else if ( object.isLight ) {
 
 				currentRenderState.pushLight( object );
 
@@ -1196,7 +1200,7 @@ function WebGLRenderer( parameters ) {
 					var geometry = objects.update( object );
 					var material = object.material;
 
-					currentRenderList.push( object, geometry, material, _vector3.z, null );
+					currentRenderList.push( object, geometry, material, groupOrder, _vector3.z, null );
 
 				}
 
@@ -1209,7 +1213,7 @@ function WebGLRenderer( parameters ) {
 
 				}
 
-				currentRenderList.push( object, null, object.material, _vector3.z, null );
+				currentRenderList.push( object, null, object.material, groupOrder, _vector3.z, null );
 
 			} else if ( object.isMesh || object.isLine || object.isPoints ) {
 
@@ -1242,7 +1246,7 @@ function WebGLRenderer( parameters ) {
 
 							if ( groupMaterial && groupMaterial.visible ) {
 
-								currentRenderList.push( object, geometry, groupMaterial, _vector3.z, group );
+								currentRenderList.push( object, geometry, groupMaterial, groupOrder, _vector3.z, group );
 
 							}
 
@@ -1250,7 +1254,7 @@ function WebGLRenderer( parameters ) {
 
 					} else if ( material.visible ) {
 
-						currentRenderList.push( object, geometry, material, _vector3.z, null );
+						currentRenderList.push( object, geometry, material, groupOrder, _vector3.z, null );
 
 					}
 
@@ -1264,7 +1268,7 @@ function WebGLRenderer( parameters ) {
 
 		for ( var i = 0, l = children.length; i < l; i ++ ) {
 
-			projectObject( children[ i ], camera, sortObjects );
+			projectObject( children[ i ], camera, groupOrder, sortObjects );
 
 		}
 
diff --git a/src/renderers/webgl/WebGLRenderLists.js b/src/renderers/webgl/WebGLRenderLists.js
index 234de8d5f..0150d2518 100644
--- a/src/renderers/webgl/WebGLRenderLists.js
+++ b/src/renderers/webgl/WebGLRenderLists.js
@@ -4,7 +4,11 @@
 
 function painterSortStable( a, b ) {
 
-	if ( a.renderOrder !== b.renderOrder ) {
+	if ( a.groupOrder !== b.groupOrder ) {
+
+		return a.groupOrder - b.groupOrder;
+
+	} else if ( a.renderOrder !== b.renderOrder ) {
 
 		return a.renderOrder - b.renderOrder;
 
@@ -30,7 +34,11 @@ function painterSortStable( a, b ) {
 
 function reversePainterSortStable( a, b ) {
 
-	if ( a.renderOrder !== b.renderOrder ) {
+	if ( a.groupOrder !== b.groupOrder ) {
+
+		return a.groupOrder - b.groupOrder;
+
+	} else if ( a.renderOrder !== b.renderOrder ) {
 
 		return a.renderOrder - b.renderOrder;
 
@@ -64,7 +72,7 @@ function WebGLRenderList() {
 
 	}
 
-	function getNextRenderItem( object, geometry, material, z, group ) {
+	function getNextRenderItem( object, geometry, material, groupOrder, z, group ) {
 
 		var renderItem = renderItems[ renderItemsIndex ];
 
@@ -76,6 +84,7 @@ function WebGLRenderList() {
 				geometry: geometry,
 				material: material,
 				program: material.program,
+				groupOrder: groupOrder,
 				renderOrder: object.renderOrder,
 				z: z,
 				group: group
@@ -90,6 +99,7 @@ function WebGLRenderList() {
 			renderItem.geometry = geometry;
 			renderItem.material = material;
 			renderItem.program = material.program;
+			renderItem.groupOrder = object.groupOrder;
 			renderItem.renderOrder = object.renderOrder;
 			renderItem.z = z;
 			renderItem.group = group;

@Mugen87
Copy link
Collaborator Author

Mugen87 commented Dec 28, 2018

I've added a WIP PR to demonstrate your suggested change. I've also added a small demo that shows that the PR solve the original issue mentioned in #14415

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.

4 participants