Addongui: expose GUIRenderingControl #1816

Merged
merged 8 commits into from Mar 7, 2013

Conversation

Projects
None yet
4 participants
Member

FernetMenta commented Nov 20, 2012

AFAIK the addon gui lib has no clients so far (504e182). I want to use it for pvr client VNSI, displaying the VDR OSD in the settings dialog.

Member

opdenkamp commented Nov 21, 2012

i can only confirm that it's not used by any client i know (i don't know guilib code well)

Member

FernetMenta commented Jan 28, 2013

@opdenkamp this mainly affects pvr and pvr addons need an update of addongui headers. As far as I can see the code came with merging of pvr. Who should I poke for review?

Member

elupus commented Jan 28, 2013

Couldn't this have been added by skin xml? Or maybe I'm missing something.

Member

FernetMenta commented Jan 28, 2013

This change makes it possible to add the GUIRenderingControl to skin xml used by an addon. Currently an addon only has access to spin, button, radio, and label.

Member

jmarshallnz commented Jan 28, 2013

Are you doing the rendering with DirectX or OpenGL inside the add-on?

Overall the guilib side of it looks fine, other than the minors on the commits.

Member

FernetMenta commented Jan 29, 2013

Are you doing the rendering with DirectX or OpenGL inside the add-on?

I have both versions and tested with DX on Windows and OpenGL on Linux:
https://github.com/FernetMenta/xbmc-pvr-addons/blob/osd/addons/pvr.vdr.vnsi/src/VNSIAdmin.cpp

I will rework the pr according to your comments.

FernetMenta referenced this pull request in opdenkamp/xbmc-pvr-addons Jan 29, 2013

Closed

Addongui: sync lib with XBMC #162

Member

jmarshallnz commented Jan 31, 2013

guilib changes signed off.

@elupus elupus commented on the diff Feb 10, 2013

xbmc/addons/AddonCallbacksGUI.cpp
@@ -1246,13 +1307,11 @@ void CAddonCallbacksGUI::ListItem_SetPath(void *addonData, GUIHANDLE handle, con
bool CGUIAddonWindow::OnAction(const CAction &action)
{
- // do the base class window first, and the call to python after this
- bool ret = CGUIWindow::OnAction(action); // we don't currently want the mediawindow actions here
- if (CBOnAction)
- {
- CBOnAction(m_clientHandle, action.GetID());
- }
- return ret;
+ // Let addon decide whether it wants to hande action first
+ if (CBOnAction && CBOnAction(m_clientHandle, action.GetID()))
+ return true;
@elupus

elupus Feb 10, 2013

Member

This changes behavior quite a lot. It could potentially break addons. I'm for the change, but it could breaks tuff.

@FernetMenta

FernetMenta Feb 10, 2013

Member

This is why @jmarshallnz wanted to have it in a separate commit. The question is are there any addons using this?

@elupus elupus commented on the diff Feb 10, 2013

xbmc/addons/AddonCallbacksGUI.cpp
+{
+ if (CBStop)
+ {
+ CBStop(m_clientHandle);
+ }
+ m_refCount--;
+ if (m_refCount <= 0)
+ delete this;
+}
+
+void CGUIAddonRenderingControl::Delete()
+{
+ m_refCount--;
+ if (m_refCount <= 0)
+ delete this;
+}
@elupus

elupus Feb 10, 2013

Member

What happen if you Create() but never call stop? Won't it retain a refcount and thus never get destroyed?

@FernetMenta

FernetMenta Feb 10, 2013

Member

The addon calls the constructor and Delete. guilib internally may call CGUIRenderingControl::FreeResources which call stop here. The ref count keeps the object alive until the addon delete it.
Yes, if the addon does not release, there's a mem leak. But the same applies for AddonWindow.

Member

FernetMenta commented Mar 1, 2013

@opdenkamp as discussed on irc, I have added a version string to the lib which is checked by pvr.

opdenkamp was assigned Mar 6, 2013

opdenkamp merged commit 140a433 into xbmc:master Mar 7, 2013

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment