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

View Space Lighting Tool #2691

Merged
merged 77 commits into from Jan 14, 2021
Merged

View Space Lighting Tool #2691

merged 77 commits into from Jan 14, 2021

Conversation

skhiat
Copy link
Contributor

@skhiat skhiat commented Nov 19, 2020

Provide a ViewSpace Lighting Tool (Game Camera Only)

Why the PR's needed?
Give a tool that help the lighter to light the scene directly on the Scene View, which is very convenient to setup cutscene, cinematics, ... For M&E, Chaplin, ....

[Post-UX Pass]
image
image

What's tested?

  1. Setup a scene
  2. Add a Spot Light
  3. Add LightAnchor Script (Add Component > Rendering > LightAnchor)
    Move the light orientation & distance (in the component), notice the lighting in the GameView

What needs more testing?

  • If the behavior is consistent with the rest of the engine
  • If that behave as expected for other users
  • Any relevant comments
  • [NEW]Deeper test of redo-undo is properly implemented and consistent with the rest of the engine.
  • [NEW]If the light placement works with timeline
  • [NEW]More info about what need to be tested:
    https://docs.google.com/document/d/1psNwpqBTKp3SISTEA7nKu0UoUPiIy7jCN1OEddNEL08/edit
  • [NEW]Make sure that will not broke for built game (resources are on another folder - not regular)

LPv6SmquQZ
Which allow to place light relative to camera space (Game Camera Only).
The last knob will control the rotation of the cookie
UKKcmdR33F

Adding a tool for LightAnchor:
ZGn8vnUfPa
Which change the gizmo to be on the target to place easily the target:
Do4Emua5ZO

@github-actions github-actions bot added the SRP label Nov 19, 2020
@skhiat skhiat requested a review from JordanL8 November 20, 2020 11:15
@skhiat skhiat added the Docs label Nov 20, 2020
@skhiat skhiat marked this pull request as ready for review November 20, 2020 11:17
@skhiat skhiat requested a review from a team as a code owner November 20, 2020 12:46
@skhiat skhiat requested review from a team as code owners January 6, 2021 10:28
@theo-pnv theo-pnv removed the request for review from a team January 7, 2021 15:22
Copy link
Collaborator

@RSlysz RSlysz left a comment

Choose a reason for hiding this comment

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

Looks better.

Though, make sure you are compliant with L10n
https://confluence.unity3d.com/display/LOC/Editor+Localization+Coding+Guidelines

…HDRP/ViewSpaceLightingTool

# Conflicts:
#	com.unity.render-pipelines.core/Editor/Lighting/Icons/DebugProbe.fbx
#	com.unity.render-pipelines.core/Editor/Lighting/Icons/DebugProbe.fbx.meta
#	com.unity.render-pipelines.core/Editor/Lighting/Icons/InstancedProbeShader.shader
#	com.unity.render-pipelines.core/Editor/Lighting/Icons/InstancedProbeShader.shader.meta
@RSlysz RSlysz self-requested a review January 12, 2021 17:40
@sebastienlagarde sebastienlagarde removed the request for review from a team January 14, 2021 12:59
Copy link
Contributor

@remi-chapelain remi-chapelain left a comment

Choose a reason for hiding this comment

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

Confluence test page has been updated, last (must fix) bug has been fixed, Approving ! ✔️

Still have some concerns about users that will want to use the tool with timeline or at runtime. I have gathered some feedback from artists that could see that as an improvement in the future. We'll see depending on the users usage and feedback to see if that makes sense !

@sebastienlagarde sebastienlagarde merged commit 0b558dd into master Jan 14, 2021
@sebastienlagarde sebastienlagarde deleted the HDRP/ViewSpaceLightingTool branch January 14, 2021 14:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
7 participants