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

Add preview Sun and Environment #46315

Merged
merged 1 commit into from
Feb 22, 2021

Conversation

reduz
Copy link
Member

@reduz reduz commented Feb 22, 2021

  • Adds both a preview sun and preview environment to the 3D editor.
  • They are valid as long as a DirectionalLight3D and WorldEnvironment are not in the scene.
  • If any is added to the scene, the respective preview is disabled.
  • Changed WorldEnvironment to better handle multiple node versions.
  • Added a function in SceneTree to get the first node in a group.
  • Fixed button minimum size to also consider font height if no text is there, this broke with the TextSever PR.

image

I am aware that users might ask adding more options to environment such as panoramas or physical sky, but this aims to be a MVP. If what is there does not suffice and more features are requested by many users, we will add them.

@Zireael07
Copy link
Contributor

They are valid as long as a DirectionalLight3D and WorldEnvironment are not in the scene.
If any is added to the scene, the respective preview is disabled.

Both or either?
If either, can we please have a preview come on if I click the 'eye' (switch to not visible) on DirectionalLight3D?

Added a function in SceneTree to get the first node in a group.

Heck yeah, that's an extremely common use case for me xDDDD

@reduz
Copy link
Member Author

reduz commented Feb 22, 2021

Both or either?
If either, can we please have a preview come on if I click the 'eye' (switch to not visible) on DirectionalLight3D?

Its individual (sun and environment). About hiding I am not sure, its not very easy to do this, lets see how much demand there is.

Heck yeah, that's an extremely common use case for me xDDDD

Yes this should have been added long ago.

@fire
Copy link
Member

fire commented Feb 22, 2021

I drew a circle spin mockup for the direction of the preview sun. The idea is the orb rotates around the sphere signals direction.

It makes it easier for touch screens and has a visual representation of the sun direction.

What do you think?

circle-spin

@aaronfranke aaronfranke self-requested a review February 22, 2021 15:21
editor/plugins/node_3d_editor_plugin.cpp Outdated Show resolved Hide resolved
editor/plugins/node_3d_editor_plugin.cpp Outdated Show resolved Hide resolved
Copy link
Member

@clayjohn clayjohn left a comment

Choose a reason for hiding this comment

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

Looks good! Just need to squash the commits together now.

@reduz
Copy link
Member Author

reduz commented Feb 22, 2021

@fire I think it should work with touchscreen already, but its not a bad idea, we could add this eventually. I wont modify this PR further since its meant to be an MVP, you guys can work over it later if more features are needed.

* Adds both a preview sun and preview environment to the 3D editor.
* They are valid as long as a DirectionalLight3D and WorldEnvironment are not in the scene.
* If any is added to the scene, the respective preview is disabled.
* Changed WorldEnvironment to better handle multiple node versions.
* Added a function in SceneTree to get the first node in a group.
* Fixed button minimum size to also consider font height if no text is there, this broke with the TextSever PR.
@reduz reduz force-pushed the add-preview-sun-and-env branch from bac4734 to d6a9cff Compare February 22, 2021 19:56
@aaronfranke
Copy link
Member

The menu button used here looks grayed out, I think it would be better if we used the white version (next to "Perspective").

Screenshot from 2021-02-22 15-01-02

For the preview sun, is it intentional that there is no way to input specific angles, and that the only way to change its orientation is to drag it around? I think it would be nice to have a way to input azimuth and angular altitude. Also, right now the tool allows the sun to have a non-zero Z rotation, but rotation around Z does nothing for DirectionalLight, it should probably be zeroed automatically.

None of the above is a blocker for merging, it can be improved later.

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

Thanks!

@reduz
Copy link
Member Author

reduz commented Feb 22, 2021

@aaronfranke I tried the white version first, but in contrast with the other icons, it was standing out much, so instead I used the grayed out version. May go for something intermediate maybe. Entering angles is doable, but lets see if users ask for it first.

@aaronfranke
Copy link
Member

Hmm, this was working when I tested it earlier, but now the sky flashes any time I try to rotate the sun. I tried deleting .godot/, ~/.config/godot/, and ~/.local/share/godot/. It also occurs on a brand new project. Ubuntu 20.04 64-bit, Nvidia GTX 1070.

bug.mp4

@reduz
Copy link
Member Author

reduz commented Feb 22, 2021

@aaronfranke it was happening before this PR too, may have to do with something else. Will have to ask @clayjohn to take a look.

@jasperbrooks79
Copy link

jasperbrooks79 commented Feb 23, 2021

This new feature is amazing, but there is the problem, of the sky making things look ' blue ' . . I have tried making some stylized characters, and it simply doesn't work, the current default environment makes things look blue, and there's no explanation, for it . .

The feature is amazing, btw, really love it, so cool . .

There are two options, make the default sky color white or, disable ' sky contribution ', or set = 0, as default . .

This might be more me, but it's difficult for me to see, when a feature, in the icons, is turned on, or off . . Maybe one could add a ' wink ' or, something to the icons, or underline them, when it's set on, or so . .

Skærmbillede (1079)

or, make so, when one hovers over icon, it says in text - box, Status: On/Off . .
Skærmbillede (1080)

Some thoughts, why can one place the sun, beneath the horizon . . Why not just make it hover, over top of sphere, since at night, you can't see the sun . .

Like . . .
Skærmbillede (1081)

Also, maybe add some sort of 2D or, 3D icon that hovers, like AaronFranke said . . but, it works really well, just not sure why, I'd ever want to place a sun, beneath the horizon, practically . .

The problem is this, 1. sky contribution is a 'real' thing, so it makes sense, it's there . . But, for people importing stylized characters, the default environment, blue sky contribution simply doesn't work . . IF the default sky was white, or sky contribution was turned off, then stylized characters would import correctly, well . . and, realistic, PBR characters would still look nice . .

I think the important thing is, when people import either stylized or, PBR 3D models, that both look ' right ', immediately . . So, just set sky contribution to 0, by default, and let people that want crazy PBR worlds, lighting find it, as they need <3 include it, in features, specific for PBR work-flow, RTX, realistic lights . . technically, by disabling it, games should also run faster, so the default environment is a bit faster, or so . .

Last, are the buttons a bit buggy, not sure <3
2021-02-23 0908
Can't see, if there off or, on, and the highlight around them, are like, well confusing, atm . .

Over-all, this is an amazing feature, it's just not clear to me, why the buttons are there, specifically . . I think, if there was an intro splash-screen, that had a bit tutorial in it, that experienced users could not use, switch off and, new users quickly getting a tutorial, on where most important buttons are, so on, that's be cool, Thanks . .

@jasperbrooks79
Copy link

ps. having tried the feature, I hope it stays, I like how it helps one find, what post - proc. one needs <3

The feature is amazing, but when do we get default sky contribution = 0 <3 . . Thx . . . .

@Zireael07
Copy link
Contributor

@jasperbrooks79: Your comment above about stylized characters and blue tint sounds like godotengine/godot-proposals#348

@jasperbrooks79
Copy link

I know, I've been thinking I'm a noob user, of the engine . .

And, there are many much more smart, cool people here, I will post less, it's in good hands <3 . .

@akien-mga
Copy link
Member

akien-mga commented Feb 23, 2021

@jasperbrooks79 Your feedback on this feature was good and appreciated, don't worry! The proposal linked by @Zireael07 is indeed related, and there might be new opportunities with the feature included in this PR to address it (e.g. adding a neutral sky preset a bit like "LookDev" mode in Blender). Let's play test the feature a bit and see how it articulates with existing proposals or potential new ones that could be written.

@jasperbrooks79
Copy link

jasperbrooks79 commented Feb 23, 2021

I can't wait, this feature was really interesting and, cool . .

So, 9.6 - 9.7 / 10, for feature . .

but, of course we want more . .

Thx . . .

ps. Is it really fair, we have to wait this long for amazing, mind - blowing soft-ware . . I, despair . . </3 . . when's, tomorrow <3 . . . .

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.

8 participants