Skip to content

Conversation

@mrdoob
Copy link
Owner

@mrdoob mrdoob commented Mar 8, 2021

Related issue: #21430

Description

After working on #21430 I now think that adding a colorCenterLine to GridHelper was a mistake. The GPU render all lines at the same time and that produces z fighting glitches in the intersections.

We can achieve something similar (and good looking) like this:

const grid1 = new THREE.GridHelper( 30, 30, 0x888888 );
scene.add( grid1 );

// center line
const grid2 = new THREE.GridHelper( 30, 2, 0x222222 );
grid2.material.depthFunc = THREE.AlwaysDepth;
scene.add( grid2 );

This is somewhat a breaking change.

@mrdoob mrdoob added this to the r127 milestone Mar 8, 2021
@WestLangley
Copy link
Collaborator

grid2.material.depthFunc = THREE.AlwaysDepth;

That overwrites existing objects in the scene.

@Mugen87
Copy link
Collaborator

Mugen87 commented Mar 8, 2021

This problem did probably not become apparent since the editor's grid is rendered in a special way.

@Mugen87
Copy link
Collaborator

Mugen87 commented Mar 8, 2021

I guess it would work if the renderOrder of GridHelper is set to -Infinity. However, this is not a very reliable solution...

@mrdoob
Copy link
Owner Author

mrdoob commented Mar 8, 2021

grid2.material.depthFunc = THREE.AlwaysDepth;

That overwrites existing objects in the scene.

Yes, the grid needs to be rendered first.

Unfortunately GL_POLYGON_OFFSET_LINE is not available in WebGL.

As some point I had solved it like this:

grid2.material.onBeforeCompile = function ( shader ) {

	// Emulate GL_POLYGON_OFFSET_LINE
	shader.vertexShader = shader.vertexShader.replace( '<worldpos_vertex>', '<worldpos_vertex>\ngl_Position.z -= 0.0001;' );

};

But I opted for THREE.AlwaysDepth as it seemed prettier.

But yes, in this PR I was going to try updating the examples and notice the side-effects of the approach.

What I know is that the current approach is a minefield. In VR those line intersections blink and it's very distracting...

@mrdoob mrdoob modified the milestones: r127, r128 Mar 30, 2021
@mrdoob mrdoob modified the milestones: r128, r129 Apr 23, 2021
@mrdoob mrdoob modified the milestones: r129, r130 May 27, 2021
@mrdoob mrdoob modified the milestones: r130, r131 Jun 30, 2021
@mrdoob mrdoob modified the milestones: r131, r132 Jul 28, 2021
@mrdoob mrdoob modified the milestones: r132, r133 Aug 26, 2021
@mrdoob mrdoob modified the milestones: r133, r134 Sep 30, 2021
@mrdoob mrdoob modified the milestones: r134, r135 Oct 28, 2021
@mrdoob mrdoob modified the milestones: r135, r136 Nov 26, 2021
@mrdoob mrdoob modified the milestones: r136, r137 Dec 24, 2021
@mrdoob mrdoob modified the milestones: r137, r138 Jan 26, 2022
@mrdoob mrdoob removed this from the r138 milestone Feb 23, 2022
@mrdoob mrdoob added this to the r145 milestone Aug 31, 2022
@mrdoob mrdoob modified the milestones: r145, r146 Sep 29, 2022
@mrdoob mrdoob modified the milestones: r146, r147 Oct 27, 2022
@mrdoob mrdoob modified the milestones: r147, r148 Nov 30, 2022
@mrdoob mrdoob modified the milestones: r148, r149 Dec 22, 2022
@mrdoob mrdoob modified the milestones: r149, r150 Jan 26, 2023
@mrdoob mrdoob modified the milestones: r150, r151 Feb 23, 2023
@mrdoob mrdoob modified the milestones: r151, r152 Mar 30, 2023
@mrdoob mrdoob modified the milestones: r152, r153 Apr 27, 2023
@Mugen87 Mugen87 modified the milestones: r153, r154 Jun 1, 2023
@mrdoob mrdoob modified the milestones: r154, r155 Jun 29, 2023
@mrdoob mrdoob modified the milestones: r155, r156 Jul 27, 2023
@mrdoob mrdoob modified the milestones: r156, r157 Aug 31, 2023
@mrdoob mrdoob modified the milestones: r157, r158 Sep 28, 2023
@Mugen87
Copy link
Collaborator

Mugen87 commented Oct 18, 2023

But yes, in this PR I was going to try updating the examples and notice the side-effects of the approach.

I think it's better to close the PR because of the mentioned issues. Seems we need a different approach to mitigate the blinking when grid lines intersect. At least there is a solution for the editor.

@Mugen87 Mugen87 closed this Oct 18, 2023
@Mugen87 Mugen87 removed this from the r158 milestone Oct 18, 2023
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