mgsx-dev / gdx-gltf

GLTF 2.0 3D format support and PBR shader implementation for LibGDX
Apache License 2.0
214 stars 50 forks source link

Allow separate radiance map creation #112

Closed dar-dev closed 1 year ago

dar-dev commented 1 year ago

Quick PR that allows the creation of separate radiance map differently sized from global environment map. This is useful when we want to use the main envmap for skybox and a small radiance cubemap.

--

mgsx-dev commented 1 year ago

you can already do that in your own code by calling getEnvMap with a different resolution and then call getRadianceMap

dar-dev commented 1 year ago

Not sure. When calling getEnvMap() the previous envmap is destroyed, so I will lose the skybox. Here's the src code:

// create env map if needed
if (!iblSettings.envMapValid)
{
    environmentCubemap = iblComposer.getEnvMap(skyboxSize, envExposure);

    // show skybox
    if (isShowIBLSkybox())
    {
        // gamma correction handling
        final Float gamma = gammaCorrectionEnabled ? gammaCorrection : null;

        // create skybox
        skybox = new SceneSkybox(environmentCubemap, SRGB.FAST, gamma);
        sceneManager.setSkyBox(skybox);
    }
}

// create irradiance map if needed
if (!iblSettings.irradianceValid)
{
    irradianceCubemap = iblComposer.getIrradianceMap(32);
}

// create radiance map if needed
if (!iblSettings.radianceValid)
{
    radianceCubemap = iblComposer.getSeparateRadianceMap(radianceSize, envExposure);
}

Do I miss something ?

mgsx-dev commented 1 year ago

Order matters : getEnvMap(low res) getRadianceMap() getEnvMap(high res) getIrradianceMap()

dar-dev commented 1 year ago

Order matters : getEnvMap(low res) getRadianceMap() getEnvMap(high res) getIrradianceMap()

Sorry, I'm not getting the point. The problem is: whenever the user changes an ibl setting (i.e. radianceSize) I invalidate it and I will re-create the new map. So.. the call to getEnvMap(low res) will destroy the previous envmap used for skybox.

This is the new impl following the suggested order:

// create radiance map if needed
if (!iblSettings.radianceValid)
{
    iblComposer.getEnvMap(radianceSize, envExposure);
    radianceCubemap = iblComposer.getRadianceMap(radianceSize);
    // radianceCubemap = iblComposer.getSeparateRadianceMap(radianceSize, envExposure);
}

// create irradiance map if needed
if (!iblSettings.irradianceValid)
{
    iblComposer.getEnvMap(32, envExposure);
    irradianceCubemap = iblComposer.getIrradianceMap(32);
}

// create env map if needed
if (!iblSettings.envMapValid)
{
    environmentCubemap = iblComposer.getEnvMap(skyboxSize, envExposure);

    // always dispose prev skybox
    disableSkybox();

    // show skybox
    if (isShowIBLSkybox())
    {
        // gamma correction handling
        final Float gamma = gammaCorrectionEnabled ? gammaCorrection : null;

        // create skybox
        skybox = new SceneSkybox(environmentCubemap, SRGB.FAST, gamma);
        sceneManager.setSkyBox(skybox);
    }
}

Result: the skybox disappears every time I change the radiance size.

This PR is just a small addition without breaking anything. Users like me need to handle maps separately creating them in any order.

--

mgsx-dev commented 1 year ago

i'm wondering why you want env map to be the same size as other maps, it makes radiance/irradiance sampling useless. It's better to sample from an high res cube map, even if other maps are smaller.

mgsx-dev commented 1 year ago

it'd be better to discuss on discord instead.

dar-dev commented 1 year ago

it'd be better to discuss on discord instead.

I replied on discord