remove virtual dispatch for PxrMayaHdShapeAdapter::IsViewport2() #451
Conversation
| @@ -177,7 +179,8 @@ class UsdMayaGL_InstancerImager : public TfWeakBase { | |||
| /// no factory has been set, returns a UsdMayaGL_InstancerShapeAdapter base | |||
| /// class object. The caller must manage the lifescope of the returned | |||
| /// object. | |||
| static UsdMayaGL_InstancerShapeAdapter* CreateInstancerShapeAdapter(); | |||
| static UsdMayaGL_InstancerShapeAdapter* CreateInstancerShapeAdapter( | |||
| const bool isViewport2); | |||
There was a problem hiding this comment.
constness on the pass-by-value argument is not necessary. Can you remove it?
Regarding to coding style, we are likely to have 100 chars per line so there is no need to break a new line.
There was a problem hiding this comment.
@kxl-adsk Have we published the coding style guildline somewhere for partners?
There was a problem hiding this comment.
Another question: can this argument be true by default?
There was a problem hiding this comment.
clang-format is published on TSC forum...pending review.
There was a problem hiding this comment.
On the argument const-ness, yeah, I can remove that.
About defaulting it to true, I had thought of doing that initially, but if we made it default to true here, it seemed like it should default to true all the way down to the shape adapter constructors. It seemed safer to me to require callers to specify a value rather than accidentally end up creating a shape adapter for the wrong viewport. If you feel strongly though, I can revisit that.
There was a problem hiding this comment.
Rebased and removed the const qualifiers in f0d2225.
There was a problem hiding this comment.
I don't have strong preference, it was just a question.
| @@ -64,10 +64,11 @@ UsdMayaProxyDrawOverride::UsdMayaProxyDrawOverride(const MObject& obj) : | |||
| MHWRender::MPxDrawOverride(obj, | |||
| UsdMayaProxyDrawOverride::draw | |||
| #if MAYA_API_VERSION >= 201651 | |||
There was a problem hiding this comment.
If we don't care about pre-2018 maya versions, we should also remove this conditional compilation.
There was a problem hiding this comment.
Makes sense. I can put up a separate PR for that later since it's not directly related to the changes here, and I'm sure there are a bunch of other places where we're testing for pre-2018 versions.
| /// | ||
| /// Returns true if the shape adapter should be used for batched | ||
| /// drawing/selection in Viewport 2.0, or false if it should be used | ||
| /// in the legacy viewport. | ||
| MAYAUSD_CORE_PUBLIC | ||
| virtual bool IsViewport2() const; | ||
| bool IsViewport2() const { |
There was a problem hiding this comment.
Just to be sure, we don't need to export this method?
There was a problem hiding this comment.
Right, we don't need to export this anymore since it's now defined inline right here.
This change also requires that the viewport renderer affiliation for the shape adapter be set when the adapter is constructed. It is not intended to change over the lifetime of the shape adapter. (Internal change: 2058187)
e916dcc to
f0d2225
Compare
…dra adapters for UFE items created by maya-usd) (#451) * HYDRA-289 Add checks to prevent creating adapters for UFE items created by maya-usd * HYDRA-289 Add extra check to prevent creating light adapters for USD's UFE lights in PreFrame light query step * HYDRA-289 Whitespace changes * HYDRA-289 Whitespace changes * HYDRA-289 : Move mayaUsd plugin check for tests into usdUtils * HYDRA-289 : Move check from usdUtils to mtohUtils * HYDRA-289 : Revert usdUtils changes * HYDRA-289 : Add test for ensuring skipping UFE items from maya-usd * HYDRA-289 Move comments describing UFE to where IsUfeItemFromMayaUsd is declared
Following on from #450, we want to start being more explicit about the division between the legacy viewport and Viewport 2.0 so that it's easier to tear out the legacy stuff a little bit at a time.
There might also be a slight performance improvement here with
PxrMayaHdShapeAdapter::IsViewport2()no longer dispatching virtually.