Add storm support for camera exposure controls #3464
Conversation
|
Filed as internal issue #USD-10512 (This is an automated message. See here for more information.) |
|
/AzurePipelines run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
pmolodo
force-pushed
the
usdGeomCamera-exposure-storm
branch
from
adb7ebb to
1a69581
Compare
pxr/imaging/hdx/colorChannelTask.h
Outdated
| @@ -54,9 +54,6 @@ class HdxColorChannelTask : public HdxTask | |||
| // Returns true if the values were updated. False if unchanged. | |||
| bool _UpdateParameterBuffer(float screenSizeX, float screenSizeY); | |||
|
|
|||
| /// Apply the color channel filtering. | |||
| void _ApplyColorChannel(); | |||
There was a problem hiding this comment.
Can you revert the changes to colorChannelTask? They seem unrelated.
There was a problem hiding this comment.
Heh, you caught me... I tried to sneak in a few commits with some minor code cleanup. Moved them to a separate PR: #3521
pxr/usd/usdGeom/schema.usda
Outdated
| @@ -2297,6 +2297,42 @@ class Camera "Camera" ( | |||
| However, it follows that if even one property is authored in the correct | |||
| scene units, then they all must be. | |||
|
|
|||
| \\section UsdGeom_CameraExposure Camera Exposure Model | |||
There was a problem hiding this comment.
It would be great to rebase this CL on top of dev; now that the exposure schema has landed, it would make the PR much smaller.
pmolodo
force-pushed
the
usdGeomCamera-exposure-storm
branch
from
684f18d to
a8793af
Compare
pmolodo
force-pushed
the
usdGeomCamera-exposure-storm
branch
from
a8793af to
77a2a03
Compare
|
/AzurePipelines run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
| } | ||
| } | ||
|
|
||
| // Currently, we're querying GetLinearExposureScale() every frame - not |
There was a problem hiding this comment.
When "linearExposureScale" is updated, the target camera will get an "HdCamera::DirtyParams" update. You could add a version counter to the HdCamera, and check if the task has the latest version, although that only seems worthwhile if _compositor->SetShaderConstants is expensive.
| @@ -44,6 +44,7 @@ pxr_library(hdx | |||
| colorCorrectionTask | |||
| drawTargetTask | |||
| effectsShader | |||
| exposureScaleTask | |||
There was a problem hiding this comment.
The makefile/package/taskController updates all seem to be "exposureScale", but "linearExposureScale" is included as well. Note that camera ended up with "GetLinearExposureScale" but not "ExposureScale", so possibly we need to nix all of the plain old "exposureScale" stuff.
I couldn't find the pow(2, exposureScale) anywhere in exposureScaleTask, but if we're going with linearExposureScale that doesn't matter.
Description of Change(s)
This is a follow-up / add-on to the PR which adds camera exposure controls:
It adds
HdxExposureScaleTaskto allow visualization of the camera exposure controls in Storm.See here to just see the additional changes, vs #3085:
Checklist
[X] I have created this PR based on the dev branch
[X] I have followed the coding conventions
[ ] I have added unit tests that exercise this functionality (Reference:
testing guidelines)
[X] I have verified that all unit tests pass with the proposed changes
[X] I have submitted a signed Contributor License Agreement (Reference:
Contributor License Agreement instructions)