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 module cleanup #8389

Open
wants to merge 17 commits into
base: main
Choose a base branch
from
Open

_imaging module cleanup #8389

wants to merge 17 commits into from

Conversation

homm
Copy link
Member

@homm homm commented Sep 18, 2024

Includes some useful changers from #8340.

Improvements

  • Move some useful macro such as MAX and MIN to ImagingUtils.h 517f16b
  • ImPlatform.h is renamed to ImagingPlatform.h d73e8c2
  • All #undef *INT* moved to Jpeg.h 8909052
  • Simplify #define *INT* by requiring stdint.h (C99) 92bf691
  • #include <Python.h> once in Imaging.h 84f0261
  • #include <math.h> in ImagingPlatform.h fbd4c98
  • Include all global libs with <*.h> instead of "*.h" 108602b
  • Removed add-imaging-libs option (is not documented, not sure why we need it) c3172e8
  • Use native functions instead of PyImaging_CheckBuffer and PyImaging_GetBuffer e972962
  • Use Py_RETURN_NONE macro when possible f58cd7d
  • Remove unused HAVE_LIBMPEG ffa0230
  • Move new_block method to _imaging and fix some functions arguments type e7bce42
  • Remove unused core.convert function e7bce42
  • Simplify Imaging_Type checking in _convert b89f791
  • Use PyErr_Format instead of sprintf, use native PyErr_Clear function, remove extra PyExc_TypeError after PySequence_Fast, remove unused Except.c file 05a67d1

src/libImaging/Jpeg.h Outdated Show resolved Hide resolved
@@ -34,9 +34,6 @@

#include "Imaging.h"

#include <math.h>
#include <stdint.h>
Copy link
Member Author

@homm homm Sep 18, 2024

Choose a reason for hiding this comment

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

One of the reason I think we can rely on C99 is stdint.h was included here unconditionally.

Copy link
Contributor

Choose a reason for hiding this comment

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

The default standard used by GCC on CentOS 7 was gnu90, which is not C99, but does support stdint.h. CentOS 7 is no longer supported though. The conclusion from this discussion (#6516) was essentially that if it compiles in all of the test environments, it's okay to use.

#if defined(_MSC_VER) && !defined(__GNUC__)
#define inline __inline
#endif
#endif

#if defined(_WIN32) || defined(__CYGWIN__) /* WIN */
Copy link
Member Author

Choose a reason for hiding this comment

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

I will examine this and return if needed:
#5807

Copy link
Member

Choose a reason for hiding this comment

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

Sorry, I'm not completely clear on your meaning - reconsidering this change is a to-do item for yourself before this PR is ready for merge?

src/_imagingcms.c Outdated Show resolved Hide resolved
Co-authored-by: Andrew Murray <3112309+radarhere@users.noreply.github.com>
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