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

Fixes for the SDL1 build #1842

Merged
merged 12 commits into from May 23, 2020
Merged

Fixes for the SDL1 build #1842

merged 12 commits into from May 23, 2020

Conversation

MyreMylar
Copy link
Contributor

@MyreMylar MyreMylar commented May 23, 2020

Three main fixes so far:

  • For draw.c, the recent optimisations don't compile on SDL1 because SDL_IntersectRect wasn't exposed outside the library until SDL 2.

  • With the recent windows SDK SDL 1 doesn't compile unless you define WINDOWS_IGNORE_PACKING_MISMATCH it doesn't seem like defining this does any harm apart from suppressing this warning in the windows headers (discovered here: Fix mhook build error. microsoft/vcpkg#10302)

  • I noticed I messed up the FORCEINLINE macro so it was spamming warnings everywhere when building SDL 1.

  • It looks like there are a few more MSVC warnings in surflock.c:

    src_c/surflock.c(52): warning C4133: 'function': incompatible types - from 'PyObject *' to 'pgSurfaceObject *'
    src_c/surflock.c(52): warning C4133: 'function': incompatible types - from 'pgSurfaceObject *' to 'PyObject *'
    src_c/surflock.c(62): warning C4133: 'function': incompatible types - from 'PyObject *' to 'pgSurfaceObject *'
    src_c/surflock.c(62): warning C4133: 'function': incompatible types - from 'pgSurfaceObject *' to 'PyObject *'
    src_c/surflock.c(69): warning C4133: 'function': incompatible types - from 'pgSurfaceObject *' to 'PyObject *'
    src_c/surflock.c(75): warning C4133: 'function': incompatible types - from 'pgSurfaceObject *' to 'PyObject *'
    src_c/surflock.c(229): warning C4133: 'function': incompatible types - from 'PyObject *' to 'pgSurfaceObject *'
    src_c/surflock.c(248): warning C4133: 'function': incompatible types - from 'PyObject *' to 'pgSurfaceObject *'

  • and in draw.c:

    src_c/draw.c(968): warning C4133: 'function': incompatible types - from 'pgSurfaceObject *' to 'PyObject *'
    src_c/draw.c(975): warning C4133: 'function': incompatible types - from 'pgSurfaceObject *' to 'PyObject *'
    src_c/draw.c(868): warning C4101: 'clipped': unreferenced local variable
    src_c/draw.c(867): warning C4101: 'result': unreferenced local variable
    src_c/draw.c(866): warning C4101: 'cliprect': unreferenced local variable
    src_c/draw.c(865): warning C4101: 'sdlrect': unreferenced local variable

  • and imageext:

    src_c/imageext.c(750): warning C4133: 'return': incompatible types - from 'PyObject *' to 'SDL_Surface *'
    src_c/imageext.c(753): warning C4133: 'return': incompatible types - from 'PyObject *' to 'SDL_Surface *'
    src_c/imageext.c(759): warning C4133: 'return': incompatible types - from 'PyObject *' to 'SDL_Surface *'
    src_c/imageext.c(779): warning C4133: 'return': incompatible types - from 'PyObject *' to 'SDL_Surface *'

  • and pypm.c:

    src_c/pypm.c(4466): warning C4113: 'PtTimestamp (__cdecl *)()' differs in parameter lists from 'PmTimeProcPtr' This is in generated Cython code

  • and ft_render.c:

    src_c/freetype/ft_render.c(325): warning C4244: '=': conversion from 'long' to 'Uint16', possible loss of data
    src_c/freetype/ft_render.c(339): warning C4244: '=': conversion from 'long' to 'Uint16', possible loss of data
    src_c/freetype/ft_render.c(747): warning C4244: '=': conversion from 'long' to 'Uint16', possible loss of data
    src_c/freetype/ft_render.c(759): warning C4244: '=': conversion from 'long' to 'Uint16', possible loss of data

  • and freetype.c:

    src_c/_freetype.c(1841): warning C4047: '=': 'PyObject *' differs in levels of indirection from 'int'
    src_c/_freetype.c(1973): warning C4047: '=': 'PyObject *' differs in levels of indirection from 'int'

  • and alphablit.c:

    src_c/alphablit.c(317): warning C4102: 'LEAVE': unreferenced label

  • and bitmask.c:

    src_c/bitmask.c(70): warning C4293: '>>': shift count negative or too big, undefined behavior This looks correct to me, but I could be wrong.

Which I will also take a look at.

Round 2:

  • mask.c:

    src_c/mask.c(793): warning C4133: 'function': incompatible types - from 'PyObject *' to 'pgSurfaceObject *'
    src_c/mask.c(818): warning C4133: 'function': incompatible types - from 'PyObject *' to 'pgSurfaceObject *'
    src_c/mask.c(1053): warning C4133: 'function': incompatible types - from 'PyObject *' to 'pgSurfaceObject *'
    src_c/mask.c(1055): warning C4133: 'function': incompatible types - from 'PyObject *' to 'pgSurfaceObject *'
    src_c/mask.c(1063): warning C4133: 'function': incompatible types - from 'PyObject *' to 'pgSurfaceObject *'
    src_c/mask.c(1065): warning C4133: 'function': incompatible types - from 'PyObject *' to 'pgSurfaceObject *'
    src_c/mask.c(2188): warning C4133: 'function': incompatible types - from 'PyObject *' to 'pgSurfaceObject *'
    src_c/mask.c(2195): warning C4133: 'function': incompatible types - from 'PyObject *' to 'pgSurfaceObject *'
    src_c/mask.c(2202): warning C4133: 'function': incompatible types - from 'PyObject *' to 'pgSurfaceObject *'
    src_c/mask.c(2215): warning C4133: 'function': incompatible types - from 'PyObject *' to 'pgSurfaceObject *'
    src_c/mask.c(2220): warning C4133: 'function': incompatible types - from 'PyObject *' to 'pgSurfaceObject *'
    src_c/mask.c(2225): warning C4133: 'function': incompatible types - from 'PyObject *' to 'pgSurfaceObject *'

  • transform.c :

    src_c/transform.c(569): warning C4133: 'function': incompatible types - from 'PyObject *' to 'pgSurfaceObject *'
    src_c/transform.c(579): warning C4133: 'function': incompatible types - from 'PyObject *' to 'pgSurfaceObject *'
    src_c/transform.c(669): warning C4133: 'function': incompatible types - from 'PyObject *' to 'pgSurfaceObject *'
    src_c/transform.c(675): warning C4133: 'function': incompatible types - from 'PyObject *' to 'pgSurfaceObject *'
    src_c/transform.c(736): warning C4133: 'function': incompatible types - from 'PyObject *' to 'pgSurfaceObject *'
    src_c/transform.c(742): warning C4133: 'function': incompatible types - from 'PyObject *' to 'pgSurfaceObject *'
    src_c/transform.c(772): warning C4133: 'function': incompatible types - from 'PyObject *' to 'pgSurfaceObject *'
    src_c/transform.c(895): warning C4133: 'function': incompatible types - from 'PyObject *' to 'pgSurfaceObject *'
    src_c/transform.c(919): warning C4133: 'function': incompatible types - from 'PyObject *' to 'pgSurfaceObject *'
    src_c/transform.c(935): warning C4133: 'function': incompatible types - from 'PyObject *' to 'pgSurfaceObject *'
    src_c/transform.c(1459): warning C4133: 'function': incompatible types - from 'PyObject *' to 'pgSurfaceObject *'
    src_c/transform.c(1478): warning C4133: 'function': incompatible types - from 'PyObject *' to 'pgSurfaceObject *'
    src_c/transform.c(1886): warning C4133: 'function': incompatible types - from 'PyObject *' to 'pgSurfaceObject *'
    src_c/transform.c(1887): warning C4133: 'function': incompatible types - from 'PyObject *' to 'pgSurfaceObject *'
    src_c/transform.c(1889): warning C4133: 'function': incompatible types - from 'PyObject *' to 'pgSurfaceObject *'
    src_c/transform.c(1898): warning C4133: 'function': incompatible types - from 'PyObject *' to 'pgSurfaceObject *'
    src_c/transform.c(1899): warning C4133: 'function': incompatible types - from 'PyObject *' to 'pgSurfaceObject *'
    src_c/transform.c(1901): warning C4133: 'function': incompatible types - from 'PyObject *' to 'pgSurfaceObject *'
    src_c/transform.c(2721): warning C4133: 'function': incompatible types - from 'PyObject *' to 'pgSurfaceObject *'
    src_c/transform.c(2742): warning C4133: 'function': incompatible types - from 'PyObject *' to 'pgSurfaceObject *'

  • image.c :

    src_c/image.c(629): warning C4133: 'function': incompatible types - from 'PyObject *' to 'pgSurfaceObject *'
    src_c/image.c(634): warning C4133: 'function': incompatible types - from 'PyObject *' to 'pgSurfaceObject *'
    src_c/image.c(645): warning C4133: 'function': incompatible types - from 'PyObject *' to 'pgSurfaceObject *'
    src_c/image.c(713): warning C4133: 'function': incompatible types - from 'PyObject *' to 'pgSurfaceObject *'
    src_c/image.c(728): warning C4133: 'function': incompatible types - from 'PyObject *' to 'pgSurfaceObject *'
    src_c/image.c(794): warning C4133: 'function': incompatible types - from 'PyObject *' to 'pgSurfaceObject *'
    src_c/image.c(805): warning C4133: 'function': incompatible types - from 'PyObject *' to 'pgSurfaceObject *'
    src_c/image.c(864): warning C4133: 'function': incompatible types - from 'PyObject *' to 'pgSurfaceObject *'
    src_c/image.c(880): warning C4133: 'function': incompatible types - from 'PyObject *' to 'pgSurfaceObject *'
    src_c/image.c(957): warning C4133: 'function': incompatible types - from 'PyObject *' to 'pgSurfaceObject *'
    src_c/image.c(973): warning C4133: 'function': incompatible types - from 'PyObject *' to 'pgSurfaceObject *'
    src_c/image.c(1050): warning C4133: 'function': incompatible types - from 'PyObject *' to 'pgSurfaceObject *'

  • surface.c :

    src_c/surface.c(887): warning C4133: 'function': incompatible types - from 'PyObject *' to 'pgSurfaceObject *'
    src_c/surface.c(936): warning C4133: 'function': incompatible types - from 'PyObject *' to 'pgSurfaceObject *'
    src_c/surface.c(993): warning C4133: 'function': incompatible types - from 'PyObject *' to 'pgSurfaceObject *'
    src_c/surface.c(1021): warning C4133: 'function': incompatible types - from 'PyObject *' to 'pgSurfaceObject *'
    src_c/surface.c(1054): warning C4133: 'function': incompatible types - from 'PyObject *' to 'pgSurfaceObject *'
    src_c/surface.c(1078): warning C4133: 'function': incompatible types - from 'PyObject *' to 'pgSurfaceObject *'
    src_c/surface.c(1132): warning C4133: 'function': incompatible types - from 'PyObject *' to 'pgSurfaceObject *'
    src_c/surface.c(1140): warning C4133: 'function': incompatible types - from 'PyObject *' to 'pgSurfaceObject *'
    src_c/surface.c(1981): WARNING: "srcsurf doesn't actually do anything?"
    src_c/surface.c(2156): warning C4133: 'function': incompatible types - from 'PyObject *' to 'pgSurfaceObject *'
    src_c/surface.c(2158): warning C4133: 'function': incompatible types - from 'PyObject *' to 'pgSurfaceObject *'
    src_c/surface.c(2504): warning C4133: 'function': incompatible types - from 'PyObject *' to 'pgSurfaceObject *'
    src_c/surface.c(2538): warning C4133: 'function': incompatible types - from 'PyObject *' to 'pgSurfaceObject *'
    src_c/surface.c(2790): warning C4133: 'function': incompatible types - from 'PyObject *' to 'pgSurfaceObject *'
    src_c/surface.c(2799): warning C4133: 'function': incompatible types - from 'PyObject *' to 'pgSurfaceObject *'
    src_c/surface.c(3007): warning C4133: 'function': incompatible types - from 'PyObject *' to 'pgSurfaceObject *'
    src_c/surface.c(3159): warning C4133: 'function': incompatible types - from 'PyObject *' to 'pgSurfaceObject *'
    src_c/surface.c(3700): warning C4133: 'function': incompatible types - from 'PyObject *' to 'pgSurfaceObject *'
    src_c/surface.c(3743): warning C4133: 'function': incompatible types - from 'PyObject *' to 'pgSurfaceObject *'

@MyreMylar
Copy link
Contributor Author

MyreMylar commented May 23, 2020

Fixing the warnings in draw.c uncovered a whole extra mess because of the mismatch between the functions as they are defined in surflock.c and as they were defined in _pygame.h. I guess these two got out of sync at some point.

Assuming this bunch of fixes work, I'll poke the newly uncovered problems .... right now it turns out.

Copy link
Contributor

@nthykier nthykier left a comment

Choose a reason for hiding this comment

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

The RAISE-then-return commit seems to be a step in the wrong direction? Can you comment on the rationale for it? (AFAICT, RAISE's purpose is exactly for this case and if it is not useful here, we should drop the macro entirely)

@MyreMylar
Copy link
Contributor Author

MyreMylar commented May 23, 2020

The RAISE-then-return commit seems to be a step in the wrong direction? Can you comment on the rationale for it? (AFAICT, RAISE's purpose is exactly for this case and if it is not useful here, we should drop the macro entirely)

I believe that the RAISE macro returns a PyObject* and the function I changed it on returns an SDL_Surface *. I think the macro is fine it was just being used wrongly in that particular place.

@MyreMylar
Copy link
Contributor Author

That's all the warnings on my local SDL1 build. We'll have to see what else Travis turns up.

@MyreMylar
Copy link
Contributor Author

The RAISE-then-return commit seems to be a step in the wrong direction? Can you comment on the rationale for it? (AFAICT, RAISE's purpose is exactly for this case and if it is not useful here, we should drop the macro entirely)

I believe that the RAISE macro returns a PyObject* and the function I changed it on returns an SDL_Surface *. I think the macro is fine it was just being used wrongly in that particular place.

And then of course Travis prefers it the way where it's cast and then returned so I switched it back :)

@MyreMylar MyreMylar marked this pull request as ready for review May 23, 2020 20:19
@MyreMylar
Copy link
Contributor Author

OK, looks like Travis is happy. I may do a take 3 of this another day to fix any remaining gcc only warnings but that is enough for now I think.

Copy link
Member

@illume illume left a comment

Choose a reason for hiding this comment

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

👍 awesome

@illume illume merged commit cc273c7 into pygame:master May 23, 2020
@MyreMylar MyreMylar deleted the fix-sd1-build-take2 branch June 5, 2020 16:41
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.

None yet

3 participants