Skip to content

Added HDR (arbitrary encoding) support to CubeTexturePass#16900

Closed
DanielSturk wants to merge 25 commits intomrdoob:devfrom
DanielSturk:mainline-cube-texture-pass-hdr
Closed

Added HDR (arbitrary encoding) support to CubeTexturePass#16900
DanielSturk wants to merge 25 commits intomrdoob:devfrom
DanielSturk:mainline-cube-texture-pass-hdr

Conversation

@DanielSturk
Copy link
Copy Markdown
Contributor

@DanielSturk DanielSturk commented Jun 24, 2019

I also added the MeshCubeMaterial to the WebGLBackground so that it wouldn't have to do that weird map injection (https://github.com/mrdoob/three.js/blob/dev/src/renderers/webgl/WebGLBackground.js#L90)

Examples:
webgl_postprocessing_backgrounds.html (for HDR CubeTexturePass)
webgl_materials_envmaps_hdr.html (for WebGLBackground)

@DanielSturk
WIP: Added hdr support to cubeTexturePass
c4dcfc5 
# Conflicts:
#	examples/js/postprocessing/CubeTexturePass.js
#	src/renderers/shaders/ShaderChunk.d.ts
#	src/renderers/shaders/ShaderLib.d.ts
@DanielSturk
Added hdr support to cubeTexturePass
5cc50bc
@DanielSturk
Replaced ShaderMaterial(ShaderLib.cube) with MeshCubeMaterial in WebG…
ade17f9 
…LBackground
@DanielSturk
Merge remote-tracking branch 'mrdoob/dev' into mainline-cube-texture-…
a853125 
…pass-hdr

# Conflicts:
#	examples/webgl_postprocessing_backgrounds.html
#	src/renderers/shaders/ShaderChunk.d.ts
#	src/renderers/shaders/ShaderLib.d.ts
@DanielSturk
Merge remote-tracking branch 'mrdoob/dev' into mainline-cube-texture-…
799bf1c 
…pass-hdr
@DanielSturk
Added changes to /examples/jsm. Fixed declarations
bac05aa
@Mugen87
Copy link
Copy Markdown
Collaborator

Mugen87 commented Jun 24, 2019

I guess this closes #9163?

@DanielSturk
Copy link
Copy Markdown
Contributor Author

DanielSturk commented Jun 24, 2019

I guess this closes #9163?

Yes, thank you for linking that

@WestLangley
Copy link
Copy Markdown
Collaborator

WestLangley commented Jun 24, 2019

I guess this closes #9163?

After spending almost an hour studying this PR, I was wondering how you came to have the skills to do this, and why you were making so many proposed changes in a single PR.

You used code #9163? What is different here?

@bhouston
Copy link
Copy Markdown
Contributor

bhouston commented Jun 24, 2019

@WestLangley btw to clear things up, @DanielSturk is working with me to porting back some of our private ThreeJS changes into ThreeJS mainline. Thus yeah, some of this is my code.

@bhouston bhouston mentioned this pull request Jun 24, 2019
Closed
@DanielSturk
Copy link
Copy Markdown
Contributor Author

DanielSturk commented Jun 24, 2019

In addition to webgl_postprocessing_backgrounds.html I tested it with all of
-webgl_envmap_*
-webgl_materials_variations_standard.html
-webgl_materials_standard.html
And it seems to work fine

@DanielSturk
Copy link
Copy Markdown
Contributor Author

DanielSturk commented Jun 24, 2019

You used code #9163? What is different here?

It's mostly a revive of #9163 (I didn't know that existed, #9163 has now been closed), plus I integrated it into WebGLBackground

@bhouston
Copy link
Copy Markdown
Contributor

bhouston commented Jun 24, 2019

We have checked and it does not appear to break anything, where as the previous PR did break things.

@Mugen87
Copy link
Copy Markdown
Collaborator

Mugen87 commented Jun 29, 2019

@DanielSturk If you resolve the merge conflicts, I'd like to approve this change. Introducing MeshCubeMaterial is a good idea.

@WestLangley
Copy link
Copy Markdown
Collaborator

WestLangley commented Jun 29, 2019

@WestLangley btw to clear things up, @DanielSturk is working with me to porting back some of our private ThreeJS changes into ThreeJS mainline

I see.

This PR was filed with little explanation. Is the objective to be able to blur backgrounds? If so, I am generally supportive of this.

I'll add some comments inline.

WestLangley
WestLangley reviewed Jun 29, 2019
View reviewed changes
WestLangley
WestLangley reviewed Jun 29, 2019
View reviewed changes
examples/js/postprocessing/CubeTexturePass.js
this.envMap = envMap;
this.envMapIntensity = 1.0;
this.opacity = ( opacity !== undefined ) ? opacity : 1.0;
this.roughness = 0.0;
Copy link
Copy Markdown
Collaborator

@WestLangley WestLangley Jun 29, 2019

Choose a reason for hiding this comment

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

Perhaps blurriness is a better property name in this case.

WestLangley
WestLangley reviewed Jun 29, 2019
View reviewed changes
WestLangley
WestLangley reviewed Jun 29, 2019
View reviewed changes
WestLangley
WestLangley reviewed Jun 29, 2019
View reviewed changes
WestLangley
WestLangley reviewed Jun 29, 2019
View reviewed changes
WestLangley
WestLangley reviewed Jun 29, 2019
View reviewed changes
WestLangley
WestLangley reviewed Jun 29, 2019
View reviewed changes
WestLangley
WestLangley reviewed Jun 29, 2019
View reviewed changes
WestLangley
WestLangley reviewed Jun 29, 2019
View reviewed changes
@WestLangley
Copy link
Copy Markdown
Collaborator

WestLangley commented Jun 29, 2019

I also added the MeshCubeMaterial to the WebGLBackground

I would request that you remove that for now since it changes the background behavoir, and the current background code works fine as-is.

Perhaps you can use your new material in WebGLBackground in a follow-up PR if there is agreement.

@mrdoob mrdoob modified the milestones: r128, r129 Apr 23, 2021
@mrdoob mrdoob modified the milestones: r129, r130 May 27, 2021
@mrdoob mrdoob modified the milestones: r130, r131 Jun 30, 2021
@mrdoob mrdoob modified the milestones: r131, r132 Jul 28, 2021
@mrdoob mrdoob modified the milestones: r132, r133 Aug 26, 2021
@mrdoob mrdoob modified the milestones: r133, r134 Sep 30, 2021
@mrdoob mrdoob modified the milestones: r134, r135 Oct 28, 2021
@mrdoob mrdoob modified the milestones: r135, r136 Nov 26, 2021
@mrdoob mrdoob modified the milestones: r136, r137 Dec 24, 2021
@mrdoob mrdoob modified the milestones: r137, r138 Jan 26, 2022
@mrdoob mrdoob modified the milestones: r138, r139 Feb 23, 2022
@mrdoob mrdoob modified the milestones: r139, r140 Mar 24, 2022
@mrdoob mrdoob modified the milestones: r140, r141 Apr 30, 2022
@mrdoob mrdoob modified the milestones: r141, r142 May 26, 2022
@mrdoob mrdoob removed this from the r142 milestone Jun 29, 2022
@Mugen87
Copy link
Copy Markdown
Collaborator

Mugen87 commented Nov 3, 2022

Closing this PR. Scene.background now properly supports HDR and blurriness (via PMREM). The cube shader was restored to its original implementation and is not used by WebGLBackground anymore. Since CubeTexturePass relies on cube shader, it provides just a limited feature set compared to Scene.background.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@WestLangley WestLangley WestLangley left review comments

@Mugen87 Mugen87 Mugen87 approved these changes

+1 more reviewer

@agondray agondray agondray left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@DanielSturk @Mugen87 @WestLangley @bhouston @agondray @mrdoob