Redesigning the shading mode export interface #168
Conversation
|
If we combine these changes with #166 , we can completely remove the need for the branching logic when reading in the shading modes from the commands, because it is possible to re-register the displayColor exporter under multiple aliases, pxrRis under an alias that displayed properly in the Option Menu, and register an empty exporter for None. The two combined also helps writing flexible shader exporters for other applications, without relying on a custom build. |
| @@ -1,264 +1,41 @@ | |||
| // | |||
There was a problem hiding this comment.
Just wondering about the removal of the copyright notice here -- was that intentional? If not, could we put it back?
| SdfPathSet _bindableRoots; | ||
| }; | ||
|
|
||
| typedef boost::function< void (PxrUsdMayaShadingModeExportContext*) > PxrUsdMayaShadingModeExporterContext; |
There was a problem hiding this comment.
From what we could tell, this typedef isn't used anywhere -- could you confirm and remove if it isn't?
Otherwise, (and this is nit-picky) -- could we rename this typedef (and use a C++11 type alias), since this type isn't really a context, it's a function accepting a context?
|
|
||
| exporter(&c); | ||
| auto exporter = exporterCreator(); | ||
| if (exporter != nullptr) { |
There was a problem hiding this comment.
Since we used an inline assignment at line 320, can we make this if check also do the same for consistency? e.g.,
if (auto exporter = exporterCreator()) { ... }
sirpalee
force-pushed
the
pr_rewriting_shading_mode_interface
branch
from
37ae3ec
to
28b0d3c
Compare
|
@sunyab Thanks for the feedback! About the copyright notice, it was probably lost during a rebase when we switched between our branch and yours, nothing intentional there. For future reference, what do you prefer when addressing feedback? Updating the original commit with the changes (cleaner history), or making an extra commit (simpler tracking of changes)? |
|
@sirpalee Thanks for the changes! I think I'd prefer to squash and get the cleaner history for now. |
sirpalee
force-pushed
the
pr_rewriting_shading_mode_interface
branch
from
28b0d3c
to
1c36c76
Compare
|
@sunyab Done! If you are happy with the current state, I'll go ahead and change the import interface in a similar manner. |
…face Redesigning the shading mode export interface
### Description of Change(s) Add more for lineStyle schema. ### Fixes Issue(s)
Description of Change(s)
This change deals with redesigning the shading mode export/import interface. As a first step, it's a take on the export interface. I tried to create a bit more flexible export process, so exporters can overwrite the whole process too if they don't want to go through the exporter context.
This is just a take on the shading mode export, so feedback can be gathered, before moving on to the importers.
Fixes Issue(s)