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.

Sign up for GitHub

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

Jump to bottom

WebGLRenderer: Remove gammaFactor and GammaEncoding. #23080

Merged
merged 1 commit into from
Dec 23, 2021
Merged

WebGLRenderer: Remove gammaFactor and GammaEncoding. #23080

merged 1 commit into from
Dec 23, 2021

Conversation

Mugen87
Copy link
Collaborator

@Mugen87 Mugen87 commented Dec 23, 2021

Related issue: -

Description

This PR removes WebGLRenderer.gammaFactor and THREE.GammaEncoding.

I think the only values WebGLRenderer.outputEncoding should accept are THREE.sRGBEncoding and THREE.LinearEncoding(with THREE.sRGBEncoding as the future default).

Users who want a special gamma correction have to use post processing and a custom version of GammaCorrectionShader now.

Supporting THREE.GammaEncoding for Texture.encoding seems not necessary to me. It should be safe when the renderer assumes only textures with THREE.sRGBEncoding or THREE.LinearEncoding are processed by its internal logic.

@mrdoob
Copy link
Owner

mrdoob commented Dec 23, 2021

This sounds good to me 👍
@WestLangley do you agree?

@mrdoob
Copy link
Owner

mrdoob commented Dec 23, 2021

(with THREE.sRGBEncoding as the future default).

While we are refactoring/cleaning up all this... Maybe we can set ouputEncoding to THREE.sRGBEncoding by default now too? (In another PR).

@Mugen87
Copy link
Collaborator Author

Mugen87 commented Dec 23, 2021

I guess I would do this after #23019 is being resolved and the entire color space/tone mapping workflow works as expected.

@mrdoob
Copy link
Owner

mrdoob commented Dec 23, 2021

Sounds good! 👍

@donmccurdy
Copy link
Collaborator

Thanks! We could also remove the copyGamma / convertGamma methods in THREE.Color I think.

Please hold off on changing the default outputEncoding a bit longer, yes.👍

@donmccurdy
Copy link
Collaborator

I think the only values WebGLRenderer.outputEncoding should accept are THREE.sRGBEncoding and THREE.LinearEncoding ...

We may want a few more over time (Display P3 when WebGL gets support?) but this sounds good for now.

@Mugen87
Copy link
Collaborator Author

Mugen87 commented Dec 23, 2021 •
edited
Loading

We could also remove the copyGamma / convertGamma methods in THREE.Color I think.

Maybe in a separate PR so this one does not get too big.

@mrdoob mrdoob added this to the r136 milestone Dec 23, 2021
@mrdoob
Copy link
Owner

mrdoob commented Dec 23, 2021

Merging this for now.

@mrdoob mrdoob merged commit 0e21088 into mrdoob:dev Dec 23, 2021
@WestLangley
Copy link
Collaborator

@WestLangley do you agree?

Yes.

I think the only values WebGLRenderer.outputEncoding should accept are THREE.sRGBEncoding and THREE.LinearEncoding

No, that is not correct, but the issue can be discussed later.

@mrdoob
Copy link
Owner

mrdoob commented Dec 23, 2021

Thanks!

donmccurdy added a commit to donmccurdy/three.js that referenced this pull request Dec 24, 2021
@donmccurdy
Color: Remove copyGammaToLinear, copyLinearToGamma, convertGammaToLin…
f72fc3e 
…ear, convertLinearToGamma.

See: mrdoob#23080
@donmccurdy donmccurdy mentioned this pull request Dec 24, 2021
Merged
@donmccurdy
Copy link
Collaborator

Following up in #23082.

donmccurdy added a commit to donmccurdy/three.js that referenced this pull request Dec 24, 2021
@donmccurdy
Color: Remove copyGammaToLinear, copyLinearToGamma, convertGammaToLin…
5d3400c 
…ear, convertLinearToGamma.

See: mrdoob#23080
mrdoob pushed a commit that referenced this pull request Dec 24, 2021
@donmccurdy
Color: Remove gamma-related methods (#23082)
9f56628 
* Color: Remove copyGammaToLinear, copyLinearToGamma, convertGammaToLinear, convertLinearToGamma.

See: #23080

* Color: Update docs for removed methods.
drcmda pushed a commit to pmndrs/three-stdlib that referenced this pull request Jan 1, 2022
@CodyJasonBennett
fix: remove depreciated encodings with r136 (#108)
3be7b18 
* fix: remove RGBM7Encoding and RGBM16Encoding

Mirrors mrdoob/three.js#23046

* fix(ColorSpaceNode): remove RGBDEncoding

Mirrors mrdoob/three.js#23049

* fix: remove RGBEEncoding and RGBEFormat

* fix: remove gammaFactor and GammaEncoding

Mirrors mrdoob/three.js#23080

* fix: unmangle typo in RGBMLoader
@joshuaellis joshuaellis mentioned this pull request Jan 3, 2022
Closed
9 tasks
Mercury21th000 pushed a commit to Mercury21th000/three-stdlib that referenced this pull request Nov 17, 2024
@CodyJasonBennett
fix: remove depreciated encodings with r136 (#108)
3fc5064 
* fix: remove RGBM7Encoding and RGBM16Encoding

Mirrors mrdoob/three.js#23046

* fix(ColorSpaceNode): remove RGBDEncoding

Mirrors mrdoob/three.js#23049

* fix: remove RGBEEncoding and RGBEFormat

* fix: remove gammaFactor and GammaEncoding

Mirrors mrdoob/three.js#23080

* fix: unmangle typo in RGBMLoader
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
r136
Development

Successfully merging this pull request may close these issues.
4 participants
@Mugen87 @mrdoob @donmccurdy @WestLangley