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

Pr/batched texture #1436

Merged
merged 18 commits into from
Dec 1, 2021
Merged

Conversation

AlexMWells
Copy link
Contributor

@AlexMWells AlexMWells commented Nov 18, 2021

Description

Add batched support for 2d texture calls, 3d texture calls, environment, and gettextureinfo.

To enable, added:

bool RenderServices::is_udim (TextureHandle *texture_handle);

TextureSystem::TextureHandle* 
BatchedRendererServices::resolve_udim_uniform(...,TextureSystem::TextureHandle*, float S, float T);

void 
BatchedRendererServices::resolve_udim(...,TextureSystem::TextureHandle*,Wide<const float> wS, Wide<const float> wT, Masked<TextureSystem::TextureHandle*> wresult);

which allows batched code gen to resolve_udim's for varying S & T and bin the resulting TextureHandle* and make calls to get_texture_info_uniform with a single resolved udim texture handle, then broadcast the results to data lanes with matching TextureHandle *'s.

Batched texture options are implemented on the llvm code gen stack using a temporary and directly populating vs. calling out to helper functions. Strategy is to have any varying inputs to uniform members of the batched texture options be binned, so calls to the underlying texture subystem only have to deal with an expected set of uniform options and subset of varying options. This means that the batched code gen handles extracting the 1st active lane of varying data from variables populating texture options and reducing the mask to lanes that match the same options, we refer to this as binning.

NOTE: The default BatchedRendererService implementation turns around and calls the texture subsystem once for each active lane, it will be up to the texture subsystem to add support to processes multiple texture requests in the future to better take advantage of outer loop SIMD.

Tests

Added extensive new regression tests:

gettextureinfo-reg
gettextureinfo-udim-reg
texture-opts-reg
texture3d-opts-reg
texture-environment-opts-reg

Enabled BATCHED execution of existing tests:

gettextureinfo-udim
gettextureinfo
texture3d
texture-alpha-derivs
texture-alpha
texture-blur
texture-connected-options
texture-derivs
texture-environment
texture-errormsg
texture-firstchannel
texture-interp
texture-missingalpha
texture-missingcolor
texture-simple
texture-smallderivs
texture-swirl
texture-udim
texture-width
texture-withderivs
texture-wrap

Checklist:

  • I have read the contribution guidelines.
  • I have previously submitted a Contributor License Agreement.
  • I have updated the documentation, if applicable.
  • I have ensured that the change is tested somewhere in the testsuite (adding new test cases if necessary).
  • My code follows the prevailing code style of this project.

@AlexMWells AlexMWells force-pushed the PR/BatchedTexture branch 2 times, most recently from 95c0e4a to 9d95c23 Compare November 22, 2021 19:22
@AlexMWells
Copy link
Contributor Author

Discovered mismatch between documentation, llvm_gen implementation, and oslc when passing user derivatives to texture3d.
oslc and documentation expect optional parameters "vector dpdx, vector dpdy, vector dpdz",
however llvm_gen and optexture implementation only accept "vector dpdx, vector dpdy"
and use 0 for dpdz when calling into the texture subsystem.

This commit includes changes to oslc to make the compiler match the implementation of accepting only "vector dpdx, vector dpdy". This change could have (un)intended consequences, such as causing existing .osl shaders that explicitly pass dpdz fail to compile. This is probably preferable to inform user that there is an issue. The osl-language.pdf still needs to be updated, assuming that not supporting dpdz is the intended behavior. Otherwise, llvm_gen and optexture would need to be updated to properly handle dpdz.

@AlexMWells
Copy link
Contributor Author

After conferring with lgritz, it was determined that user provided dPdz should be respected and llvm_gen should be fixed to properly handle it. commit reverts the changes made and fixes the behavior for texture3d when user provided derivates are passed.

To enable, added:
  bool RenderServices::is_udim (TextureHandle *texture_handle)
  TextureSystem::TextureHandle* BatchedRendererServices::resolve_udim_uniform(...,TextureSystem::TextureHandle*, float S, float T)
  void BatchedRendererServices::resolve_udim(...,TextureSystem::TextureHandle*,Wide<const float> wS, Wide<const float> wT, Masked<TextureSystem::TextureHandle*> wresult)

which allows batched code gen to resolve_udim's for varying S & T and bin the resulting TextureHandle* and make calls to get_texture_info_uniform with a single resolved udim texture handle, then broadcast the results to data lanes with matching TextureHandle *'s.

Added extensive new regression tests:
    gettextureinfo-reg
    gettextureinfo-udim-reg
    texture-opts-reg

Enabled BATCHED execution of existing tests:
    gettextureinfo-udim
    gettextureinfo
    texture-alpha-derivs
    texture-alpha
    texture-blur
    texture-connected-options
    texture-derivs
    texture-errormsg
    texture-firstchannel
    texture-interp
    texture-missingalpha
    texture-missingcolor
    texture-simple
    texture-smallderivs
    texture-swirl
    texture-udim
    texture-width
    texture-withderivs
    texture-wrap

Signed-off-by: Alex M. Wells <[email protected]>
Signed-off-by: Alex M. Wells <[email protected]>
…eing checked, when it should have been OpenImageIO_VERSION.

Added quiet option to oiio-tool calls in regression tests to allow produced out.txt to match between baseline and batched runs.

Signed-off-by: Alex M. Wells <[email protected]>
…ing us to avoid generating warnings for file already exists that was causing differences when comparing baseline/out.txt and out.txt in regression tests

Signed-off-by: Alex M. Wells <[email protected]>
…ture option.

Fixed some other compilation issues when OSL_DEV is defined.

Signed-off-by: Alex M. Wells <[email protected]>
Discovered mismatch between documentation, llvm_gen implementation, and oslc when passing user derivatives to texture3d.
oslc and documentation expect optional parameters "vector dpdx, vector dpdy, vector dpdz",
however llvm_gen and optexture implementation only accept "vector dpdx, vector dpdy"
and use 0 for dpdz when calling into the texture subsystem.

This commit includes changes to oslc to make the compiler match the implementation of accepting only "vector dpdx, vector dpdy".

Enable BATCHED execution of testsuite/texture3d
Add new regression test textur3d-opts-reg

Signed-off-by: Alex M. Wells <[email protected]>
… only user inputs that include dpdz: "vector dpdx, vector dpdy, vector dpdz".

Fixed llvm_gen and optexture to properly extract user dpdz from texture3d options or initialize a Vec3(0) and pass through to texture subsystem.
Modified batched_llvm_gen and wide_optexture to properly extract user dpdz from texture3d options or initialize a Wide<Vec3>(0) and pass through to texture subsystem.
Updated texture3d-opts-reg test to pass dpdz for calls taking user derivatives.

Signed-off-by: Alex M. Wells <[email protected]>
Added BatchedRendererServices::environment virtual method

Added new test environment
Added new regression test environment-opt-reg

Signed-off-by: Alex M. Wells <[email protected]>
…t-opts-reg to line up with a different PR adding testsuite/texture-environment.

Signed-off-by: Alex M. Wells <[email protected]>
…rted option for environment.

Added test and handling for "time" option to batched texture3d.  Current choice is to just silently ignore "time" as currently supported 3d formats do not support time.

Signed-off-by: Alex M. Wells <[email protected]>
Enabled BATCHED execution of texture-environment

Signed-off-by: Alex M. Wells <[email protected]>
…and transform-reg.regress.batched.opt which were causing spurious CI timeouts.

Reduced some multipliers of user supplied derivs to environment tests to reduce differences in results.

Signed-off-by: Alex M. Wells <[email protected]>
@AlexMWells AlexMWells requested a review from lgritz November 24, 2021 23:34
Copy link
Collaborator

@lgritz lgritz left a comment

Choose a reason for hiding this comment

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

LGTM

@lgritz lgritz merged commit e7ca092 into AcademySoftwareFoundation:main Dec 1, 2021
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.

2 participants