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

Work around PyQt memory leak on the Krita::documents() method. #1359

Merged
merged 1 commit into from
Nov 5, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 8 additions & 4 deletions ai_diffusion/document.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
from .layer import Layer, LayerManager, LayerType
from .pose import Pose
from .localization import translate as _
from .util import acquire_elements


class Document(QObject):
Expand Down Expand Up @@ -120,7 +121,10 @@ def __init__(self, krita_document: krita.Document):
@classmethod
def active(cls):
if doc := Krita.instance().activeDocument():
if doc not in Krita.instance().documents() or doc.activeNode() is None:
if (
doc not in acquire_elements(Krita.instance().documents())
or doc.activeNode() is None
):
return None
id = doc.rootNode().uniqueId().toString()
return cls._instances.get(id) or KritaDocument(doc)
Expand Down Expand Up @@ -237,7 +241,7 @@ def current_time(self):

@property
def is_valid(self):
return self._doc in Krita.instance().documents()
return self._doc in acquire_elements(Krita.instance().documents())

@property
def is_active(self):
Expand Down Expand Up @@ -303,14 +307,14 @@ def update(self):

layer = cast(krita.VectorLayer, layer.node)
pose = self._layers.setdefault(layer.uniqueId(), Pose(doc.extent))
self._update(layer, layer.shapes(), pose, doc.resolution)
self._update(layer, acquire_elements(layer.shapes()), pose, doc.resolution)

def add_character(self, layer: krita.VectorLayer):
doc = KritaDocument.active()
assert doc is not None
pose = self._layers.setdefault(layer.uniqueId(), Pose(doc.extent))
svg = Pose.create_default(doc.extent, pose.people_count).to_svg()
shapes = layer.addShapesFromSvg(svg)
shapes = acquire_elements(layer.addShapesFromSvg(svg))
self._update(layer, shapes, pose, doc.resolution)

def _update(
Expand Down
12 changes: 8 additions & 4 deletions ai_diffusion/layer.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
from PyQt5.QtGui import QImage

from .image import Extent, Bounds, Image
from .util import ensure, maybe, client_logger as log
from .util import acquire_elements, ensure, maybe, client_logger as log
from . import eventloop


Expand Down Expand Up @@ -143,7 +143,11 @@ def parent_layer(self):

@property
def child_layers(self):
return [self._manager.wrap(child) for child in self._node.childNodes() if _is_real(child)]
return [
self._manager.wrap(child)
for child in acquire_elements(self._node.childNodes())
if _is_real(child)
]

@property
def is_root(self):
Expand Down Expand Up @@ -199,7 +203,7 @@ def get_mask(self, bounds: Bounds | None = None, time: int | None = None):

def move_to_top(self):
parent = self._node.parentNode()
if parent.childNodes()[-1] == self._node:
if acquire_elements(parent.childNodes())[-1] == self._node:
return # already top-most layer
with RestoreActiveLayer(self._manager):
parent.removeChildNode(self.node)
Expand Down Expand Up @@ -556,7 +560,7 @@ def __bool__(self):


def traverse_layers(node: krita.Node, type_filter: list[str] | None = None):
for child in node.childNodes():
for child in acquire_elements(node.childNodes()):
type = child.type()
if _is_real(type) and (not type_filter or type in type_filter):
yield child
Expand Down
16 changes: 15 additions & 1 deletion ai_diffusion/util.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,12 @@
import statistics
import zipfile
from typing import Any, Callable, Iterable, Optional, Sequence, TypeVar
from PyQt5.QtCore import QStandardPaths
from PyQt5 import sip
from PyQt5.QtCore import QObject, QStandardPaths

T = TypeVar("T")
R = TypeVar("R")
QOBJECT = TypeVar("QOBJECT", bound=QObject)

is_windows = sys.platform.startswith("win")
is_macos = sys.platform == "darwin"
Expand Down Expand Up @@ -243,3 +245,15 @@ def _extract_member(self, member, targetpath, pwd):


ZipFile = LongPathZipFile if is_windows else zipfile.ZipFile


def acquire_elements(l: list[QOBJECT]) -> list[QOBJECT]:
# Many Pykrita functions return a `QList<QObject*>` where the objects are
# allocated for the caller. SIP does not handle this case and just leaks
# the objects outright. Fix this by taking explicit ownership of the objects.
# Note: ONLY call this if you are confident that the Pykrita function
# allocates the list members!
for obj in l:
if obj is not None:
sip.transferback(obj)
return l
Loading