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

Request to expose the downsize to balance between quality and performance for Storm DomeLight #3342

Open
lilike-adsk opened this issue Oct 2, 2024 · 9 comments

Comments

@lilike-adsk
Copy link

Description of Issue

When pre-peocessing the envrionment map for domelight, Storm uses half of source map as the default downsize. Test shows the loading time can be reduced from 10 sec to 5 sec for an envrionment map (8,234*4,117 pixels, 100M) if the downsize is reduced from half to 1/16. Could we expose an option to adjust the downsize?

see https://github.com/PixarAnimationStudios/OpenUSD/blob/release/pxr/imaging/hdSt/domeLightComputations.cpp#L171 and
https://github.com/PixarAnimationStudios/OpenUSD/blob/release/pxr/imaging/hdSt/simpleLightingShader.cpp#L599

Steps to Reproduce

  1. Load a simple scene with dome light including a large envrionment map (e.g,. 8,234*4,117 pixels, 100M)
  2. The loading time can take 10 sec
  3. Try to reduce the downsize to 1/16 and check the loading time

System Information (OS, Hardware)

Package Versions

Build Flags

@spitzak
Copy link

spitzak commented Oct 3, 2024

Might be better to have a maximum size. The downsize is selected based on the image and maximum size. Then it would be practical for it to be a pre-set constant.

@lilike-adsk
Copy link
Author

Might be better to have a maximum size. The downsize is selected based on the image and maximum size. Then it would be practical for it to be a pre-set constant.

Right, that should also work.

@jesschimein
Copy link

Filed as internal issue #USD-10239

@spiffmon
Copy link
Member

spiffmon commented Oct 3, 2024

By no means stepping on or ignoring the need laid out here, but 10s processing a single texture seems crazy. Can I ask what texture format you are using? With large textures, and especially ones that are going to be used all over the place, light a DomeLight texture, it can be really really helpful to pre-bake MIP levels, in a format like OpenEXR that can hold them... and AVIF is now supported in USD, as well.

@lilike-adsk
Copy link
Author

It's exr format.

The most time is spent on generating the Prefilter maps 13 times for 13 mip levels (due to the size of the exr is 8,234*4,117, and hard-coded downsize is 1/2) by compute shader.

const unsigned int numPrefilterLevels = 
        std::max((unsigned int)(std::log2(std::max(srcDim[0], srcDim[1]))), 1u);

    // Prefilter map computations. mipLevel = 0 allocates texture.
    for (unsigned int mipLevel = 0; mipLevel < numPrefilterLevels; ++mipLevel) {
        const float roughness = (numPrefilterLevels == 1) ? 0.f :
            (float)mipLevel / (float)(numPrefilterLevels - 1);

        ctx.AddComputation(
            nullptr,
            std::make_shared<HdSt_DomeLightComputationGPU>(
                _tokens->domeLightPrefilter, 
                thisShader,
                numPrefilterLevels,
                mipLevel,
                roughness),
            HdStComputeQueueZero);
    }

https://github.com/PixarAnimationStudios/OpenUSD/blob/release/pxr/imaging/hdSt/simpleLightingShader.cpp#L616

@lilike-adsk
Copy link
Author

This slowness was originally found in our Hydra viewport. And it was similar case in USDView.

One simple way to validate this exr in USDView is put it to the default domelight texture folder: lib\usd\hdx\resources\textures\ and modify the code to use this exr as default domelight texture:
https://github.com/PixarAnimationStudios/OpenUSD/blob/release/pxr/imaging/hdx/package.cpp#L171, then click "Enable Default Dome Light" in USDView.

Another way to validate is through usd scene, but I'm not sure why USDView complaints "Dome light has no texture asset path", maybe there was something I missed in the usd scene.

#usda 1.0 
def DomeLight "domeLight"
{
    asset texture:file = @test.exr@
    token texture:format = "latlong"
}

@lilike-adsk
Copy link
Author

For map to test, a 8k HDRI should demostrate the slowness of loading, can't attach here as it's larger than 25M. Try grab one from https://hdri-haven.com/ or let me know if you need a test map.

@lilike-adsk
Copy link
Author

Hi, I'm thinking to add a new Hydra render setting to set the max size of processed textures (irradiance/prefilter/BRDF) , this can give opportunity to choose speed over quality or vice versa, would this make sense?

@lilike-adsk
Copy link
Author

I made a change to expose the max size as a Storm's Render Settings, will open a PR for it.
One thing we noticed that if a small size was used (e.g., 1024) for a 8K HDRI, the reflection looks blurring for materials with smooth reflection. This seems to be a limitation of the current algorithm.

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

No branches or pull requests

4 participants