Skip to content

Commit

Permalink
Improve ext.Window memory safety
Browse files Browse the repository at this point in the history
  • Loading branch information
@a-hurst
a-hurst committed Sep 3, 2022
1 parent bcf3a51 commit 634c12f
Show file tree
Hide file tree
Showing 3 changed files with 56 additions and 12 deletions.
5 changes: 5 additions & 0 deletions doc/news.rst
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,9 @@ New Features:
* Updated to wrap new functions and constants in SDL2 2.24.0 (PR #246).
* Added the error-handling :func:`sdl2.ext.raise_sdl_err` function to the public
API.
* Added informative exceptions when trying perform operations (e.g. show, hide,
minimize) on a closed :obj:`sdl2.ext.Window`, and generally improved memory
safety of the Window class.

Fixed Bugs:

Expand All @@ -29,6 +32,8 @@ API Changes:

* Moved :class:`sdl2.ext.SDLError` and :func:`sdl2.ext.raise_sdl_err`
internally to a new submodule :mod:`sdl2.ext.err`.
* :obj:`sdl2.ext.Window` objects can now be repositioned when closed (would
previously raise an ``SDLError`` exception).


0.9.13
Expand Down
42 changes: 30 additions & 12 deletions sdl2/ext/window.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -95,45 +95,53 @@ def __init__(self, title, size, position=None, flags=None):
if flags is None:
flags = self.DEFAULTFLAGS

self.window = None
self._title = title
self._position = position
self._flags = flags
self._size = size

self.window = None
self.create()

def __del__(self):
"""Releases the resources of the Window, implicitly destroying the
underlying SDL2 window."""
self.close()

def _ensure_window(self, action):
if not self.window:
raise RuntimeError("Cannot {0} a closed window.".format(action))

@property
def position(self):
"""tuple: The (x, y) pixel coordinates of the top-left corner of the
window.
"""
if not self.window:
return self._position
x, y = c_int(), c_int()
video.SDL_GetWindowPosition(self.window, byref(x), byref(y))
return x.value, y.value

@position.setter
def position(self, value):
if not self.window:
raise SDLError("The window is not available")
video.SDL_SetWindowPosition(self.window, value[0], value[1])
if self.window:
video.SDL_SetWindowPosition(self.window, value[0], value[1])
self._position = value[0], value[1]

@property
def title(self):
"""str: The title of the window."""
if not self.window:
return stringify(self._title, "utf-8")
return stringify(video.SDL_GetWindowTitle(self.window), "utf-8")

@title.setter
def title(self, value):
title_bytes = utf8(value).encode('utf-8')
video.SDL_SetWindowTitle(self.window, title_bytes)
if self.window:
title_bytes = utf8(value).encode('utf-8')
video.SDL_SetWindowTitle(self.window, title_bytes)
self._title = value

@property
Expand All @@ -142,30 +150,33 @@ def size(self):
``(width, height)``.
"""
if not self.window:
return self._size
w, h = c_int(), c_int()
video.SDL_GetWindowSize(self.window, byref(w), byref(h))
return w.value, h.value

@size.setter
def size(self, value):
video.SDL_SetWindowSize(self.window, value[0], value[1])
self._size = value[0], value[1]
if self.window:
video.SDL_SetWindowSize(self.window, value[0], value[1])

def create(self):
"""Creates the window if it does not already exist."""
if self.window != None:
if self.window:
return
window = video.SDL_CreateWindow(utf8(self._title).encode('utf-8'),
self._position[0], self._position[1],
self._size[0], self._size[1],
self._flags)
if not window:
raise SDLError()
raise_sdl_err("creating the window")
self.window = window.contents

def open(self):
"""Creates and shows the window."""
if self.window is None:
if not self.window:
self.create()
self.show()

Expand All @@ -177,7 +188,7 @@ def close(self):
:meth:`create` method.
"""
if hasattr(self, "window") and self.window != None:
if self.window:
video.SDL_DestroyWindow(self.window)
self.window = None

Expand All @@ -187,6 +198,7 @@ def show(self):
If the window is already visible, this method does nothing.
"""
self._ensure_window("show")
video.SDL_ShowWindow(self.window)

def hide(self):
Expand All @@ -195,14 +207,17 @@ def hide(self):
If the window is already hidden, this method does nothing.
"""
self._ensure_window("hide")
video.SDL_HideWindow(self.window)

def maximize(self):
"""Maximizes the window."""
self._ensure_window("maximize")
video.SDL_MaximizeWindow(self.window)

def minimize(self):
"""Minimizes the window into the system's dock or task bar."""
self._ensure_window("minimize")
video.SDL_MinimizeWindow(self.window)

def restore(self):
Expand All @@ -212,6 +227,7 @@ def restore(self):
nothing.
"""
self._ensure_window("restore")
video.SDL_RestoreWindow(self.window)

def refresh(self):
Expand All @@ -222,6 +238,7 @@ def refresh(self):
modified using :meth:`get_surface`.
"""
self._ensure_window("refresh")
video.SDL_UpdateWindowSurface(self.window)

def get_surface(self):
Expand All @@ -245,7 +262,8 @@ def get_surface(self):
:obj:`~sdl2.SDL_Surface`: The surface associated with the window.
"""
self._ensure_window("get the surface of")
sf = video.SDL_GetWindowSurface(self.window)
if not sf:
raise SDLError()
raise_sdl_err("getting the window surface")
return sf.contents
21 changes: 21 additions & 0 deletions sdl2/test/sdl2ext_window_test.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,12 @@ def test_title(self, with_sdl):
assert window.title == "Window"
window.title = b"Test1234"
assert window.title == "Test1234"
video.SDL_SetWindowTitle(window.window, b"manual override")
assert window.title == "manual override"
window.close()
assert window.title == "Test1234"
window.title = "set when closed"
assert window.title == "set when closed"

def test_show_hide(self, with_sdl):
get_flags = video.SDL_GetWindowFlags
Expand All @@ -46,6 +51,11 @@ def test_show_hide(self, with_sdl):
window.hide()
assert get_flags(window.window) & SDL_WINDOW_SHOWN != SDL_WINDOW_SHOWN
window.close()
# Test informative exceptions for closed window
with pytest.raises(RuntimeError):
window.show()
with pytest.raises(RuntimeError):
window.hide()

@pytest.mark.skipif(SKIP_ANNOYING, reason="Skip unless requested")
def test_maximize(self, with_sdl):
Expand All @@ -59,6 +69,9 @@ def test_maximize(self, with_sdl):
if not DRIVER_DUMMY:
assert get_flags(window.window) & max_flag == max_flag
window.close()
# Test informative exception for closed window
with pytest.raises(RuntimeError):
window.maximize()

@pytest.mark.skipif(SKIP_ANNOYING, reason="Skip unless requested")
def test_minimize_restore(self, with_sdl):
Expand All @@ -73,6 +86,11 @@ def test_minimize_restore(self, with_sdl):
window.restore()
assert get_flags(window.window) & min_flag != min_flag
window.close()
# Test informative exceptions for closed window
with pytest.raises(RuntimeError):
window.minimize()
with pytest.raises(RuntimeError):
window.restore()

@pytest.mark.skip("not implemented")
def test_refresh(self, with_sdl):
Expand All @@ -83,6 +101,9 @@ def test_get_surface(self, with_sdl):
sf = window.get_surface()
assert isinstance(sf, surface.SDL_Surface)
window.close()
# Test informative exception for closed window
with pytest.raises(RuntimeError):
sf = window.get_surface()

def test_open_close(self, with_sdl):
get_flags = video.SDL_GetWindowFlags
Expand Down

0 comments on commit 634c12f

Please sign in to comment.