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

WebGPURenderer: Pipelines - Clean up. #28495

Merged
merged 2 commits into from
May 27, 2024
Merged

WebGPURenderer: Pipelines - Clean up. #28495

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
83783da
WebGPURenderer: Pipelines - Clean up.
Methuselah96 May 27, 2024
d5dcdf0
Update Attributes
Methuselah96 May 27, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions examples/jsm/renderers/common/Attributes.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@ class Attributes extends DataMap {

}

return attributeData;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated another case where DataMap.delete is being overridden.


}

update( attribute, type ) {
Expand Down
4 changes: 2 additions & 2 deletions examples/jsm/renderers/common/Pipelines.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ class Pipelines extends DataMap {

if ( pipeline === undefined ) {

if ( previousPipeline && previousPipeline.usedTimes === 0 ) this._releasePipeline( computeNode );
if ( previousPipeline && previousPipeline.usedTimes === 0 ) this._releasePipeline( previousPipeline );
Copy link
Contributor Author

@Methuselah96 Methuselah96 May 27, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

I think this is just a typo? this._releasePipeline takes a pipeline and uses its cacheKey property to remove it from this.caches.


pipeline = this._getComputePipeline( computeNode, stageCompute, cacheKey, bindings );

Expand Down Expand Up @@ -205,7 +205,7 @@ class Pipelines extends DataMap {

}

super.delete( object );
return super.delete( object );
Copy link
Contributor Author

@Methuselah96 Methuselah96 May 27, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

This method is overriding DataMap.delete which returns the value for the deleted key. For consistency with how the base method works, it seems best to continue returning the deleted value.


}

Expand Down