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

Imaging as shared lib #8340

Closed
wants to merge 7 commits into from
Closed

Conversation

homm
Copy link
Member

@homm homm commented Sep 1, 2024

No description provided.

@aclark4life
Copy link
Member

Interesting!

@homm
Copy link
Member Author

homm commented Sep 1, 2024

@aclark4life Not sure the work will be finished ) Right now I just need various test environments from this PR.

@Yay295
Copy link
Contributor

Yay295 commented Sep 1, 2024

Is there any way in the build we can declare the minimum C version? pyproject.toml has requires-python = ">=3.9" for the Python version, but I don't see a setting that can be used for C. NumPy uses Meson, so their minimum C version is declared in the meson.build file.

@homm
Copy link
Member Author

homm commented Sep 1, 2024

Build with python ./setup.py develop --pillow-configuration=parallel=1.

Currently, the _imaging extension contains a batch of very useful functions and types that are not available in other extensions (such as _webp or _imagingcms). As a result, these other extensions do not utilize the useful functions from _imaging (for example, _webp is the only extension that uses ImagingSectionEnter/Leave) and do not perform any type checking. Here is a typical code example from extensions that use the Imaging type:

static PyObject *
cms_transform_apply(CmsTransformObject *self, PyObject *args) {
    Py_ssize_t idIn;
    Py_ssize_t idOut;
    Imaging im;
    Imaging imOut;

    int result;

    if (!PyArg_ParseTuple(args, "nn:apply", &idIn, &idOut)) {
        return NULL;
    }

    im = (Imaging)idIn;
    imOut = (Imaging)idOut;

    result = pyCMSdoTransform(im, imOut, self->transform);

    return Py_BuildValue("i", result);
}

So, it just converts an arbitrary Py_ssize_t to a pointer, which is not safe. What I'm trying to achieve is to use the _imaging extension as a library for other extensions. Unfortunately, as I see it, this is only possible by changing the name to lib_imaging since gcc -l always appends the lib prefix and there is no way to specify the full library name.

@Yay295
Copy link
Contributor

Yay295 commented Sep 1, 2024

_imaging also contains a bunch of things for the imaging module though, which aren't needed elsewhere. I think it would be cleaner to leave _imaging as it is, and create a new library that _imaging depends on. Then code can be moved from _imaging to the new library, and the other extensions can also be changed to use the new library.

@homm
Copy link
Member Author

homm commented Sep 2, 2024

@Yay295 This involves massive extension redesign, I'm not ready for this )

@radarhere radarhere mentioned this pull request Sep 11, 2024
@homm homm closed this Sep 18, 2024
@homm homm mentioned this pull request Sep 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants