[Marble-devel] Review Request: Fix to Python bindings for MarbleWidget::addLayer() and MarbleWidget::removeLayer()

[Commit Hook]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/104655/#review22724
-----------------------------------------------------------


This review has been submitted with commit b41a90bc753d0494768252f21ec64b71daa4a97a by Jason Roehm to branch master.

- Commit Hook


On April 18, 2012, 2:15 p.m., Jason Roehm wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/104655/
> -----------------------------------------------------------
> 
> (Updated April 18, 2012, 2:15 p.m.)
> 
> 
> Review request for Marble.
> 
> 
> Description
> -------
> 
> MarbleWidget::addLayer() takes a pointer to a LayerInterface-derived object as an argument. The pointer is added to an internal list of layers that are rendered in the Marble widget. When exposing this function via Python, the /Transfer/ SIP parameter annotation must be supplied. This notation indicates to SIP that control of the lifetime of the object passed via addLayer() is placed in the C++ library's hands. If you don't do this, then Python might decide to garbage-collect the layer object when it's no longer used on that side. That is obviously a bad thing if the layer is still being used by the widget.
> 
> Likewise, the /TransferBack/ annotation is used to return ownership of the object back to Python if the removeLayer() function is later called to remove the layer from the widget. At that time, the object can safely be garbage-collected since it is no longer used by the widget C++ library.
> 
> 
> Diffs
> -----
> 
>   src/bindings/python/sip/MarbleWidget.sip 9f96274 
> 
> Diff: http://git.reviewboard.kde.org/r/104655/diff/
> 
> 
> Testing
> -------
> 
> Without these annotations, adding an instance of a Python subclass of LayerInterface via addLayer() yielded a crash (at least on my system) the first time the C++ library tried to invoke any member functions on the layer. This is consistent with the layer having been destroyed before the C++ library could use it. After adding the annotations, I am able to add Python layer classes and have them render properly. The below test can be used; it dumps to stdout when any of the member functions are called, and draws some text at 40 deg N, 80 deg W that should be visible.
> 
> #!/usr/bin/python
> 
> # Import the PyQt modules that we need.
> from PyQt4.QtCore import *
> from PyQt4.QtGui import *
> # Import the Marble widget library.
> from PyKDE4.marble import *
> # Import other modules that we need.
> import os
> import sys
> 
> class OurLayer(Marble.LayerInterface):
>     def __init__(self):
>         super(OurLayer, self).__init__()
> 
>     def renderPosition(self):
>         print "renderposition"
>         return QStringList("SURFACE")
> 
>     def render(self, painter, viewport, renderPos, layer):
>         print "render"
>         painter.setRenderHint(QPainter.Antialiasing, True)
>         painter.setPen(QPen(QBrush(QColor.fromRgb(255, 255, 255, 200)), 3.0, Qt.SolidLine, Qt.RoundCap))
>         painter.drawText(Marble.GeoDataCoordinates(-80, 40, 0, Marble.GeoDataCoordinates.Degree), "This layer works!")
>         return True
> 
>     def zValue(self):
>         print "zvalue"
>         return 0
> 
> # Main function that runs when we execute the script.
> def main():
>     # Create the Qt application object.
>     app = QApplication(sys.argv)
> 
>     # Instantiate the Marble widget.
>     marble = Marble.MarbleWidget()
>     marble.setMapThemeId("earth/bluemarble/bluemarble.dgml")
>     marble.addLayer(OurLayer())
>     marble.show()
>     
>     # Execute the application.
>     sys.exit(app.exec_())
> 
> if __name__ == "__main__":
>     main()
> 
> 
> Thanks,
> 
> Jason Roehm
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/marble-devel/attachments/20121128/1fdbbd2b/attachment.html>