Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added instructions for Windows OS users to resolve a common error by importing the OCCT DLL directory. #1347

Closed
wants to merge 1 commit into from

Conversation

farleyrunkel
Copy link
Contributor

@farleyrunkel farleyrunkel commented Jul 8, 2024

Summary by Sourcery

This pull request updates the INSTALL.md file to include a troubleshooting guide for Windows OS users, addressing a specific DLL import error and providing steps to resolve it.

  • Documentation:
    • Added a troubleshooting guide for Windows OS users to resolve the 'DLL load failed while importing _gp' error in the INSTALL.md file.

Copy link

sourcery-ai bot commented Jul 8, 2024

Reviewer's Guide by Sourcery

This pull request updates the INSTALL.md file to include a new section specifically for Windows OS users. It addresses a common ImportError related to DLL loading and provides a code snippet to resolve the issue by importing the OCCT DLL directory.

File-Level Changes

Files Changes
INSTALL.md Added instructions for Windows OS users to resolve a DLL load error by importing the OCCT DLL directory.

Tips
  • Trigger a new Sourcery review by commenting @sourcery-ai review on the pull request.
  • Continue your discussion with Sourcery by replying directly to review comments.
  • You can change your review settings at any time by accessing your dashboard:
    • Enable or disable the Sourcery-generated pull request summary or reviewer's guide;
    • Change the review language;
  • You can always contact us if you have any questions or feedback.

Copy link

@sourcery-ai sourcery-ai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @farleyrunkel - I've reviewed your changes and they look great!

Here's what I looked at during the review
  • 🟢 General issues: all looks good
  • 🟢 Security: all looks good
  • 🟢 Testing: all looks good
  • 🟢 Complexity: all looks good
  • 🟢 Documentation: all looks good

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment to tell me if it was helpful.

@farleyrunkel farleyrunkel changed the title Add use guide for windows os user Added instructions for Windows OS users to resolve a DLL load error by importing the OCCT DLL directory. Jul 8, 2024
@farleyrunkel farleyrunkel changed the title Added instructions for Windows OS users to resolve a DLL load error by importing the OCCT DLL directory. Added instructions for Windows OS users to resolve a common error by importing the OCCT DLL directory. Jul 8, 2024
@Andrej730
Copy link
Contributor

Andrej730 commented Jul 12, 2024

Is this all that was required? In my case I also was needed to provide paths to occt dependecies.

import os
os.add_dll_directory(r'occt-7.8.0-vc143-64\win64\vc14\bin')
os.add_dll_directory(r'occt-7.8.0-vc143-64\3rdparty-vc14-64\tbb2021.5-vc14-x64\bin')
os.add_dll_directory(r'occt-7.8.0-vc143-64\3rdparty-vc14-64\jemalloc-vc14-64\bin')
os.add_dll_directory(r'occt-7.8.0-vc143-64\3rdparty-vc14-64\freeimage-3.17.0-vc14-64\bin')
os.add_dll_directory(r'occt-7.8.0-vc143-64\3rdparty-vc14-64\openvr-1.14.15-64\bin\win64')
os.add_dll_directory(r'occt-7.8.0-vc143-64\3rdparty-vc14-64\ffmpeg-3.3.4-64\bin')
os.add_dll_directory(r'occt-7.8.0-vc143-64\3rdparty-vc14-64\freetype-2.5.5-vc14-64\bin')

# Test dependencies:
# requires: tbb, jemalloc
from OCC.Core import gp
# requires: freeimage, openvr, ffmpeg, freetype
from OCC.Core import V3d

from OCC.Core.gp import gp_Pnt
p = gp_Pnt(1.0, 2.0, 3.0)
print(p.X())

@farleyrunkel
Copy link
Contributor Author

@Andrej730 you are right.adding occt dependencies is also required if not adding them to system path.but in my case,adding occt bin directory to system path does not work.i don't know if you meet the same issue?

@Andrej730
Copy link
Contributor

@farleyrunkel yes, that's the same issue I've met 😁

Here's a very insightful issue about it this - https://bugs.python.org/issue43173

Basically Python is checking these places when it's trying to find a dll:

* the loaded extension module's directory
* the application directory (e.g. that of python.exe)
* the user DLL search directories that get added by 
  SetDllDirectory() and AddDllDirectory(), such as with 
  os.add_dll_directory()
* %SystemRoot%\System32
Note that the above list does not include the current working directory or %PATH% directories.

It used to work with the system path before Python 3.8 but then they decided it should work in more safe manner - https://docs.python.org/3/library/os.html#os.add_dll_directory

So, I guess the best way is to either add binaries next to .pyd files or add them using os.add_dll_directory. If you still have issues enabling OCC, you can use procmon to see what exact dlls are missing for you (in procmon you case see every path python is trying to look up and the status whether it found something by that path or not).

PS It would be nice if pythonocc would do any of these things automatically when you're trying to install it on Windows.

@farleyrunkel
Copy link
Contributor Author

@Andrej730 Yes, I think so too.

PS It would be nice if pythonocc would do any of these things automatically when you're trying to install it on Windows.

@Andrej730
Copy link
Contributor

I'm thinking maybe simple test should also be extended to include some imports that will require dll-dependencies to be successfully located by Python? I've updated my first post in this PR with an example.

@tpaviot
Copy link
Owner

tpaviot commented Jul 22, 2024

It's been a long time I stopped building occt/pythonocc on Windows. I'm now working on Linux, I only take care that the conda-build pass on Windows and OSX, but my experience with building pythonocc from scratch on Windows is quite outdated. I will welcome a PR that fixes this dll issue.

@Bill-XU
Copy link

Bill-XU commented Nov 9, 2024

@farleyrunkel @Andrej730

I wrote this, maybe more convenient.

def initialize_occt_libraries() -> None:
    """
    Initializes the OCCT libraries by setting the OCCT_ESSENTIALS_PATH environment variable and adding all DLL directories to the DLL search path.

    Raises:
        AssertionError: If the OCCT_ESSENTIALS_PATH environment variable is not set.

    """
    import os
    
    assert "OCCT_ESSENTIALS_PATH" in os.environ, "OCCT_ESSENTIALS_PATH is not set. e.x., export OCCT_ESSENTIALS_PATH=\"C:/Program Files (x86)/OpenCASCADE-7.8.1-vc14-64\""
    _occt_essentials_path = os.environ["OCCT_ESSENTIALS_PATH"]
    assert os.path.exists(_occt_essentials_path), f'OCCT_ESSENTIALS_PATH({_occt_essentials_path}) is not set correctly.'
    
    def __add_all_dll_directories(directory:str) -> None:
        """
        Recursively adds all directories containing DLL files to the DLL search path.
        Exclude path with "debug" in it.

        Args:
            directory (str): The directory to start searching from.

        """
        for root, dirs, files in os.walk(directory):
            if "debug" in root.lower():
                continue
            for file in files:
                if file.endswith(".dll"):
                    os.add_dll_directory(root)
                    break
                pass
        pass
    
    __add_all_dll_directories(_occt_essentials_path)
    pass

try:
    from OCC.Core.AIS import AIS_Shape
except ImportError:
    initialize_occt_libraries()

@Andrej730
Copy link
Contributor

I wrote this, maybe more convenient.

@Bill-XU Looks good! Do you plan to submit it as a PR? Would be nice to have it as OCC.initialize_occt_libraries. It can also have something like if platform.system() != "Windows": return at the start, so it will be safe to include to cross-platform scripts.

@Bill-XU
Copy link

Bill-XU commented Nov 12, 2024

I wrote this, maybe more convenient.

@Bill-XU Looks good! Do you plan to submit it as a PR? Would be nice to have it as OCC.initialize_occt_libraries. It can also have something like if platform.system() != "Windows": return at the start, so it will be safe to include to cross-platform scripts.

@Andrej730
I have little knowledge of progamming such a project mixed with C and Python, but will have a try.
(I suppose that the initialization process shall be included in PkgBase/init.py.)

tpaviot added a commit that referenced this pull request Nov 12, 2024
@tpaviot tpaviot mentioned this pull request Nov 12, 2024
@tpaviot
Copy link
Owner

tpaviot commented Nov 12, 2024

Superseded by #1384

@tpaviot
Copy link
Owner

tpaviot commented Nov 15, 2024

Fixed by #1384

@tpaviot tpaviot closed this Nov 15, 2024
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.

4 participants