[pieh]

It'd be good if you could list the times in ms (even just for the "fastest" and "slowest") so we know the relative ones.

I almost finished splitting code in CGUIControlFactory::Create - I have some minor glitches and need to fix them, then I'll present table that compares old method and method with selective element parsing.

Just a note - timing values are only useful while referencing other values that were produced in same condition - as I ran my test in debug mode values itself are enormous (here are results of my initial test if any one is interested): Incl. = time needed to resolve include

Code:

[b]Skin        Incl.    Control Creation[/b]
Night        9100    2550
Aeon65        8550    3750
Alaska Rev    1870    1550
Back-row    4200    2700
Confluence     500     750
Elipsis        2120    3725
Foundation      14      32
Shade        8400    2133
SLik         200     185

When I finish splitting maybe I'll build release version so values would be more reliable). This is how I compare them:

Code:

CGUIControl* CGUIControlFactory::Create(int parentID, const CRect &rect, TiXmlElement* pControlNode, bool insideContainer)
{
  g_statsCatcher.Start();
  CGUIControl *ctrl2 = Create_new(parentID, rect, pControlNode, insideContainer);
  g_statsCatcher.Control_New();

  delete ctrl2;

  g_statsCatcher.Start();
  CGUIControl *control = Create_old(parentID, rect, pControlNode, insideContainer);
  g_statsCatcher.Control_Old();

  return control;
}

statsCatcher take care of storing what skin is used, what window is being initialised and of course values of timing.

Initial results are quite promising: Create_new seems to be 2-5 times faster than Create_old.

Could you ad PM3.HD to tested skins. I think it's best performing one.

I'll check it, but purpose of my tests are to catch slowest skins and see what can be done to load those skins faster - Confluence is really fast and it's good reference point for other skins.

[pecinko]

I'll check it, but purpose of my tests are to catch slowest skins and see what can be done to load those skins faster - Confluence is really fast and it's good reference point for other skins.

Just for info - PM3.HD is the only skin performing smoothly on ATV2.

[jmarshall]

I think you'll find Release builds will bring down the range of possible values significantly - unlikely to reorder them those.

[pieh]

I think you'll find Release builds will bring down the range of possible values significantly - unlikely to reorder them those.

Well, I don't expect anything else - but it's really time consuming to do 15 tests on every skin when loading window take so long. Proportion will be preserved I think. That's what I meant by "timing values are only useful while referencing other values that were produced in same condition".

[pecinko]

I was discussing this in another thread, but probably didn't explain it very well:

When coding Videos, you have a lot of conditional checks Files vs Folders vs Library. Library root view vs Movies vs TV Shows vs Episodes. Banners vs Posters vs Auto generated thumbs.

I thought it could be solved by separation of MyMoviesNav to a few xmls - TV Shows, Episodes, Movies.xml. Maybe a better solution can be found.

[jmarshall]

A better solution is so you don't need said checks all over the show. See my post on Container.Content as a starting point.

[Loto_Bak]

Always great to see discussion of GUI improvements.

From an end user perspective
I find the slowest things in XBMC for me these days are db queries for big listings.
Things such as 'artists' and 'songs' in musicdb are very slow and makes navigation of these branches a very unfriendly experience.
(Granted I do have a very large db, around 80,000 songs)
Perhaps there anyway to cache these listings the first time to speed up load times when both navigating forward/back and changing windows?

[pieh]

On my git branch https://github.com/pieh/xbmc/commits/xml..._splitting, I did initial commit to parse only needed xml tags for control creation

Overview:

• All controls have some common attributes - we read them always.
  
• Focusable controls have some additional attributes - we read them only for focusable controls.
  
• For additional attributes we check type of control and read only those attributes that are used by given control type
  
  
• Additional helper methods:
  • bool IsFocusable(CGUIControl::GUICONTROLTYPES type);
    
  • void GetLabelInfo(const TiXmlNode* pControlNode, CLabelInfo &labelInfo); // read text/font attributes (font, colors, angle, alignment and textwidth) - this can be further improved by adding flag to look only for needed text attributes (i.e. we don't need to read focusedcolor for Label Control)
    
  • void GetOrientation(const TiXmlNode *pControlNode, const char *tag, ORIENTATION &orientation);
    

I will gather statistics to compare performance of this method and old one soon.

[vanMiez]

5. Python modules:

playlist:
allow callbacks for onNextItem, onPrevItem and allow overriding GetNextItem, etc

would make posible to create playlist script for advanced playlist operations: such as "play after current", smart shuffle/random - random could be based on inner rating if user skips song it decrease inner rating of the song - again this could be used just by 5% of users

If you could make this happen by helping the devs with patches i would be so - what is the increase of grateful?-

that is definitely something i am waiting for and i think all people out there who aren't just watching movies and tv-shows on their htpc.
please

[pieh]

If you could make this happen by helping the devs with patches i would be so - what is the increase of grateful?-

that is definitely something i am waiting for and i think all people out there who aren't just watching movies and tv-shows on their htpc.
please

Before patches thinking / discussion "how it should work" must be made

I think you'll find Release builds will bring down the range of possible values significantly - unlikely to reorder them those.

Ok, did some stats - on Release Build it seems that "selective xml parsing" gave around 50% boost (instead of average of 650ms Aeon had 400ms time of control creation). Boost on light skins was hardly noted (Convluence: average 23ms vs 27ms).

Im now in process of moving xml parsing to constructors.

Question:
There seems to be unused Control feature (it's defined in base class CGUIControl):

Code:

virtual void OnNextControl();
virtual void OnPrevControl();

these are triggered via action:

Code:

#define ACTION_NEXT_CONTROL           181
#define ACTION_PREV_CONTROL           182

These methods make use of parsed <onprev>, <onnext> values to determine ID of control to be selected.

It seems it was meant to add functionality of changing focused control to next/previous one (like a Tab/shift+Tab key in some GUIs), but except of Key.h and GuiControl.h/cpp those aren't used anywhere and therefore they aren't available for users. What should be done with this? Should it be removed or added to ButtonTranslator?

[jmarshall]

The code is implemented and works. It's just that it wasn't advertised so no skin supports it. It was implemented by the boxee guys.

I'm not sure if it makes sense for most skins to have it or not. It seems to me to be something else that the skinner needs to think about which isn't really a good thing.

I say remove it - obviously in a separate commit

Cheers,
Jonathan

[pieh]

Maybe we should ask skinners if they would make use of this feature (it's already working - almost - we need just add string->action translation and update keymaps - for keyboard something non invasive like shift + left/right arrow)?

I share Your point of view and this could bring more mess (and complaints about it) than good, but maybe some skinner will figure out some good way of using this. After all, "boxee guys" had to have something on their mind when implementing it.

----

I have also some technical questions about making constructors that get xml as paramter: some of the control classes have members that don't have default ctors.

Example: CGUILabel CGUILabelControl::m_label - CGUILabel doesn't have default ctor and therefore I must init it in ctor's initialization list - but can't really do it as im yet to parse given xml to extract relevant data. Now I'm simply adding empty default constructors to such classes.

I'm just wondering what's the best way here. Other option I see: convert those members to smart pointers and init them whenever we like - lot of changes would have to be done with this way.

[jmarshall]

Default constructors seem like a reasonable bet. The other option is to pass the appropriate XML node along I guess (which could be NULL).

[pieh]

Always great to see discussion of GUI improvements.

From an end user perspective
I find the slowest things in XBMC for me these days are db queries for big listings.
Things such as 'artists' and 'songs' in musicdb are very slow and makes navigation of these branches a very unfriendly experience.
(Granted I do have a very large db, around 80,000 songs)
Perhaps there anyway to cache these listings the first time to speed up load times when both navigating forward/back and changing windows?

I was thinking about it and have idea to evaluate:

AFAIK currently window loading is synchronised (1 threaded). It means that first we have to load Window (read xml, create controls), then xbmc start getting content for that window. What if getting content (list of movies, etc) would be moved to background thread (CJob) that would be started before loading window and results would be submited in portions: items would show up in list as they are read from database (CJobManager::OnJobProgress). This could apply to WindowVideoNav and WindowMusicNav.

---

Default constructors seem like a reasonable bet. The other option is to pass the appropriate XML node along I guess (which could be NULL).

what's better approach in this case - object created with dummy values - simply create proper object and assign it's values to out object (red color) or use Setters to set correct values (blue color)?

I also wanted to show how control ctors will look like and ask if there are any suggestions about it before I move on with doing rest of controls.

Code:

CGUIImage::CGUIImage(int parentID, const CRect &rect, TiXmlElement* pControlNode)
  : CGUIControl(parentID, rect, pControlNode)
  , [b]m_texture(0 ,0, 0, 0, CTextureInfo())[/b] <- creating object with dummy values
{
  m_crossFadeTime = 0;
  m_currentFadeTime = 0;
  m_lastRenderTime = 0;
  ControlType = GUICONTROL_IMAGE;
  m_bDynamicResourceAlloc=false;

  CTextureInfo textureTmp;
  CGUIControlFactory::GetInfoTexture(pControlNode, "texture", textureTmp, m_info);

  if (pControlNode->Attribute("type") == "largeimage")
    textureTmp.useLarge = true;

  [color=#FF0000]//m_texture = CGUITexture(m_posX, m_posY, m_width, m_height, textureTmp);[/color]
  [color=#0000FF]m_texture.SetTextureInfo(textureTmp); // I've added this method
  m_texture.SetPosition(m_posX, m_posY);
  m_texture.SetWidth(m_width);
  m_texture.SetHeight(m_height);[/color]

  if (m_info.IsConstant())
    m_texture.SetFileName(m_info.GetLabel(0));

  CAspectRatio aspectRatioTmp;
  CGUIControlFactory::GetAspectRatio(pControlNode, "aspectratio", aspectRatioTmp);
  m_texture.SetAspectRatio(aspectRatioTmp);

  XMLUtils::GetUInt(pControlNode,"fadetime", m_crossFadeTime);
  if (!m_crossFadeTime && m_texture.IsLazyLoaded() && !m_info.GetFallback().IsEmpty())
    m_crossFadeTime = 1;
}

[pieh]

in meantime I was checking my initial idea - that is:

CGUIWindow::Load could save copy of loaded window to (let's call it) SavedWindows, next time it would just create copy of stored window instead of reading and parsing same .xml again.

ReloadSkin would clear SavedWindows. Feature could be toggled in AdvancedSettings

and, well - it's all there already:

Code:

case GUI_MSG_WINDOW_INIT:
    {
      CLog::Log(LOGDEBUG, "------ Window Init (%s) ------", GetProperty("xmlfile").c_str());
      if (m_dynamicResourceAlloc || !m_bAllocated) AllocResources();
      OnInitWindow();
      return true;
    }
    break;

  case GUI_MSG_WINDOW_DEINIT:
    {
      CLog::Log(LOGDEBUG, "------ Window Deinit (%s) ------", GetProperty("xmlfile").c_str());
      OnDeinitWindow(message.GetParam1());
      // now free the window
      if (m_dynamicResourceAlloc) FreeResources();
      return true;
    }

after disabling m_dynamicResourceAlloc it seemed to be working just like that - ofcourse there should to be added some additional checking if window needs to be reloaded (like that map<conditionsid,boolvalue> of includes), but apart from it - any reason why not to check it further?