[prev in list] [next in list] [prev in thread] [next in thread] 

List:       pykde
Subject:    Re: [PyQt] QIconEngine ownership regression in PyQt5 5.12
From:       Phil Thompson <phil () riverbankcomputing ! com>
Date:       2019-03-19 14:20:19
Message-ID: 00C2850E-3B09-4A5B-BE0F-7EE2C5F86260 () riverbankcomputing ! com
[Download RAW message or body]

On 19 Mar 2019, at 12:35 pm, Ales Erjavec <ales.erjavec324@gmail.com> wrote:
> 
> Maybe the ownership could be transferred to the C++ (sip)QIconEngine itself?

That's what the previous buggy code actually did.

> If I construct the QIcon with:
> 
> ## code
> 
> def qicon_transfer(engine):
> # type: (QIconEngine) -> QIcon
> import sip
> icon = QIcon(engine)
> sip.transferto(engine, engine)
> return icon
> 
> ## /code

sip.transferto(engine, None) is better.

> Then the engine is preserved until the last icon is deleted.

That's because QIconEngine has a virtual dtor which is used to detect when C++ \
invokes it.

> ## code
> 
> def test_icon_transfer():
> from weakref import ref
> eng = IconEngine("a", QColor("red"))
> engref = ref(eng)
> 
> icon = qicon_transfer(eng)
> 
> del eng
> assert engref() is not None
> icon1 = QIcon(icon)
> 
> del icon
> assert engref() is not None
> icon2 = QIcon(icon1)
> 
> del icon1
> assert engref() is not None
> 
> del icon2
> assert engref() is None
> 
> ## /code
> 
> There should be no issue with the transfer back because there is no
> way to get the engine out of the QIcon.
> 
> I think this preserves the Qt's design. QIcon has value type semantics
> and practically all API's that accept a QIcon do so by value (copy construct).

It is very unusual (I can't think of another case) where a class with value type \
semantics can take ownership of another object. sip does not have explicit support \
for this pattern so I'll add handwritten code to do the right thing.

Thanks,
Phil
_______________________________________________
PyQt mailing list    PyQt@riverbankcomputing.com
https://www.riverbankcomputing.com/mailman/listinfo/pyqt


[prev in list] [next in list] [prev in thread] [next in thread] 

Configure | About | News | Add a list | Sponsored by KoreLogic