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

SDL 1 CI build fixes #1832

Merged
merged 8 commits into from May 22, 2020
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion .travis.yml
Expand Up @@ -72,7 +72,7 @@ install:
$PIP_CMD install --upgrade pip
$PIP_CMD install --upgrade numpy
$PYTHON_EXE setup.py -config -auto $WHICH_SDL_BUILD
$PYTHON_EXE setup.py install -pygame-ci -noheaders -auto
$PYTHON_EXE setup.py install -pygame-ci -noheaders
fi
- |
if [[ "$TRAVIS_OS_NAME" == "osx" ]] && [[ "$TRAVIS_PULL_REQUEST" == "false" ]] && [[ -n "$TRAVIS_TAG" ]]; then
Expand Down
10 changes: 8 additions & 2 deletions src_c/alphablit.c
Expand Up @@ -26,8 +26,14 @@
#include "_surface.h"

#ifdef PG_ENABLE_ARM_NEON
// sse2neon.h is from here: https://github.com/DLTcollab/sse2neon
#include "include/sse2neon.h"
// sse2neon.h is from here: https://github.com/DLTcollab/sse2neon
#include "include/sse2neon.h"
#else
#if IS_SDLv1
Copy link
Member

Choose a reason for hiding this comment

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

👍

// SDL 1 doesn't import the latest intrinsics, this should should pull
// them all in for us
#include <immintrin.h>
#endif /* IS_SDLv1 */
#endif /* PG_ENABLE_ARM_NEON */

/* The structure passed to the low level blit functions */
Expand Down
4 changes: 2 additions & 2 deletions src_c/base.c
Expand Up @@ -584,8 +584,8 @@ pg_get_error(PyObject *self, PyObject *args)
#if IS_SDLv1 && PY3 && !defined(PYPY_VERSION)
/* SDL 1's encoding is ambiguous */
PyObject *obj;
if (obj = PyUnicode_DecodeUTF8(SDL_GetError(),
strlen(SDL_GetError()), "strict"))
if ((obj = PyUnicode_DecodeUTF8(SDL_GetError(),
strlen(SDL_GetError()), "strict")))
return obj;
PyErr_Clear();
return PyUnicode_DecodeLocale(SDL_GetError(), "surrogateescape");
Expand Down
10 changes: 5 additions & 5 deletions src_c/image.c
Expand Up @@ -30,7 +30,7 @@

#include "doc/image_doc.h"

#if __SSE4_2__ || PG_COMPILE_SSE4_2
#if (__SSE4_2__ || PG_COMPILE_SSE4_2) && (SDL_VERSION_ATLEAST(2, 0, 0))
#include <emmintrin.h>
/* SSSE 3 */
#include <tmmintrin.h>
Expand Down Expand Up @@ -320,7 +320,7 @@ image_get_extended(PyObject *self, PyObject *arg)
return PyInt_FromLong(GETSTATE(self)->is_extended);
}

#if __SSE4_2__ || PG_COMPILE_SSE4_2
#if (__SSE4_2__ || PG_COMPILE_SSE4_2) && (SDL_VERSION_ATLEAST(2, 0, 0))
#define SSE42_ALIGN_NEEDED 16
#define SSE42_ALIGN __attribute__((aligned(SSE42_ALIGN_NEEDED)))

Expand Down Expand Up @@ -476,7 +476,7 @@ tostring_surf_32bpp_sse42(SDL_Surface *surf, int flipped, char *data,
}
}
}
#endif /* __SSE4_2__ || PG_COMPILE_SSE4_2 */
#endif /* __SSE4_2__ || PG_COMPILE_SSE4_2 && (SDL_VERSION_ATLEAST(2, 0, 0)) */


#if IS_SDLv2
Expand Down Expand Up @@ -510,7 +510,7 @@ tostring_surf_32bpp(SDL_Surface *surf, int flipped,
Uint32 Bloss = surf->format->Bloss;
Uint32 Aloss = surf->format->Aloss;

#if __SSE4_2__ || PG_COMPILE_SSE4_2
#if (__SSE4_2__ || PG_COMPILE_SSE4_2) && (SDL_VERSION_ATLEAST(2, 0, 0))
if (/* SDL uses Uint32, SSE uses int for building vectors.
* Related, we assume that Uint32 is packed so 4 of
* them perfectly matches an __m128i.
Expand Down Expand Up @@ -542,7 +542,7 @@ tostring_surf_32bpp(SDL_Surface *surf, int flipped,
color_offset, alpha_offset);
return;
}
#endif /* __SSE4_2__ || PG_COMPILE_SSE4_2 */
#endif /* __SSE4_2__ || PG_COMPILE_SSE4_2 && (SDL_VERSION_ATLEAST(2, 0, 0)) */

for (h = 0; h < surf->h; ++h) {
Uint32 *pixel_row = (Uint32 *)DATAROW(
Expand Down
18 changes: 17 additions & 1 deletion test/rect_test.py
Expand Up @@ -2,11 +2,12 @@
import sys
import unittest

from pygame import Rect, Vector2
from pygame import Rect, Vector2, get_sdl_version
from pygame.tests import test_utils


PY3 = sys.version_info >= (3, 0, 0)
SDL1 = get_sdl_version()[0] < 2


class RectTypeTest(unittest.TestCase):
Expand Down Expand Up @@ -844,6 +845,7 @@ def test_clip(self):
r1, r1.clip(Rect(r1)), "r1 does not clip an identical rect to itself"
)

@unittest.skipIf(SDL1, "rect.clipline not available in SDL1")
def test_clipline(self):
"""Ensures clipline handles four int parameters.

Expand All @@ -861,6 +863,7 @@ def test_clipline(self):
self.assertIsInstance(clipped_line, tuple)
self.assertTupleEqual(clipped_line, expected_line)

@unittest.skipIf(SDL1, "rect.clipline not available in SDL1")
def test_clipline__two_sequences(self):
"""Ensures clipline handles a sequence of two sequences.

Expand All @@ -883,6 +886,7 @@ def test_clipline__two_sequences(self):
self.assertIsInstance(clipped_line, tuple)
self.assertTupleEqual(clipped_line, expected_line)

@unittest.skipIf(SDL1, "rect.clipline not available in SDL1")
def test_clipline__sequence_of_four_ints(self):
"""Ensures clipline handles a sequence of four ints.

Expand All @@ -899,6 +903,7 @@ def test_clipline__sequence_of_four_ints(self):
self.assertIsInstance(clipped_line, tuple)
self.assertTupleEqual(clipped_line, expected_line)

@unittest.skipIf(SDL1, "rect.clipline not available in SDL1")
def test_clipline__sequence_of_two_sequences(self):
"""Ensures clipline handles a sequence of two sequences.

Expand All @@ -924,6 +929,7 @@ def test_clipline__sequence_of_two_sequences(self):
self.assertIsInstance(clipped_line, tuple)
self.assertTupleEqual(clipped_line, expected_line)

@unittest.skipIf(SDL1, "rect.clipline not available in SDL1")
def test_clipline__floats(self):
"""Ensures clipline handles float parameters."""
rect = Rect((1, 2), (35, 40))
Expand All @@ -943,6 +949,7 @@ def test_clipline__floats(self):
self.assertIsInstance(clipped_line, tuple)
self.assertTupleEqual(clipped_line, expected_line)

@unittest.skipIf(SDL1, "rect.clipline not available in SDL1")
def test_clipline__no_overlap(self):
"""Ensures lines that do not overlap the rect are not clipped."""
rect = Rect((10, 25), (15, 20))
Expand All @@ -962,6 +969,7 @@ def test_clipline__no_overlap(self):

self.assertTupleEqual(clipped_line, expected_line)

@unittest.skipIf(SDL1, "rect.clipline not available in SDL1")
def test_clipline__both_endpoints_outside(self):
"""Ensures lines that overlap the rect are clipped.

Expand Down Expand Up @@ -1009,6 +1017,7 @@ def test_clipline__both_endpoints_outside(self):

self.assertTupleEqual(clipped_line, expected_line)

@unittest.skipIf(SDL1, "rect.clipline not available in SDL1")
def test_clipline__both_endpoints_inside(self):
"""Ensures lines that overlap the rect are clipped.

Expand Down Expand Up @@ -1040,6 +1049,7 @@ def test_clipline__both_endpoints_inside(self):

self.assertTupleEqual(clipped_line, expected_line)

@unittest.skipIf(SDL1, "rect.clipline not available in SDL1")
def test_clipline__endpoints_inside_and_outside(self):
"""Ensures lines that overlap the rect are clipped.

Expand Down Expand Up @@ -1092,6 +1102,7 @@ def test_clipline__endpoints_inside_and_outside(self):

self.assertTupleEqual(clipped_line, expected_line)

@unittest.skipIf(SDL1, "rect.clipline not available in SDL1")
def test_clipline__edges(self):
"""Ensures clipline properly clips line that are along the rect edges.
"""
Expand Down Expand Up @@ -1128,6 +1139,7 @@ def test_clipline__edges(self):

self.assertTupleEqual(clipped_line, expected_line)

@unittest.skipIf(SDL1, "rect.clipline not available in SDL1")
def test_clipline__equal_endpoints_with_overlap(self):
"""Ensures clipline handles lines with both endpoints the same.

Expand All @@ -1149,6 +1161,7 @@ def test_clipline__equal_endpoints_with_overlap(self):

self.assertTupleEqual(clipped_line, expected_line)

@unittest.skipIf(SDL1, "rect.clipline not available in SDL1")
def test_clipline__equal_endpoints_no_overlap(self):
"""Ensures clipline handles lines with both endpoints the same.

Expand All @@ -1163,6 +1176,7 @@ def test_clipline__equal_endpoints_no_overlap(self):

self.assertTupleEqual(clipped_line, expected_line)

@unittest.skipIf(SDL1, "rect.clipline not available in SDL1")
def test_clipline__zero_size_rect(self):
"""Ensures clipline handles zero sized rects correctly."""
expected_line = ()
Expand All @@ -1174,6 +1188,7 @@ def test_clipline__zero_size_rect(self):

self.assertTupleEqual(clipped_line, expected_line)

@unittest.skipIf(SDL1, "rect.clipline not available in SDL1")
def test_clipline__negative_size_rect(self):
"""Ensures clipline handles negative sized rects correctly."""
expected_line = ()
Expand Down Expand Up @@ -1229,6 +1244,7 @@ def test_clipline__negative_size_rect(self):

self.assertTupleEqual(clipped_line, expected_line)

@unittest.skipIf(SDL1, "rect.clipline not available in SDL1")
def test_clipline__invalid_line(self):
"""Ensures clipline handles invalid lines correctly."""
rect = Rect((0, 0), (10, 20))
Expand Down
32 changes: 18 additions & 14 deletions test/surface_test.py
Expand Up @@ -24,6 +24,7 @@
import ctypes

IS_PYPY = "PyPy" == platform.python_implementation()
SDL1 = pygame.get_sdl_version()[0] < 2


def intify(i):
Expand Down Expand Up @@ -2402,26 +2403,29 @@ def test_premul_surf(src_col,
src_size=(17, 67),
dst_size=(69, 69)
))

self.assertEqual(*test_premul_surf(pygame.Color(30, 20, 0, 255),
pygame.Color(40, 20, 0, 51),
src_size=(17, 67),
dst_size=(69, 69),
src_has_alpha=False,
))
# These tests trigger some of the weird SDL1 alpha blending behaviour
Copy link
Member

Choose a reason for hiding this comment

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

It makes me a little nervous that these aren't correct on SDL1. Are the tests correct, or is SDL1 incorrect? Has the behaviour changed in these blit routines now? Do the SDL2 ones not match the SDL1 ones?

Copy link
Member

Choose a reason for hiding this comment

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

This doesn't need a response necessarily :) I was just thinking out loud.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No, you are right to ask I'm definitely going to poke at it and see if I can figure out why this works differently.

I mean in theory you don't need to do pre-multiplied alpha blending in any of those cases because both surfaces don't have alpha but I believe I would expect it to just work boringly without going through the fancy intrinsic code....

Anyway, it just didn't seem the moment to go off on a tangent figuring that out in this PR.

if not SDL1:
self.assertEqual(*test_premul_surf(pygame.Color(30, 20, 0, 255),
pygame.Color(40, 20, 0, 51),
src_size=(17, 67),
dst_size=(69, 69),
src_has_alpha=False,
))
self.assertEqual(*test_premul_surf(pygame.Color(30, 20, 0, 51),
pygame.Color(40, 20, 0, 255),
src_size=(17, 67),
dst_size=(69, 69),
dst_has_alpha=False,
))
self.assertEqual(*test_premul_surf(pygame.Color(30, 20, 0, 255),
pygame.Color(40, 20, 0, 255),
src_size=(17, 67),
dst_size=(69, 69),
src_has_alpha=False,
dst_has_alpha=False,
))
# These tests trigger some of the weird SDL1 alpha blending behaviour
if not SDL1:
self.assertEqual(*test_premul_surf(pygame.Color(30, 20, 0, 255),
pygame.Color(40, 20, 0, 255),
src_size=(17, 67),
dst_size=(69, 69),
src_has_alpha=False,
dst_has_alpha=False,
))


def test_blit_blend_big_rect(self):
Expand Down
32 changes: 18 additions & 14 deletions test/video_test.py
@@ -1,21 +1,25 @@
import unittest
import pygame
from pygame._sdl2 import video

SDL2 = pygame.get_sdl_version()[0] >= 2

class VideoModuleTest(unittest.TestCase):
default_caption = "pygame window"
if SDL2:
from pygame._sdl2 import video

def test_renderer_set_viewport(self):
""" works.
"""
window = video.Window(title=self.default_caption, size=(800, 600))
renderer = video.Renderer(window=window)
renderer.logical_size = (1920, 1080)
rect = pygame.Rect(0, 0, 1920, 1080)
renderer.set_viewport(rect)
self.assertEqual(renderer.get_viewport(), (0, 0, 1920, 1080))

class VideoModuleTest(unittest.TestCase):
default_caption = "pygame window"

if __name__ == "__main__":
unittest.main()
def test_renderer_set_viewport(self):
""" works.
"""
window = video.Window(title=self.default_caption, size=(800, 600))
renderer = video.Renderer(window=window)
renderer.logical_size = (1920, 1080)
rect = pygame.Rect(0, 0, 1920, 1080)
renderer.set_viewport(rect)
self.assertEqual(renderer.get_viewport(), (0, 0, 1920, 1080))


if __name__ == "__main__":
unittest.main()