Implementing a vectorized version of FromPickHit #3413
Conversation
… This amortizes the high cost of computing instancer information when generating pick hit information for multiple hits with one function call.
|
Amazing, glad it was straightforward and the approach looks great! We've got internal tests for this; some of the usdImaging and Presto picking tests exercise this when we turn hydra 2 on. |
|
Filed as internal issue #USD-10417 |
|
/AzurePipelines run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
pxr/imaging/hdx/pickTask.cpp
Outdated
| return resultVec; | ||
| } | ||
|
|
||
| for (int i = 0, n = hits.size(); i < n; i++) { |
There was a problem hiding this comment.
Is there a risk that i could overflow here because hits.size() is a size_t? Could you use std::transform here?
There was a problem hiding this comment.
Yes, I think changing int to size_t is the correct thing here, thanks.
std::transform I had to look up :) It seems it was introduced in C++17? So my objections to this change would be that it makes USD require C++17 (maybe it does already?).
But more importantly I think such a change would obfuscate the operation of the code (I, for example, would have had to look up std::transform to know what the modified code was doing). If there is some performance advantage to std::transform I'm happy to do it. But otherwise I'd rather leave the old-school code, as purely an aesthetic/readability choice.
|
Actually |
…ows. Addresses one of the points in feedback from @nvmkuruc.
6 participants
Description of Change(s)
Implementing a vectorized version of FromPickHit called FromPickHits. This
amortizes the high cost of computing instancer information when generating pick hit information for multiple hits with one function call.
@jesschimein @tcauchois this is my stab at vectorizing FromPickHit we discussed. This was easier than I expected. It gets Houdini's selection performance with scene index imaging up to par with UsdImagingDelegate performance using GetScenePrimPaths.
Checklist
[ X ] I have created this PR based on the dev branch
[ X ] I have followed the [coding conventions]
[ ] I have added unit tests that exercise this functionality
I did not do this because there is no unit test for FromPickHit, and I suspect a fair bit of infrastructure would be required to unit test this. I have however verified it functioning in an unreleased build of Houdini.
[ X ] I have verified that all unit tests pass with the proposed changes
[ X ] I have submitted a signed Contributor License Agreement (Reference:
Contributor License Agreement instructions)