Skip to content

Conversation

davidsminor
Copy link
Contributor

Added an ensureComponentNames() method to FnSceneShape, for forcing the computation of the component names if the shape hasn't been drawn in the gl viewport. Also added component names unit test.

@johnhaddon
Copy link
Member

Is there any reason ensureComponentNames() needs to be a public method rather than just something that happens behind the scenes at appropriate times? It's a bit sucky to have to call a function which is basically just pleaseMakeSureThatTheFunctionICallNextDoesTheRightThing().

@davidsminor
Copy link
Contributor Author

It would indeed be nice if selectionName() and componentNames() didn't require a call to glScene() to actually work. I need them to work for a unit test though. At the moment, my options are:

  1. Completely rewrite the selectionName() and componentNames() code so they're not computed directly from the IECoreGL::Scene. This is an awkward task as it's not my code and they're quite intimately tied together. I'd rather not tackle it at this point, as there's other stuff people want me to do and I don't want to introduce a load of bugs at a critical stage in production.

  2. put the following line in selectionName() and componentNames():

const_cast<SceneShapeInterface>( this )->glScene();

I'm a bit worried this might have unintended consequences when not called on the main thread - is this true? Does a deferred IECoreGL::Renderer make opengl calls?

  1. add an extra python method that I can call to get my unit test working.

I figured my lighting tool work might require a bit of a SceneShape revamp in the future to support fancy gl viewport features, and I'd probably be able to fix this more comprehensively as part of that work, so I went for the safe/minimal option, which was number 3.

@johnhaddon
Copy link
Member

  1. is what we do in FnProceduralHolder already for what I think is the same situation :

https://github.com/ImageEngine/cortex/blob/master/python/IECoreMaya/FnProceduralHolder.py#L88

Deferred IECoreGL::Renderers shouldn't make any GL calls at all (they execute procedurals in parallel, and those threads don't have a GL context). Unless testing shows a real problem then I think 2) is the better option...

@davidsminor
Copy link
Contributor Author

oh. Well that's nice then, innit... Any preferences about weather to put that in FnSceneShape.py or just put a const_cast( this )->glScene(); directly into ScneShapeInterface.cpp?

@johnhaddon
Copy link
Member

If it doesn't cause problems, then I'd go for the .cpp because it fixes the issue as close to the root cause as possible...

@davidsminor
Copy link
Contributor Author

there we go then. I'm a lot less worried about it if we were already doing it in the proc.

@andrewkaufman
Copy link
Member

Can you rebase squash those 2 commits?

SceneShapeInterface::componentNames() and SceneShapeInterface::selectionName() now automatically refresh the gl scene when they're called, in case they get called before a maya viewport refresh has happened
@davidsminor
Copy link
Contributor Author

there ya go

andrewkaufman added a commit that referenced this pull request May 20, 2014
@andrewkaufman andrewkaufman merged commit 3582375 into ImageEngine:master May 20, 2014
@davidsminor davidsminor deleted the sceneShapeComponentNames branch July 16, 2014 16:42
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.

3 participants