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

WebGLRenderer: Defer blocking queries against WebGLPrograms after linking. #19745

Merged
merged 1 commit into from
Oct 13, 2023

Conversation

toji
Copy link
Contributor

@toji toji commented Jun 26, 2020

Program linking can happen asynchronously, but is forced to block and wait for completion the first time information about that program is queried after linking. This includes calls to getAttribLocation/getUniformLocation/getUniform/getProgramParameter/getProgramLogInfo, etc.

This change defers most of those calls until the first time until the first time that they're actually needed by the page, which can allow the browser time to complete linking asynchronously and thus reduce blocking, especially when used in conjunction with renderer.compile().

It's worth noting that many materials are initialized the first time they are encountered by the render loop and then immediately used for rendering, which negates most of the benefit that this could otherwise provide, but it lays the groundwork for more efficient patterns in the future and can significantly help classes like PMREMGenerator that are already using renderer.compile() internally.

The ideal endpoint is for Three to start taking advantage of KHR_parallel_shader_compile as proposed by #16321 and #16662. This is just a lower-impact stepping stone along the way.

@toji
Copy link
Contributor Author

toji commented Jul 26, 2020

Review ping?

@mrdoob mrdoob added this to the r120 milestone Jul 27, 2020
@toji toji force-pushed the defer-program-queries branch 2 times, most recently from e575855 to 13f6f5b Compare July 27, 2020 16:49
@toji
Copy link
Contributor Author

toji commented Jul 27, 2020

Thanks for the feedback! Addressed all comments.

@Mugen87
Copy link
Collaborator

Mugen87 commented Jul 27, 2020

I've tested your branch locally and it seems the webgl_lightningstrike example is failing. I just get a black screen with no console logs.

@toji
Copy link
Contributor Author

toji commented Jul 27, 2020

I'll look into it, thanks!

@toji toji force-pushed the defer-program-queries branch from 13f6f5b to f4222b3 Compare July 27, 2020 21:11
@toji
Copy link
Contributor Author

toji commented Jul 27, 2020

Fixed. Explanation above.

@mrdoob mrdoob modified the milestones: r120, r121 Aug 25, 2020
@mrdoob mrdoob modified the milestones: r121, r122 Sep 29, 2020
@mrdoob mrdoob modified the milestones: r122, r123 Oct 28, 2020
@mrdoob mrdoob modified the milestones: r123, r124 Nov 25, 2020
@mrdoob mrdoob modified the milestones: r124, r125 Dec 24, 2020
@mrdoob mrdoob modified the milestones: r125, r126 Jan 27, 2021
@mrdoob mrdoob modified the milestones: r126, r127 Feb 23, 2021
@mrdoob mrdoob added this to the r149 milestone Dec 22, 2022
@mrdoob mrdoob modified the milestones: r149, r150 Jan 26, 2023
@mrdoob mrdoob modified the milestones: r150, r151 Feb 23, 2023
@mrdoob mrdoob modified the milestones: r151, r152 Mar 30, 2023
@mrdoob mrdoob modified the milestones: r152, r153 Apr 27, 2023
@Mugen87 Mugen87 modified the milestones: r153, r154 Jun 1, 2023
@mrdoob mrdoob modified the milestones: r154, r155 Jun 29, 2023
@mrdoob mrdoob modified the milestones: r155, r156 Jul 27, 2023
@mrdoob mrdoob modified the milestones: r156, r157 Aug 31, 2023
@mrdoob mrdoob modified the milestones: r157, r158 Sep 28, 2023
@toji toji force-pushed the defer-program-queries branch from 5800fb4 to 8574346 Compare October 12, 2023 18:38
Program linking can happen asynchronously, but is forced to block and wait for completion the first time information about that program is queried after linking. This includes calls to getAttribLocation/getUniformLocation/getUniform/getProgramParameter/getProgramLogInfo, etc.

This change defers most of those calls until the first time until the first time that they're actually needed by the page, which can allow the program linker time to complete and thus reduce blocking, especially when used in conjuntion with renderer.compile().

It's worth noting that many materials are initialized the first time they are encountered by the render loop and then immediately used for rendering, which negates most of the benefit that this could otherwise provide, but it lays the groundwork for more efficient patterns in the future and can significantly help classes like PMREMGenerator that are already using renderer.compile() internally.
@github-actions
Copy link

github-actions bot commented Oct 12, 2023

📦 Bundle size

Full ESM build, minified and gzipped.

Filesize dev Filesize PR Diff
651.4 kB (161.4 kB) 651.5 kB (161.4 kB) +107 B

🌳 Bundle size after tree-shaking

Minimal build including a renderer, camera, empty scene, and dependencies.

Filesize dev Filesize PR Diff
444.1 kB (107.4 kB) 444.2 kB (107.4 kB) +107 B

@toji toji force-pushed the defer-program-queries branch from 8574346 to 8ef3f65 Compare October 12, 2023 18:45
@toji
Copy link
Contributor Author

toji commented Oct 12, 2023

Rebased to resolve merge conflicts after discussion kicked up on #19752 again.

@Mugen87
Copy link
Collaborator

Mugen87 commented Oct 13, 2023

Since I think Ricardo feels positive about this change and supporting KHR_parallel_shader_compile is a valuable addition to the engine, I'll merge so we can move on with #19752.

@Mugen87 Mugen87 merged commit da47522 into mrdoob:dev Oct 13, 2023
19 checks passed
@Mugen87 Mugen87 changed the title Defer blocking queries against WebGLPrograms after linking WebGLRenderer: Defer blocking queries against WebGLPrograms after linking. Oct 13, 2023
@toji
Copy link
Contributor Author

toji commented Oct 13, 2023

Thank you!

@toji toji deleted the defer-program-queries branch October 13, 2023 16:23
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

Successfully merging this pull request may close these issues.

5 participants