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 storm support for camera exposure controls #3464

Open
wants to merge 34 commits into
base: dev
Choose a base branch
from

Conversation

pmolodo
Copy link
Contributor

@pmolodo pmolodo commented Dec 13, 2024

Description of Change(s)

This is a follow-up / add-on to the PR which adds camera exposure controls:

It adds HdxExposureScaleTask to 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)

anderslanglands and others added 27 commits May 14, 2024 16:00
Adds exposure:time, exposure:iso, exposure:fNumber, exposure:responsivity attributes and repurposes exposure attribute to be interpreted as exposure compensation. Adds UsdGeomCamera::GetExposureScale() method which allows users to calculate the imaging ratio (i.e. given a certain luminance on the sensor, what is the photometric exposure output) from these attributes.

Also adds a unit test to test the above.
- move factor of 100 in exposure equation to tie it to ISO
- clarify that the exposure equation also takes into account the lens system
- clarify how shutter:open/close, exposure:time, fStop and exposure:fNumber are related
the intent of hijacking the current GetExposure() on the HdCamera was to automatically and transparently upgrade existing delegates to the extended behaviour provided by the new exposure controls. However, there was a bug in the implementation: existing delegates will be expecting the result of GetExposure() to be logarithmic, i.e. they use the result by raising 2^exposure, whereas the result of UsdGeomCamera::GetExposureScale() is a simple scalar multiplier. So, to keep backwards compatibility we need to take the log2() of the exposure scale before handing it off to Hydra
# Conflicts:
#	pxr/usd/usdGeom/wrapTokens.cpp
...by making exposure hydra attribute map to exposure USD attribute,
and exposureScale map to ComputeLinearExposureScale.

This means that existing render delegates that already query exposure
will not get the more advanced behavior "for free", and will need to be
updated to query exposureScale instead, but should lessen future
confusion and potential bugs.
@jesschimein
Copy link
Collaborator

Filed as internal issue #USD-10512

(This is an automated message. See here for more information.)

@jesschimein
Copy link
Collaborator

/AzurePipelines run

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@pmolodo pmolodo force-pushed the usdGeomCamera-exposure-storm branch from adb7ebb to 1a69581 Compare December 18, 2024 20:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants