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

3D editor grid improvements #45594

Merged
merged 1 commit into from
Feb 1, 2021
Merged

Conversation

JFonS
Copy link
Contributor

@JFonS JFonS commented Jan 30, 2021

This commit adds a view-dependant fade to the 3D viewport grid. It fades out at steep view angles to hide the solid color regions that appear far from the camera.

I also improved the dynamic grid when the camera is in orthogonal mode by properly handling zoom, and moving the grid center to the intersection point between the grid plane and the camera forward ray.

I increased the default grid size from 200 to 400, so it's harder to reach the edge of the grid, and you don't even notice the grid is actually dynamic. I don't think it will have a significant performance impact, but I might be wrong.

Also, while working on this PR I noticed the dynamic grid system only works for one viewport. All 3D viewports render the same grid, which only cares about the first viewport. That means the rest of viewports may have a grid completely out of view, or with a wrong zoom level.

Fixing the multiple viewports issue is a bit involved, so I think it should be done in a separate PR.

Small comparison

Before:
no-fade
After:
fade

@JFonS JFonS added enhancement topic:editor cherrypick:3.x Considered for cherry-picking into a future 3.x release topic:3d labels Jan 30, 2021
@JFonS JFonS added this to the 4.0 milestone Jan 30, 2021
@akien-mga akien-mga requested a review from a team January 30, 2021 21:27
@@ -5217,6 +5217,29 @@ void Node3DEditor::_init_indicators() {
indicator_mat->set_flag(StandardMaterial3D::FLAG_SRGB_VERTEX_COLOR, true);
indicator_mat->set_transparency(StandardMaterial3D::Transparency::TRANSPARENCY_ALPHA_DEPTH_PRE_PASS);

Ref<Shader> grid_shader = memnew(Shader);
grid_shader->set_code("\n"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sending the first "\n" to the next line should play nicer with clang-format (indent everything with 2 tabs, instead of indent everything with 2 tabs after the open parenthesis.

Copy link
Member

@akien-mga akien-mga left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Style nitpick, otherwise looks great to me.

@aaronfranke aaronfranke self-requested a review January 30, 2021 21:50
Copy link
Member

@aaronfranke aaronfranke left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The fade effect needs to change its distance based on the grid size. Right now this happens if I set a grid size of 50:

Screenshot from 2021-01-30 17-20-48

And also, as this PR currently is, setting a larger grid size won't actually make more grid lines visible.

editor/plugins/node_3d_editor_plugin.cpp Outdated Show resolved Hide resolved
editor/plugins/node_3d_editor_plugin.cpp Outdated Show resolved Hide resolved
@reduz reduz closed this Jan 31, 2021
@reduz
Copy link
Member

reduz commented Jan 31, 2021

oops sorry, I was trying to comment and I closed it. I meant that you might want to enable depth write and set the priority so it draws before other objects with transparency, this way they can work together.

@reduz reduz reopened this Jan 31, 2021
@JFonS
Copy link
Contributor Author

JFonS commented Jan 31, 2021

The fade effect needs to change its distance based on the grid size. Right now this happens if I set a grid size of 50:

I don't think this is a must, but it would be nice to have. Reducing the high line density and hiding the edges of the grid are two separate issues to me, but I can try to add it while we are at it.

And also, as this PR currently is, setting a larger grid size won't actually make more grid lines visible.

I can see it making a difference when the camera is far from the grid plane. Probably not noticeable in most of the projects I guess, so I will bring it back to 200.

This commit adds a view-dependant fade to the 3D viewport grid. It fades out
at steep view angles to hide the solid regions that appear far from the camera.
I also included a fade to hide the grid borders.

I added some improvements to the dynamic grid when the camera is in orthogonal mode.
It properly handles zoom now, and the grid center is now set to the intersection point
between the grid plane and the camera forward ray, keeping the grid
always visible.
@JFonS
Copy link
Contributor Author

JFonS commented Feb 1, 2021

I updated the PR to add a distance based fade, the grid edges should not be visible now.

I fixed the style issues too.

Copy link
Member

@aaronfranke aaronfranke left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did some testing, this is great!

This should be merged soon and cherry-picked for 3.2.4. Since we are introducing the infinite grid in 3.2.4, then we may as well make it as nice as possible from the start.

@akien-mga akien-mga merged commit b24c24f into godotengine:master Feb 1, 2021
@akien-mga
Copy link
Member

Thanks!

@akien-mga
Copy link
Member

Cherry-picked for 3.2.4.

@akien-mga akien-mga removed the cherrypick:3.x Considered for cherry-picking into a future 3.x release label Feb 2, 2021
@JFonS JFonS deleted the improve_3d_grid branch May 4, 2021 07:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants