From 18bd525d00ee3ce16164900293ee6ea8c2204589 Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Wed, 11 Jul 2012 08:14:34 -0400 Subject: MAINT-1175: Forbid LLRegistry[Singleton]. Back out code that selects LLTypeInfoLookup for the underlying map implementation when KEY = [const] std::type_info*, because LLTypeInfoLookup's API is changing to become incompatible with std::map. Instead, fail with STATIC_ASSERT when LLRegistry's KEY is [const] std::type_info*. Fix all existing uses to use std::type_info::name() string instead. --- indra/llui/lluictrlfactory.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'indra/llui/lluictrlfactory.h') diff --git a/indra/llui/lluictrlfactory.h b/indra/llui/lluictrlfactory.h index d612ad5005..1f7a8e08ce 100644 --- a/indra/llui/lluictrlfactory.h +++ b/indra/llui/lluictrlfactory.h @@ -69,9 +69,9 @@ protected: friend class LLSingleton; }; -// lookup widget name by type +// lookup widget name by type (actually by std::type_info::name()) class LLWidgetNameRegistry -: public LLRegistrySingleton +: public LLRegistrySingleton {}; // lookup function for generating empty param block by widget type -- cgit v1.2.3 From 709c1eeae90dae106800e3742f3655bd7b590b7b Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Wed, 11 Jul 2012 14:13:45 -0400 Subject: MAINT-1175: Properly pass LLRegistry's COMPARATOR to underlying map. Although LLRegistry and LLRegistrySingleton have always defined a COMPARATOR template parameter, it wasn't used for the underlying map. Therefore every type, including any pointer type, was being compared using std::less. This happens to work most of the time -- but is tripping us up now. Pass COMPARATOR to underlying std::map. Fix a couple minor bugs in LLRegistryDefaultComparator (never before used!). Specialize for const char*. Remove CompareTypeID and LLCompareTypeID because we now actively forbid using LLRegistry; remove only known reference (LLWidgetNameRegistry definition). --- indra/llui/lluictrlfactory.h | 11 +---------- 1 file changed, 1 insertion(+), 10 deletions(-) (limited to 'indra/llui/lluictrlfactory.h') diff --git a/indra/llui/lluictrlfactory.h b/indra/llui/lluictrlfactory.h index 1f7a8e08ce..a5fd83e555 100644 --- a/indra/llui/lluictrlfactory.h +++ b/indra/llui/lluictrlfactory.h @@ -34,15 +34,6 @@ class LLView; -// sort functor for typeid maps -struct LLCompareTypeID -{ - bool operator()(const std::type_info* lhs, const std::type_info* rhs) const - { - return lhs->before(*rhs); - } -}; - // lookup widget constructor funcs by widget name template class LLChildRegistry : public LLRegistrySingleton @@ -71,7 +62,7 @@ protected: // lookup widget name by type (actually by std::type_info::name()) class LLWidgetNameRegistry -: public LLRegistrySingleton +: public LLRegistrySingleton {}; // lookup function for generating empty param block by widget type -- cgit v1.2.3 From 7f609b6a6958f519bb1becb604132b583ada3fad Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Wed, 18 Jul 2012 15:51:34 -0400 Subject: Backed out changeset a25bfa87418d (using std::type_info::name()) The changeset above touched every consumer of the two LLRegistrySingletons originally defined with std::type_info* as keys. Those two LLRegistrySingletons were changed to use const char* as keys, then all consumers were changed to pass std::type_info::name() instead of the plain std::type_info* pointer -- to deal with the observed fact that on Linux, a given type might produce different std::type_info* pointers in different load modules. Since then, Richard turned up the fascinating fact that at least some implementations of gcc's std::type_info::before() method already accommodate this peculiarity. It seems worth backing out the (dismayingly pervasive) change to see if properly using std::type_info::before() as the map comparator will work just as well, with conceptually simpler source code. This backout is transitional: we don't expect things to build/run properly until we've cherry-picked certain other pertinent changes. --- indra/llui/lluictrlfactory.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'indra/llui/lluictrlfactory.h') diff --git a/indra/llui/lluictrlfactory.h b/indra/llui/lluictrlfactory.h index 1f7a8e08ce..d612ad5005 100644 --- a/indra/llui/lluictrlfactory.h +++ b/indra/llui/lluictrlfactory.h @@ -69,9 +69,9 @@ protected: friend class LLSingleton; }; -// lookup widget name by type (actually by std::type_info::name()) +// lookup widget name by type class LLWidgetNameRegistry -: public LLRegistrySingleton +: public LLRegistrySingleton {}; // lookup function for generating empty param block by widget type -- cgit v1.2.3 From 2e83dfa217feb90e7b94e499346ad9b98fa711b2 Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Wed, 18 Jul 2012 20:33:54 -0400 Subject: MAINT-1175: Ditch LLTypeInfoLookup, make map work. Instead of forbidding std::map outright (which includes LLRegistry and LLRegistrySingleton), try to make it work by specializing std::less to use std::type_info::before(). Make LLRegistryDefaultComparator use std::less so it can capitalize on that specialization. --- indra/llui/lluictrlfactory.h | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) (limited to 'indra/llui/lluictrlfactory.h') diff --git a/indra/llui/lluictrlfactory.h b/indra/llui/lluictrlfactory.h index ab16805cb1..4e54354731 100644 --- a/indra/llui/lluictrlfactory.h +++ b/indra/llui/lluictrlfactory.h @@ -31,6 +31,7 @@ #include "llinitparam.h" #include "llregistry.h" #include "llxuiparser.h" +#include "llstl.h" class LLView; @@ -62,14 +63,14 @@ protected: // lookup widget name by type class LLWidgetNameRegistry -: public LLRegistrySingleton +: public LLRegistrySingleton {}; // lookup function for generating empty param block by widget type // this is used for schema generation //typedef const LLInitParam::BaseBlock& (*empty_param_block_func_t)(); //class LLDefaultParamBlockRegistry -//: public LLRegistrySingleton +//: public LLRegistrySingleton //{}; extern LLFastTimer::DeclareTimer FTM_WIDGET_SETUP; -- cgit v1.2.3 From 3653727e7f84f10caefb6ea7dc33859455ebfa0b Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Wed, 10 Oct 2012 14:57:43 -0400 Subject: Introduce new LLDir::findSkinnedFilenames() method. Use as needed. In a number of different places, for different reasons, the viewer wants to load a UI-related file that might be overridden by a non-default skin; and within that skin, might further be overridden by a non-default language. Apparently, for each of those use cases, every individual developer approached it as an entirely new problem, solving it idiosyncratically for that one case. Not only is this a maintenance problem, but it rubs one's nose in the fact that most such solutions consider only a subset of the relevant skin directories. Richard and I evolved an API intended to address all such cases: a central LLDir method returning a list of relevant pathnames, from most general to most localized, filtered to present only existing files; plus a couple of convenience methods to specifically obtain the most general and most localized available file. There were several load-skinned-file methods (LLFloater::buildFromFile(), LLPanel::buildFromFile() and LLUICtrlFactory::createFromFile() -- apparently cloned-and-modified from each other) that contained funky bolted-on logic to output the loaded data to an optional passed LLXMLNodePtr param. The trouble is that passing that param forced each of these methods to subvert its normal search: specifically for that case, it needed to find the baseline XML file instead of the localized one. Richard agreed that for the intended usage (reformatting XML files) we should use XML schema instead, and that the hacky functionality should be removed. Remove it. Also remove LLUICtrlFactory::getLocalizedXMLNode(), only used for those three special cases. Some callers explicitly passed the optional LLXMLNodePtr param as NULL. Remove that. Remove LLFloaterUIPreview::displayFloater(save) param, which relied on the optional output LLXMLNodePtr param. Make onClickSaveFloater() and onClickSaveAll() emit popupAndPrintWarning() about discontinued functionality. Recast LLFloater::buildFromFile(), LLPanel::buildFromFile(), LLUICtrlFactory::createFromFile(), LLNotifications::loadTemplates(), LLUI::locateSkin(), LLFontRegistry::parseFontInfo(), LLUIColorTable::loadFromSettings(), LLUICtrlFactory::loadWidgetTemplate(), LLUICtrlFactory::getLayeredXMLNode(), LLUIImageList::initFromFile(), LLAppViewer::launchUpdater() and LLMediaCtrl::navigateToLocalPage() to use findSkinnedFilenames(). (Is LLAppViewer::launchUpdater() ever called any more? Apparently so -- though the linux-updater.bin logic to process the relevant command-line switch has been disabled. Shrug.) (Is LLMediaCtrl::navigateToLocalPage() ever used?? If so, why?) Remove LLUI::setupPaths(), getXUIPaths(), getSkinPath() and getLocalizedSkinPath(). Remove the skins/paths.xml file read by setupPaths(). The only configuration it contained was the pair of partial paths "xui/en" and "xui/[LANGUAGE]" -- hardly likely to change. getSkinPath() specifically returned the first of these, while getLocalizedSkinPath() specifically returned the second. This knowledge is now embedded in findSkinnedFilenames(). Also remove paths.xml from viewer_manifest.py. Remove injected xui_paths from LLFontGL::initClass() and LLFontRegistry::LLFontRegistry(). These are no longer needed since LLFontRegistry can now directly consult LLDir for its path search. Stop passing LLUI::getXUIPaths() to LLFontGL::initClass() in LLViewerWindow's constructor and initFonts() method. Add LLDir::append() and add() methods for the simple task of combining two path components separated by getDirDelimiter() -- but only if they're both non-empty. Amazing how often that logic is replicated. Replace some existing concatenations with add() or append(). New LLDir::findSkinnedFilenames() method must know current language. Allow injecting current language by adding an LLDir::setSkinFolder(language) param, and pass it where LLAppViewer::init() and initConfiguration() currently call setSkinFolder(). Also add LLDir::getSkinFolder() and getLanguage() methods. Change LLFLoaterUIPreview's LLLocalizationResetForcer helper to "forcibly reset language" using LLDir::setSkinFolder() instead of LLUI::setupPaths(). Update LLDir stubs in lldir_stub.cpp and llupdaterservice_test.cpp. Add LLDir::getUserDefaultSkinDir() to obtain often-overlooked possible skin directory -- like getUserSkinDir() but with "default" in place of the current skin name as the last path component. (However, we hope findSkinnedFilenames() obviates most explicit use of such individual skin directory pathnames.) Add LLDir unit tests for new findSkinnedFilenames() and add() methods -- the latter exercises append() as well. Tweak indra/integration_tests/llui_libtest/llui_libtest.cpp for all the above. Notably, comment out its export_test_floaters() function, since the essential LLFloater::buildFromFile(optional LLXMLNodePtr) functionality has been removed. This may mean that llui_libtest.cpp has little remaining value, not sure. --- indra/llui/lluictrlfactory.h | 17 +++-------------- 1 file changed, 3 insertions(+), 14 deletions(-) (limited to 'indra/llui/lluictrlfactory.h') diff --git a/indra/llui/lluictrlfactory.h b/indra/llui/lluictrlfactory.h index d612ad5005..56e5f3eb7b 100644 --- a/indra/llui/lluictrlfactory.h +++ b/indra/llui/lluictrlfactory.h @@ -169,7 +169,7 @@ public: LLView* createFromXML(LLXMLNodePtr node, LLView* parent, const std::string& filename, const widget_registry_t&, LLXMLNodePtr output_node ); template - static T* createFromFile(const std::string &filename, LLView *parent, const widget_registry_t& registry, LLXMLNodePtr output_node = NULL) + static T* createFromFile(const std::string &filename, LLView *parent, const widget_registry_t& registry) { T* widget = NULL; @@ -178,23 +178,13 @@ public: { LLXMLNodePtr root_node; - //if exporting, only load the language being exported, - //instead of layering localized version on top of english - if (output_node) - { - if (!LLUICtrlFactory::getLocalizedXMLNode(filename, root_node)) - { - llwarns << "Couldn't parse XUI file: " << filename << llendl; - goto fail; - } - } - else if (!LLUICtrlFactory::getLayeredXMLNode(filename, root_node)) + if (!LLUICtrlFactory::getLayeredXMLNode(filename, root_node)) { llwarns << "Couldn't parse XUI file: " << skinned_filename << llendl; goto fail; } - LLView* view = getInstance()->createFromXML(root_node, parent, filename, registry, output_node); + LLView* view = getInstance()->createFromXML(root_node, parent, filename, registry, NULL); if (view) { widget = dynamic_cast(view); @@ -223,7 +213,6 @@ fail: static void createChildren(LLView* viewp, LLXMLNodePtr node, const widget_registry_t&, LLXMLNodePtr output_node = NULL); static bool getLayeredXMLNode(const std::string &filename, LLXMLNodePtr& root); - static bool getLocalizedXMLNode(const std::string &xui_filename, LLXMLNodePtr& root); private: //NOTE: both friend declarations are necessary to keep both gcc and msvc happy -- cgit v1.2.3 From 6ca37068080e3b26e5cb163dfd874bc0e2f4c837 Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Fri, 12 Oct 2012 08:42:18 -0400 Subject: LLUICtrlFactory::getLayeredXMLNode() gets LLDir::ESkinConstraint. At this point, LLUICtrlFactory::getLayeredXMLNode() is a pretty thin wrapper around LLDir::findSkinnedFilenames() and LLXMLNode::getLayeredXMLNode(). Until now, LLUICtrlFactory::getLayeredXMLNode() passed (by default) LLDir::CURRENT_SKIN to LLDir::findSkinnedFilenames(). But that meant that a caller such as LLTransUtil::parseStrings() that wants almost the same functionality, but with LLDir::ALL_SKINS instead, had to clone the logic from LLUICtrlFactory::getLayeredXMLNode(). Allowing its caller to pass the desired LLDir::ESkinConstraint enum value eliminates the need to clone its logic. Remove cloned logic from LLTransUtil::parseStrings(). --- indra/llui/lluictrlfactory.h | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) (limited to 'indra/llui/lluictrlfactory.h') diff --git a/indra/llui/lluictrlfactory.h b/indra/llui/lluictrlfactory.h index 56e5f3eb7b..1acfd24112 100644 --- a/indra/llui/lluictrlfactory.h +++ b/indra/llui/lluictrlfactory.h @@ -31,6 +31,7 @@ #include "llinitparam.h" #include "llregistry.h" #include "llxuiparser.h" +#include "lldir.h" class LLView; @@ -212,7 +213,8 @@ fail: static void createChildren(LLView* viewp, LLXMLNodePtr node, const widget_registry_t&, LLXMLNodePtr output_node = NULL); - static bool getLayeredXMLNode(const std::string &filename, LLXMLNodePtr& root); + static bool getLayeredXMLNode(const std::string &filename, LLXMLNodePtr& root, + LLDir::ESkinConstraint constraint=LLDir::CURRENT_SKIN); private: //NOTE: both friend declarations are necessary to keep both gcc and msvc happy -- cgit v1.2.3 From bf99126a37062d80e7887e2634b34f1da0ed309e Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Mon, 15 Oct 2012 09:38:28 -0400 Subject: Remove LLUICtrlFactory::findSkinnedFilename(): cf. LLDir method. Richard points out that LLUICtrlFactory::findSkinnedFilename() adds little value. It was called from exactly one place, and that one place could easily obtain the information another way. The concern is that it could confuse a reader of the code with regard to the other findSkinnedFilename[s]() methods in LLDir. Clarifying the code base is a Good Thing. Removing. --- indra/llui/lluictrlfactory.h | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) (limited to 'indra/llui/lluictrlfactory.h') diff --git a/indra/llui/lluictrlfactory.h b/indra/llui/lluictrlfactory.h index 1acfd24112..ca6ad254b1 100644 --- a/indra/llui/lluictrlfactory.h +++ b/indra/llui/lluictrlfactory.h @@ -173,18 +173,17 @@ public: static T* createFromFile(const std::string &filename, LLView *parent, const widget_registry_t& registry) { T* widget = NULL; - - std::string skinned_filename = findSkinnedFilename(filename); + instance().pushFileName(filename); { LLXMLNodePtr root_node; if (!LLUICtrlFactory::getLayeredXMLNode(filename, root_node)) { - llwarns << "Couldn't parse XUI file: " << skinned_filename << llendl; + llwarns << "Couldn't parse XUI file: " << instance().getCurFileName() << llendl; goto fail; } - + LLView* view = getInstance()->createFromXML(root_node, parent, filename, registry, NULL); if (view) { @@ -299,9 +298,6 @@ private: // this exists to get around dependency on llview static void setCtrlParent(LLView* view, LLView* parent, S32 tab_group); - // Avoid directly using LLUI and LLDir in the template code - static std::string findSkinnedFilename(const std::string& filename); - class LLPanel* mDummyPanel; std::vector mFileNames; }; -- cgit v1.2.3