PMREMGenerator: Remove calls of convertSRGBToLinear().

[Mugen87]

Related issue: see #22286 (comment)

Description

PMREMGenerator now honors WebGLRenderer.outputEncoding when converting to linear color space.

This PR removes calls of convertSRGBToLinear(). Background/clear colors are assumed to be in linear color space.

[Mugen87]

webgl_furnace_test assumes sRGB for Scene.background.

three.js/examples/webgl_furnace_test.html

Line 113 in f4601f3

envScene.background = new THREE.Color( COLOR ); // sRGB colorspace assumed, apparently

That means it is now necessary to set WebGLRenderer.outputEncoding to sRGBEncoding which slightly changes its visuals.

[WestLangley]

That means it is now necessary to set WebGLRenderer.outputEncoding to sRGBEncoding

No, that does not follow. The furnace test can be rendered in any colorspace. I'd remove the changes to the furnace test.

I think it would be best if @gkjohnson reviews the PMREM edits in this PR.

[Mugen87]

I've edited the example since Scene.background is explicitly defined as a sRGB color (see comment).

[WestLangley]

I've edited the example since Scene.background is explicitly defined as a sRGB color (see comment).

I wrote the comment, and the comment is referring to the fact that PMREMGenerator is assuming the color is sRGB. This PR supposedly fixes that limitation of PMREMGenerator.

The furnace test is broken until other issues with PMREMGenerator are fixed.

[gkjohnson]

The changes to PMREMGenerator look good -- I'm not sure if any other types of output encodings are ever used for the renderer but my only comment is if only SRGB and Linear are supported and other output encodings might be used by users (not sure why they would be) you might want to log a warning.

I'm not familiar enough with the furnace test or the interpretation of RGBE colors in the shaders to be much help there, though. @elalish might be a better one to ping when you start looking into that more.

[Mugen87]

I'm not sure if any other types of output encodings are ever used for the renderer

Um, I guess I would add support for THREE.GammaEncoding. The conversion can be done via convertGammaToLinear(). For all other constants, a warning should be logged.

[Mugen87]

With the last commit I've added support for THREE.GammaEncoding. PMREMGenerator also logs a warning now when color spaces other than linear, gamma and srgb are used.

[WestLangley]

@Mugen87 @gkjohnson Sorry, but this PR is not correct.

PMREMGenerator.fromScene() was written to create a PMREM from something like Room Environment. The current renderer's output colorspace is irrelevant in that case. envScene background color should be specified in linear space because the renderer will render the PMREM in linear space. The background color should not be converted by PMREM.

If @gkjohnson is creating a PMREM from his current scene, that would be different use case.

EDIT- And thank you @Mugen87 for working on this because it has helped me to clarify the problem. I think my statements are correct. :-)

[Mugen87]

The background color should not be converted by PMREM.

But in this case, any calls of convertSRGBToLinear() should be removed from PMREMGenerator, right?

[WestLangley]

But in this case, any calls of convertSRGBToLinear() should be removed from PMREMGenerator, right?

Yes.

And as I said, sRGB output encoding in webgl_furnace_test.html is optional, and should be noted as such. The conversion of envScene background color to linear (if needed) belongs at the app level.

[Mugen87]

Changed the title of the PR and updated the code.

[gkjohnson]

If @gkjohnson is creating a PMREM from his current scene, that would be different use case.

I see what you're saying. The couple times I've used PMREMGenerator it's been to render my current scene in order to get reflections on objects in that scene. It seems it would be difficult to simply support both use cases here.

This comes back to something like #22275 and / or bookkeeping color space information on the Color class so the renderer has the relevant information to interpret the colors correctly.

[WestLangley]

Merging for now so #22326 can be fixed.

[mrdoob]

Thanks!