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
2 times, most recently
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.
There was a problem hiding this comment.
I updated the note to essentially just say what you just did:
// Currently, we're querying GetLinearExposureScale() every frame -
// if we wanted to be more selective:
//
// - target camera should get a HdCamera::DirtyParams update whenever
// any input attrs to linearExposureScale change
// - we could store a version counter on the HdCamera
// - ...and a `_lastCamVersion` on this task, and then only update
// our _linearExposureScale if it didn't == cam's version
//
// However, this shouldn't be a problem unless
// _compositor->SetShaderConstants is expensive, so not going to
// optimize unless we have evidence of that.
I can also just remove the comment entirely, if you think this is unlikely to ever be an issue.
pxr/imaging/hdx/CMakeLists.txt
Outdated
| @@ -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.
There was a problem hiding this comment.
Apologies - this was apparently in an incomplete state of transition to the new "everything is named linearExposure" changes.
So, anyway, completed the renames of all the *ExposureScale to *LinearExposureScale:
pmolodo
force-pushed
the
usdGeomCamera-exposure-storm
branch
2 times, most recently
from
cf0b28a to
671584d
Compare
pmolodo
force-pushed
the
usdGeomCamera-exposure-storm
branch
from
671584d to
cb9b2a6
Compare
|
Fixed an issue due to changes from this commit: |
|
Hi @tcauchois - sorry for fixing this up sooner! Comfirmed this is building and executing correctly, though... |
|
Also - this repo contains some quick tests I slapped together to verify functionality: https://github.com/pmolodo/usd_cam_exposure_test/tree/main Would you want me to clean this up / proceduralize it, and turn them into proper unittests? |
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)