GafferDelight : Import IECoreDelight on Windows #5082
Conversation
ericmehl
force-pushed
the
gafferDelight-Windows-fix
branch
from
798e7cd to
b3215a4
Compare
python/GafferDelight/__init__.py
Outdated
| # GafferDelight does not need any symbols from IECoreDelight, so MSVC tries | ||
| # to be helpful and not load IECoreDelight.dll. Skipping that means that | ||
| # 3Delight is never registered as a renderer, so we import / register it manually. | ||
|
|
||
| import os | ||
|
|
||
| ieCoreDelight = None | ||
|
|
||
| if os.name == "nt" and ieCoreDelight is None : | ||
|
|
||
| import ctypes | ||
|
|
||
| try : | ||
| ieCoreDelight = ctypes.CDLL( "IECoreDelight.dll" ) | ||
| except : | ||
| raise ImportError |
There was a problem hiding this comment.
In #5040, you mentioned that the IECoreDelight module had this same problem, so I think perhaps it would be best to put the fix in IECoreDelight/__init__.py instead? I presume that would be necessary to get the IECoreDelightTest module passing. Then we could just __import__( IECoreDelight ) at the top of this file, so the same fix works for GafferDelight.
There was a problem hiding this comment.
Yes, that's definitely a better solution. Fixed in e95e2f3
python/GafferDelight/__init__.py
Outdated
|
|
||
| import os | ||
|
|
||
| ieCoreDelight = None |
There was a problem hiding this comment.
This variable seems to be unused (other than to be assigned again at L54) - do we really need it?
There was a problem hiding this comment.
I also use it to on L49 to check that we haven't already imported IECoreDelight.dll. I think that was left over from a debugging effort though. I don't think there's much harm in asking for an already loaded library to be loaded.
There was a problem hiding this comment.
Is it possible for it to be set True between L47 and L49 though?
There was a problem hiding this comment.
Ahhhhhh, right. Don't mind me. 🗿 Fixed in e95e2f3
The IECoreDelight module does not need any symbols from IECoreDelight, so MSVC tries to be helpful and not load IECoreDelight.dll. Skipping that means that 3Delight is never registered as a renderer, so we import / register it manually.
ericmehl
force-pushed
the
gafferDelight-Windows-fix
branch
from
b3215a4 to
e95e2f3
Compare
|
I have the two comments fixed up, ready for another review as long as CI succeeds. |
GafferDelight does not need any symbols from IECoreDelight, so MSVC tries to be helpful and not load IECoreDelight.dll. Skipping that means that 3Delight is never registered as a renderer, so we import / register it manually.
This was causing errors loading the UI because GafferDelight was not throwing an exception when it was expected to. It also caused Gaffer to crash when trying to render with 3delight.
Fixes #5081
Checklist