From db5846c1456de27be8a572f1262d944c12c1f54c Mon Sep 17 00:00:00 2001 From: Aleric Inglewood Date: Thu, 7 Feb 2013 17:42:54 +0100 Subject: [PATCH 01/16] Added documentation on LLHTTPClient Responders. --- doc/responders.txt | 82 ++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 82 insertions(+) create mode 100644 doc/responders.txt diff --git a/doc/responders.txt b/doc/responders.txt new file mode 100644 index 000000000..b1bf31f50 --- /dev/null +++ b/doc/responders.txt @@ -0,0 +1,82 @@ +All Responders are derived from ResponderBase, however you normally do never derived from that directly yourself. +Instead, Responder classes are derived from one of: + +1. Responder base classes + + ResponderHeadersOnly -- Derived classes are used with HTTPClient::head or HTTPClient::getHeaderOnly. + ResponderWithCompleted -- Derived classes implement completed(U32, std::string const&, LLSD const&), + or completedRaw(U32, std::string const&, LLChannelDescriptors const&, buffer_ptr_t const&) + if the response is not (always) LLSD. + ResponderWithResult -- Derived classes implement result(LLSD const&) and optionally + errorWithContent(U32, std::string const&, LLSD const&) OR error(U32, std::string const&). + +2. Special base classes + + ResponderIgnoreBody -- Same as ResponderWithResult but already implements result() that ignored the body. + LLAssetUploadResponder -- Derived from ResponderWithResult. Base class for responders that upload assets via capabilities. + LegacyPolledResponder -- Used for old code that needs polling (do not use). + + +There is one non-base class Responder with a more general purpose: + +3. Special purpose responders: + + ResponderIgnore -- Derived from ResponderIgnoreBody. Used for "fire and forget" requests as it ignores any response. + + +4. Signatures. + +Every final (derived) responder class must implement 'getName(void) const' and 'getHTTPTimeoutPolicy(void)', +except the base classes (this is to alert the developer they have to implement getName as it is pure virtual). + +For example: + + extern AIHTTPTimeoutPolicy myResponder_timeout; // Add 'P(myResponder)' to indra/llmessage/aihttptimeoutpolicy.cpp. + + class MyResponder : public LLHTTPClient::SomeResponderBaseClass { + ... + /*virtual*/ AIHTTPTimeoutPolicy const& getHTTPTimeoutPolicy(void) const { return myResponder_timeout; } + /*virtual*/ char const* getName(void) const { return "MyResponder"; } + }; + +Note the convention that the name of a AIHTTPTimeoutPolicy (what goes between the brackets of P()) is +the class name minus any 'AI' or 'LL' prefix and starting with a lowercase character. + +Then, depending on the three main base classes that was derived from, the signatures should be: + + class MyResponder1 : public LLHTTPClient::ResponderHeadersOnly { + /*virtual*/ void completedHeaders(U32 status, std::string const& reason, AIHTTPReceivedHeaders const& headers); + // See for example PostImageResponder + ... + }; + + class MyResponder2 : public LLHTTPClient::ResponderWithCompleted { + /*virtual*/ void completedRaw(U32 status, std::string const& reason, LLChannelDescriptors const& channels, buffer_ptr_t const& buffer); + // See for example PostImageRedirectResponder + >>>OR<<< + + /*virtual*/ void completed(U32 status, const std::string& reason, const LLSD& content); // See for example LLImportPostResponder + + }; + + class MyResponder3 : public LLHTTPClient::ResponderWithResult { + /*virtual*/ void result(const LLSD& content); // See for example LLInventoryModelFetchItemResponder + /*virtual*/ void error(U32 status, const std::string& reason); + >>>OR instead error()<<< + /*virtual*/ void errorWithContent(U32 status, const std::string& reason, const LLSD& content); + // See for example LLSDMessage::EventResponder + }; + +Finally, if a responder derived from ResponderWithCompleted or ResponderWithResult needs to process +individual headers, you need to override 'needsHeaders': + + /*virtual*/ bool needsHeaders(void) const { return true; } // See for example LLWebProfileResponders::PostImageResponder + // which will cause this to be called: + /*virtual*/ void completedHeaders(U32 status, std::string const& reason, AIHTTPReceivedHeaders const& headers); + +And if it needs redirection to work (you'll get an assert if you forget this and it is being redirected): + + /*virtual*/ bool followRedir(void) const { return true; } // See for example LLWebProfileResponders::ConfigResponder + +This is not necessary for ResponderHeadersOnly because that already defines both. + From adf4c9a0ce29e8685d298e49c22a8e12d038c46e Mon Sep 17 00:00:00 2001 From: Aleric Inglewood Date: Mon, 4 Feb 2013 22:41:46 +0100 Subject: [PATCH 02/16] Make the type of the read/write lock of AIThreadSafe a template parameter. This allows passing a different type than the default AIRWLock for debugging purposes. The signature of the class used for RWLOCK should be: struct RWLock { // Default constructor. RWLock(void); // Block until it's safe to read the data. // high_priority is a hint that granting this thread the read lock is more important than granting another thread a write lock. void rdlock(bool high_priority = false); // Release the obtained read lock. void rdunlock(); // Block until it's safe to write to the data. void wrlock(); // Release the obtained write lock. void wrunlock(); // Convert the obtained write lock into a read lock. void wr2rdlock(); // Block until it is possible to convert the obtained read lock into a write lock. void rd2wrlock(); // Return true when a read or write is obtained and subsequent calls to release the lock are expected. bool isLocked() const; }; --- indra/llcommon/aithreadsafe.h | 66 +++++++++++++++++------------------ 1 file changed, 33 insertions(+), 33 deletions(-) diff --git a/indra/llcommon/aithreadsafe.h b/indra/llcommon/aithreadsafe.h index 425741601..461bb4cc5 100644 --- a/indra/llcommon/aithreadsafe.h +++ b/indra/llcommon/aithreadsafe.h @@ -106,9 +106,9 @@ // on a compiler that doesn't need this hack. #define AI_NEED_ACCESS_CC (defined(__GNUC__) && (((__GNUC__ == 4) && (__GNUC_MINOR__ < 3)) || (__GNUC__ < 4))) -template struct AIReadAccessConst; -template struct AIReadAccess; -template struct AIWriteAccess; +template struct AIReadAccessConst; +template struct AIReadAccess; +template struct AIWriteAccess; template struct AIAccessConst; template struct AIAccess; template struct AISTAccessConst; @@ -225,17 +225,17 @@ protected: * * */ -template +template class AIThreadSafe : public AIThreadSafeBits { protected: // Only these may access the object (through ptr()). - friend struct AIReadAccessConst; - friend struct AIReadAccess; - friend struct AIWriteAccess; + friend struct AIReadAccessConst; + friend struct AIReadAccess; + friend struct AIWriteAccess; // Locking control. - AIRWLock mRWLock; + RWLOCK mRWLock; // For use by AIThreadSafeDC AIThreadSafe(void) { } @@ -310,20 +310,20 @@ public: * * which is not possible with AITHREADSAFE. */ -template -class AIThreadSafeDC : public AIThreadSafe +template +class AIThreadSafeDC : public AIThreadSafe { public: // Construct a wrapper around a default constructed object. - AIThreadSafeDC(void) { new (AIThreadSafe::ptr()) T; } + AIThreadSafeDC(void) { new (AIThreadSafe::ptr()) T; } // Allow an arbitrary parameter to be passed for construction. - template AIThreadSafeDC(T2 const& val) { new (AIThreadSafe::ptr()) T(val); } + template AIThreadSafeDC(T2 const& val) { new (AIThreadSafe::ptr()) T(val); } }; /** * @brief Read lock object and provide read access. */ -template +template struct AIReadAccessConst { //! Internal enum for the lock-type of the AI*Access object. @@ -336,8 +336,8 @@ struct AIReadAccessConst }; //! Construct a AIReadAccessConst from a constant AIThreadSafe. - AIReadAccessConst(AIThreadSafe const& wrapper, bool high_priority = false) - : mWrapper(const_cast&>(wrapper)), + AIReadAccessConst(AIThreadSafe const& wrapper, bool high_priority = false) + : mWrapper(const_cast&>(wrapper)), mState(readlocked) #if AI_NEED_ACCESS_CC , mIsCopyConstructed(false) @@ -369,14 +369,14 @@ struct AIReadAccessConst protected: //! Constructor used by AIReadAccess. - AIReadAccessConst(AIThreadSafe& wrapper, state_type state) + AIReadAccessConst(AIThreadSafe& wrapper, state_type state) : mWrapper(wrapper), mState(state) #if AI_NEED_ACCESS_CC , mIsCopyConstructed(false) #endif { } - AIThreadSafe& mWrapper; //!< Reference to the object that we provide access to. + AIThreadSafe& mWrapper; //!< Reference to the object that we provide access to. state_type const mState; //!< The lock state that mWrapper is in. #if AI_NEED_ACCESS_CC @@ -393,39 +393,39 @@ private: /** * @brief Read lock object and provide read access, with possible promotion to write access. */ -template -struct AIReadAccess : public AIReadAccessConst +template +struct AIReadAccess : public AIReadAccessConst { - typedef typename AIReadAccessConst::state_type state_type; - using AIReadAccessConst::readlocked; + typedef typename AIReadAccessConst::state_type state_type; + using AIReadAccessConst::readlocked; //! Construct a AIReadAccess from a non-constant AIThreadSafe. - AIReadAccess(AIThreadSafe& wrapper, bool high_priority = false) : AIReadAccessConst(wrapper, readlocked) { this->mWrapper.mRWLock.rdlock(high_priority); } + AIReadAccess(AIThreadSafe& wrapper, bool high_priority = false) : AIReadAccessConst(wrapper, readlocked) { this->mWrapper.mRWLock.rdlock(high_priority); } protected: //! Constructor used by AIWriteAccess. - AIReadAccess(AIThreadSafe& wrapper, state_type state) : AIReadAccessConst(wrapper, state) { } + AIReadAccess(AIThreadSafe& wrapper, state_type state) : AIReadAccessConst(wrapper, state) { } - friend struct AIWriteAccess; + friend struct AIWriteAccess; }; /** * @brief Write lock object and provide read/write access. */ -template -struct AIWriteAccess : public AIReadAccess +template +struct AIWriteAccess : public AIReadAccess { - using AIReadAccessConst::readlocked; - using AIReadAccessConst::read2writelocked; - using AIReadAccessConst::writelocked; - using AIReadAccessConst::write2writelocked; + using AIReadAccessConst::readlocked; + using AIReadAccessConst::read2writelocked; + using AIReadAccessConst::writelocked; + using AIReadAccessConst::write2writelocked; //! Construct a AIWriteAccess from a non-constant AIThreadSafe. - AIWriteAccess(AIThreadSafe& wrapper) : AIReadAccess(wrapper, writelocked) { this->mWrapper.mRWLock.wrlock();} + AIWriteAccess(AIThreadSafe& wrapper) : AIReadAccess(wrapper, writelocked) { this->mWrapper.mRWLock.wrlock();} //! Promote read access to write access. - explicit AIWriteAccess(AIReadAccess& access) - : AIReadAccess(access.mWrapper, (access.mState == readlocked) ? read2writelocked : write2writelocked) + explicit AIWriteAccess(AIReadAccess& access) + : AIReadAccess(access.mWrapper, (access.mState == readlocked) ? read2writelocked : write2writelocked) { if (this->mState == read2writelocked) { From e7aeb3feafb28a5353691bfdb0abd7dea8c58b77 Mon Sep 17 00:00:00 2001 From: Aleric Inglewood Date: Mon, 4 Feb 2013 22:56:40 +0100 Subject: [PATCH 03/16] Add AINRLock for debugging purposes. This class can be used as RWLOCK parameter to AIThreadSafe to check that data is only accessed by a single thread (like AIThreadSafeSingleThreaded) AND is never write locked when a read or write lock was already obtained (by the same thread). It doesn't actually lock anything, it just keeps track if the "lock" was obtained before. The use case is to check if STL containers aren't being used (read or write locked) by a calling function when additional write access is necessary, as write access might invalidate iterator that point to the container (where the previous lock was taken). --- indra/llcommon/llthread.h | 37 +++++++++++++++++++++++++++++++++++++ 1 file changed, 37 insertions(+) diff --git a/indra/llcommon/llthread.h b/indra/llcommon/llthread.h index 08d6e2110..f64200c95 100644 --- a/indra/llcommon/llthread.h +++ b/indra/llcommon/llthread.h @@ -397,6 +397,43 @@ public: #endif }; +#if LL_DEBUG +class LL_COMMON_API AINRLock +{ +private: + int read_locked; + int write_locked; + + mutable bool mAccessed; + mutable AIThreadID mTheadID; + + void accessed(void) const + { + if (!mAccessed) + { + mAccessed = true; + mTheadID.reset(); + } + else + { + llassert_always(mTheadID.equals_current_thread()); + } + } + +public: + AINRLock(void) : read_locked(false), write_locked(false), mAccessed(false) { } + + bool isLocked() const { return read_locked || write_locked; } + + void rdlock(bool high_priority = false) { accessed(); ++read_locked; } + void rdunlock() { --read_locked; } + void wrlock() { llassert(!isLocked()); accessed(); ++write_locked; } + void wrunlock() { --write_locked; } + void wr2rdlock() { llassert(false); } + void rd2wrlock() { llassert(false); } +}; +#endif + //============================================================================ void LLThread::lockData() From c1e6812f412a66939750af01feb45922f336c245 Mon Sep 17 00:00:00 2001 From: Aleric Inglewood Date: Tue, 5 Feb 2013 02:03:14 +0100 Subject: [PATCH 04/16] Remove unused variable. --- indra/newview/llfolderview.cpp | 6 ------ 1 file changed, 6 deletions(-) diff --git a/indra/newview/llfolderview.cpp b/indra/newview/llfolderview.cpp index 82e12f525..bea49eb6b 100644 --- a/indra/newview/llfolderview.cpp +++ b/indra/newview/llfolderview.cpp @@ -1513,12 +1513,6 @@ BOOL LLFolderView::handleKeyHere( KEY key, MASK mask ) LLMenuGL::sMenuContainer->hideMenus(); } - LLView *item = NULL; - if (getChildCount() > 0) - { - item = *(getChildList()->begin()); - } - switch( key ) { case KEY_F2: From ea114986bee76f63eab7cdbf22cbd7c61b587ba6 Mon Sep 17 00:00:00 2001 From: Aleric Inglewood Date: Tue, 5 Feb 2013 21:08:57 +0100 Subject: [PATCH 05/16] Use viewList_t and child_list_t in appropriate places. Both types are currently the same, but soon they will be made different. Currently they are used a bit mixed up. This patch fixes that. --- indra/llui/lluictrl.cpp | 14 +++++++------- indra/llui/llview.cpp | 20 ++++++++++---------- indra/llui/llview.h | 4 ++-- indra/llui/llviewquery.cpp | 8 ++++---- indra/llui/llviewquery.h | 2 +- indra/newview/llfloatereditui.cpp | 6 +++--- indra/newview/llpanelmediahud.cpp | 4 ++-- 7 files changed, 29 insertions(+), 29 deletions(-) diff --git a/indra/llui/lluictrl.cpp b/indra/llui/lluictrl.cpp index 58cb3e5fa..5e388e233 100644 --- a/indra/llui/lluictrl.cpp +++ b/indra/llui/lluictrl.cpp @@ -302,8 +302,8 @@ BOOL LLUICtrl::focusFirstItem(BOOL prefer_text_fields, BOOL focus_flash) LLCtrlQuery query = getTabOrderQuery(); // sort things such that the default tab group is at the front query.setSorter(DefaultTabGroupFirstSorter::getInstance()); - child_list_t result = query(this); - if(result.size() > 0) + viewList_t result = query(this); + if(!result.empty()) { LLUICtrl * ctrl = static_cast(result.front()); if(!ctrl->hasFocus()) @@ -322,7 +322,7 @@ BOOL LLUICtrl::focusFirstItem(BOOL prefer_text_fields, BOOL focus_flash) { LLCtrlQuery query = getTabOrderQuery(); query.addPreFilter(LLUICtrl::LLTextInputFilter::getInstance()); - child_list_t result = query(this); + viewList_t result = query(this); if(result.size() > 0) { LLUICtrl * ctrl = static_cast(result.front()); @@ -358,7 +358,7 @@ BOOL LLUICtrl::focusLastItem(BOOL prefer_text_fields) { LLCtrlQuery query = getTabOrderQuery(); query.addPreFilter(LLUICtrl::LLTextInputFilter::getInstance()); - child_list_t result = query(this); + viewList_t result = query(this); if(result.size() > 0) { LLUICtrl * ctrl = static_cast(result.back()); @@ -372,7 +372,7 @@ BOOL LLUICtrl::focusLastItem(BOOL prefer_text_fields) } } // no text field found, or we don't care about text fields - child_list_t result = getTabOrderQuery().run(this); + viewList_t result = getTabOrderQuery().run(this); if(result.size() > 0) { LLUICtrl * ctrl = static_cast(result.back()); @@ -395,7 +395,7 @@ BOOL LLUICtrl::focusNextItem(BOOL text_fields_only) { query.addPreFilter(LLUICtrl::LLTextInputFilter::getInstance()); } - child_list_t result = query(this); + viewList_t result = query(this); return focusNext(result); } @@ -407,7 +407,7 @@ BOOL LLUICtrl::focusPrevItem(BOOL text_fields_only) { query.addPreFilter(LLUICtrl::LLTextInputFilter::getInstance()); } - child_list_t result = query(this); + viewList_t result = query(this); return focusPrev(result); } diff --git a/indra/llui/llview.cpp b/indra/llui/llview.cpp index 07f99132b..40ee816a7 100644 --- a/indra/llui/llview.cpp +++ b/indra/llui/llview.cpp @@ -518,21 +518,21 @@ LLRect LLView::getRequiredRect() BOOL LLView::focusNextRoot() { - LLView::child_list_t result = LLView::getFocusRootsQuery().run(this); + viewList_t result = LLView::getFocusRootsQuery().run(this); return LLView::focusNext(result); } BOOL LLView::focusPrevRoot() { - LLView::child_list_t result = LLView::getFocusRootsQuery().run(this); + viewList_t result = LLView::getFocusRootsQuery().run(this); return LLView::focusPrev(result); } // static -BOOL LLView::focusNext(LLView::child_list_t & result) +BOOL LLView::focusNext(viewList_t& result) { - LLView::child_list_iter_t focused = result.end(); - for(LLView::child_list_iter_t iter = result.begin(); + viewList_t::iterator focused = result.end(); + for(viewList_t::iterator iter = result.begin(); iter != result.end(); ++iter) { @@ -542,7 +542,7 @@ BOOL LLView::focusNext(LLView::child_list_t & result) break; } } - LLView::child_list_iter_t next = focused; + viewList_t::iterator next = focused; next = (next == result.end()) ? result.begin() : ++next; while(next != focused) { @@ -565,10 +565,10 @@ BOOL LLView::focusNext(LLView::child_list_t & result) } // static -BOOL LLView::focusPrev(LLView::child_list_t & result) +BOOL LLView::focusPrev(viewList_t& result) { - LLView::child_list_reverse_iter_t focused = result.rend(); - for(LLView::child_list_reverse_iter_t iter = result.rbegin(); + viewList_t::reverse_iterator focused = result.rend(); + for(viewList_t::reverse_iterator iter = result.rbegin(); iter != result.rend(); ++iter) { @@ -578,7 +578,7 @@ BOOL LLView::focusPrev(LLView::child_list_t & result) break; } } - LLView::child_list_reverse_iter_t next = focused; + viewList_t::reverse_iterator next = focused; next = (next == result.rend()) ? result.rbegin() : ++next; while(next != focused) { diff --git a/indra/llui/llview.h b/indra/llui/llview.h index 233e56bbc..fbd132a8a 100644 --- a/indra/llui/llview.h +++ b/indra/llui/llview.h @@ -582,9 +582,9 @@ public: static std::string escapeXML(const std::string& xml, std::string& indent); // focuses the item in the list after the currently-focused item, wrapping if necessary - static BOOL focusNext(LLView::child_list_t & result); + static BOOL focusNext(viewList_t& result); // focuses the item in the list before the currently-focused item, wrapping if necessary - static BOOL focusPrev(LLView::child_list_t & result); + static BOOL focusPrev(viewList_t& result); // returns query for iterating over controls in tab order static const LLCtrlQuery & getTabOrderQuery(); diff --git a/indra/llui/llviewquery.cpp b/indra/llui/llviewquery.cpp index 1c9ba6b8f..2c1b9c2cd 100644 --- a/indra/llui/llviewquery.cpp +++ b/indra/llui/llviewquery.cpp @@ -113,12 +113,12 @@ viewList_t LLViewQuery::run(LLView* view) const void LLViewQuery::filterChildren(LLView * view, viewList_t & filtered_children) const { - LLView::child_list_t views(*(view->getChildList())); + viewList_t views(*(view->getChildList())); if (mSorterp) { (*mSorterp)(view, views); // sort the children per the sorter } - for(LLView::child_list_iter_t iter = views.begin(); + for(viewList_t::iterator iter = views.begin(); iter != views.end(); iter++) { @@ -127,7 +127,7 @@ void LLViewQuery::filterChildren(LLView * view, viewList_t & filtered_children) } } -filterResult_t LLViewQuery::runFilters(LLView * view, const viewList_t children, const filterList_t filters) const +filterResult_t LLViewQuery::runFilters(LLView* view, viewList_t const& children, filterList_t const& filters) const { filterResult_t result = filterResult_t(TRUE, TRUE); for(filterList_const_iter_t iter = filters.begin(); @@ -143,7 +143,7 @@ filterResult_t LLViewQuery::runFilters(LLView * view, const viewList_t children, class SortByTabOrder : public LLQuerySorter, public LLSingleton { - /*virtual*/ void operator() (LLView * parent, LLView::child_list_t &children) const + /*virtual*/ void operator() (LLView* parent, viewList_t& children) const { children.sort(LLCompareByTabOrder(parent->getCtrlOrder())); } diff --git a/indra/llui/llviewquery.h b/indra/llui/llviewquery.h index 98d9bf879..e058985cc 100644 --- a/indra/llui/llviewquery.h +++ b/indra/llui/llviewquery.h @@ -126,7 +126,7 @@ public: private: - filterResult_t runFilters(LLView * view, const viewList_t children, const filterList_t filters) const; + filterResult_t runFilters(LLView* view, viewList_t const& children, filterList_t const& filters) const; filterList_t mPreFilters; filterList_t mPostFilters; diff --git a/indra/newview/llfloatereditui.cpp b/indra/newview/llfloatereditui.cpp index ff67a2069..fbef26b18 100644 --- a/indra/newview/llfloatereditui.cpp +++ b/indra/newview/llfloatereditui.cpp @@ -56,9 +56,9 @@ void LLFloaterEditUI::navigateHierarchyButtonPressed(void* data) const LLView::child_list_t* viewChildren = view->getChildList(); const LLView::child_list_t* parentChildren = parent->getChildList(); //LLView::child_list_t::iterator - std::list::const_iterator itor; - std::list::size_type idx; - std::list::size_type sidx; + LLView::child_list_t::const_iterator itor; + LLView::child_list_t::size_type idx; + LLView::child_list_t::size_type sidx; for(idx = 0,itor = parentChildren->begin();itor!=parentChildren->end();itor++,idx++){ if((*itor)==view)break; } diff --git a/indra/newview/llpanelmediahud.cpp b/indra/newview/llpanelmediahud.cpp index 27d1473d6..e56e976f5 100644 --- a/indra/newview/llpanelmediahud.cpp +++ b/indra/newview/llpanelmediahud.cpp @@ -424,8 +424,8 @@ void LLPanelMediaHUD::setAlpha(F32 alpha) LLViewQuery query; LLView* query_view = mMediaFocus ? getChildView("media_focused_controls") : getChildView("media_hover_controls"); - child_list_t children = query(query_view); - for (child_list_iter_t child_iter = children.begin(); + viewList_t children = query(query_view); + for (viewList_t::iterator child_iter = children.begin(); child_iter != children.end(); ++child_iter) { LLUICtrl* ctrl = dynamic_cast(*child_iter); From baab1d81a7f7633aa123e45e563054cf22508d40 Mon Sep 17 00:00:00 2001 From: Aleric Inglewood Date: Wed, 6 Feb 2013 03:06:06 +0100 Subject: [PATCH 06/16] Wrap LLView::child_list_t in a class AIList. So far not doing anything. Iterators already keep a pointer to the container they belong to. --- indra/llui/CMakeLists.txt | 1 + indra/llui/ailist.h | 191 +++++++++++++++++++++++++++++++++++++ indra/llui/llview.h | 3 +- indra/llui/llviewquery.cpp | 4 +- 4 files changed, 196 insertions(+), 3 deletions(-) create mode 100644 indra/llui/ailist.h diff --git a/indra/llui/CMakeLists.txt b/indra/llui/CMakeLists.txt index 6aa14e8d6..e35d5b5dd 100644 --- a/indra/llui/CMakeLists.txt +++ b/indra/llui/CMakeLists.txt @@ -82,6 +82,7 @@ set(llui_SOURCE_FILES set(llui_HEADER_FILES CMakeLists.txt + ailist.h llalertdialog.h llbutton.h llcallbackmap.h diff --git a/indra/llui/ailist.h b/indra/llui/ailist.h new file mode 100644 index 000000000..821f679cb --- /dev/null +++ b/indra/llui/ailist.h @@ -0,0 +1,191 @@ +/** + * @file ailist.h + * @brief A linked list with iterators that advance when their element is deleted. + * + * Copyright (c) 2013, Aleric Inglewood. + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 2 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see . + * + * There are special exceptions to the terms and conditions of the GPL as + * it is applied to this Source Code. View the full text of the exception + * in the file doc/FLOSS-exception.txt in this software distribution. + * + * CHANGELOG + * and additional copyright holders. + * + * 05/02/2013 + * Initial version, written by Aleric Inglewood @ SL + */ + +#ifndef AILIST_H +#define AILIST_H + +#include + +template +class AIConstListIterator; + +template +class AIListIterator { + private: + typedef AIListIterator _Self; + typedef std::list _Container; + typedef typename _Container::iterator _Iterator; + + _Container* mContainer; + _Iterator mIterator; + + public: + typedef typename _Iterator::difference_type difference_type; + typedef typename _Iterator::iterator_category iterator_category; + typedef typename _Iterator::value_type value_type; + typedef typename _Iterator::pointer pointer; + typedef typename _Iterator::reference reference; + + AIListIterator(void) : mContainer(NULL) { } + AIListIterator(_Container* __c, _Iterator const& __i) : mContainer(__c), mIterator(__i) { } + + _Self& operator=(_Self const& __x) { mContainer = __x.mContainer; mIterator = __x.mIterator; return *this; } + + reference operator*() const { return *mIterator; } + pointer operator->() const { return mIterator.operator->(); } + _Self& operator++() { ++mIterator; return *this; } + _Self operator++(int) { _Self tmp = *this; ++mIterator; return tmp; } + _Self& operator--() { --mIterator; return *this; } + _Self operator--(int) { _Self tmp = *this; --mIterator; return tmp; } + + bool operator==(_Self const& __x) const { return mIterator == __x.mIterator; } + bool operator!=(_Self const& __x) const { return mIterator != __x.mIterator; } + + template friend class AIConstListIterator; + template friend bool operator==(AIListIterator const& __x, AIConstListIterator const& __y); + template friend bool operator!=(AIListIterator const& __x, AIConstListIterator const& __y); +}; + +template +class AIConstListIterator { + private: + typedef AIConstListIterator _Self; + typedef std::list _Container; + typedef typename _Container::const_iterator _ConstIterator; + typedef AIListIterator iterator; + + _Container const* mContainer; + _ConstIterator mConstIterator; + + public: + typedef typename _ConstIterator::difference_type difference_type; + typedef typename _ConstIterator::iterator_category iterator_category; + typedef typename _ConstIterator::value_type value_type; + typedef typename _ConstIterator::pointer pointer; + typedef typename _ConstIterator::reference reference; + + AIConstListIterator(void) : mContainer(NULL) { } + AIConstListIterator(_Container const* __c, _ConstIterator const& __i) : mContainer(__c), mConstIterator(__i) { } + AIConstListIterator(iterator const& __x) : mContainer(__x.mContainer), mConstIterator(__x.mIterator) { } + + _Self& operator=(_Self const& __x) { mContainer = __x.mContainer; mConstIterator = __x.mConstIterator; return *this; } + _Self& operator=(iterator const& __x) { mContainer = __x.mContainer; mConstIterator = __x.mIterator; return *this; } + + reference operator*() const { return *mConstIterator; } + pointer operator->() const { return mConstIterator.operator->(); } + _Self& operator++() { ++mConstIterator; return *this; } + _Self operator++(int) { _Self tmp = *this; ++mConstIterator; return tmp; } + _Self& operator--() { --mConstIterator; return *this; } + _Self operator--(int) { _Self tmp = *this; --mConstIterator; return tmp; } + + bool operator==(_Self const& __x) const { return mConstIterator == __x.mConstIterator; } + bool operator!=(_Self const& __x) const { return mConstIterator != __x.mConstIterator; } + + template friend bool operator==(AIListIterator const& __x, AIConstListIterator const& __y); + template friend bool operator!=(AIListIterator const& __x, AIConstListIterator const& __y); +}; + +template +inline bool operator==(AIListIterator const& __x, AIConstListIterator const& __y) +{ + return __x.mIterator == __y.mConstIterator; +} + +template +inline bool operator!=(AIListIterator const& __x, AIConstListIterator const& __y) +{ + return __x.mIterator != __y.mConstIterator; +} + +template +class AIList { + private: + std::list mContainer; + + public: + typedef T value_type; + typedef T* pointer; + typedef T const* const_pointer; + typedef T& reference; + typedef T const& const_reference; + typedef AIListIterator iterator; + typedef AIConstListIterator const_iterator; + typedef std::reverse_iterator reverse_iterator; + typedef std::reverse_iterator const_reverse_iterator; + typedef size_t size_type; + typedef ptrdiff_t difference_type; + + AIList(void) { } + //AIList(std::list const& __list) : mContainer(__list) { } + + explicit AIList(size_type __n, value_type const& __value = value_type()) : mContainer(__n, __value) { } + AIList(AIList const& __list) : mContainer(__list.mContainer) { } + + template + AIList(_InputIterator __first, _InputIterator __last) : mContainer(__first, __last) { } + + AIList& operator=(AIList const& __list) { mContainer = __list.mContainer; return *this; } + + iterator begin() { return iterator(&mContainer, mContainer.begin()); } + const_iterator begin() const { return const_iterator(&mContainer, mContainer.begin()); } + iterator end() { return iterator(&mContainer, mContainer.end()); } + const_iterator end() const { return const_iterator(&mContainer, mContainer.end()); } + reverse_iterator rbegin() { return reverse_iterator(end()); } + const_reverse_iterator rbegin() const { return const_reverse_iterator(end()); } + reverse_iterator rend() { return reverse_iterator(begin()); } + const_reverse_iterator rend() const { return const_reverse_iterator(begin()); } + + bool empty() const { return mContainer.empty(); } + size_type size() const { return mContainer.size(); } + size_type max_size() const { return mContainer.max_size(); } + + reference front() { return mContainer.front(); } + const_reference front() const { return mContainer.front(); } + reference back() { return mContainer.back(); } + const_reference back() const { return mContainer.back(); } + + void push_front(value_type const& __x) { mContainer.push_front(__x); } + void pop_front() { mContainer.pop_front(); } + void push_back(value_type const& __x) { mContainer.push_back(__x); } + void pop_back() { mContainer.pop_back(); } + iterator insert(iterator __position, value_type const& __x) { return iterator(&mContainer, mContainer.insert(__position, __x)); } + iterator erase(iterator __position) { return iterator(&mContainer, mContainer.erase(__position)); } + void clear() { mContainer.clear(); } + void remove(value_type const& __value) { mContainer.remove(__value); } + + // Use this with care. No iterators should be left pointing at elements after the code returns. + std::list const& get_std_list(void) const { return mContainer; } + + void sort() { mContainer.sort(); } + template + void sort(_StrictWeakOrdering pred) { mContainer.sort(pred); } +}; + +#endif diff --git a/indra/llui/llview.h b/indra/llui/llview.h index fbd132a8a..2ec5ce58c 100644 --- a/indra/llui/llview.h +++ b/indra/llui/llview.h @@ -57,6 +57,7 @@ #include "llinitparam.h" #include "llfocusmgr.h" #include +#include "ailist.h" const U32 FOLLOWS_NONE = 0x00; const U32 FOLLOWS_LEFT = 0x01; @@ -231,7 +232,7 @@ public: SNAP_BOTTOM }; - typedef std::list child_list_t; + typedef AIList child_list_t; typedef child_list_t::iterator child_list_iter_t; typedef child_list_t::const_iterator child_list_const_iter_t; typedef child_list_t::reverse_iterator child_list_reverse_iter_t; diff --git a/indra/llui/llviewquery.cpp b/indra/llui/llviewquery.cpp index 2c1b9c2cd..7faf53726 100644 --- a/indra/llui/llviewquery.cpp +++ b/indra/llui/llviewquery.cpp @@ -76,7 +76,7 @@ viewList_t LLViewQuery::run(LLView* view) const viewList_t result; // prefilter gets immediate children of view - filterResult_t pre = runFilters(view, *view->getChildList(), mPreFilters); + filterResult_t pre = runFilters(view, view->getChildList()->get_std_list(), mPreFilters); if(!pre.first && !pre.second) { // not including ourselves or the children @@ -113,7 +113,7 @@ viewList_t LLViewQuery::run(LLView* view) const void LLViewQuery::filterChildren(LLView * view, viewList_t & filtered_children) const { - viewList_t views(*(view->getChildList())); + viewList_t views(view->getChildList()->get_std_list()); if (mSorterp) { (*mSorterp)(view, views); // sort the children per the sorter From d5482e6c74cabb2b9e79c76d259ed13acc3873aa Mon Sep 17 00:00:00 2001 From: Aleric Inglewood Date: Wed, 6 Feb 2013 03:09:17 +0100 Subject: [PATCH 07/16] Do not pre- increment the loop iterator in LLView::drawChildren This is in fact much safer, because the only way to invalidate an interator in the first place (in this code) is by calling removeChild, which *already* has an assert never to remove a child that is being iterated over (by means of the mInDraw flag). --- indra/llui/llview.cpp | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-) diff --git a/indra/llui/llview.cpp b/indra/llui/llview.cpp index 40ee816a7..6c1bce689 100644 --- a/indra/llui/llview.cpp +++ b/indra/llui/llview.cpp @@ -1170,16 +1170,13 @@ void LLView::drawChildren() LLView* rootp = getRootView(); ++sDepth; - for (child_list_reverse_iter_t child_iter = mChildList.rbegin(); child_iter != mChildList.rend();) // ++child_iter) + for (child_list_const_reverse_iter_t child_iter = mChildList.rbegin(); child_iter != mChildList.rend(); ++child_iter) { - child_list_reverse_iter_t child = child_iter++; - LLView *viewp = *child; - - if (viewp == NULL) - { - continue; - } - + LLView *viewp = *child_iter; + if (viewp == NULL) + { + continue; + } if (viewp->getVisible() && /*viewp != focus_view && */viewp->getRect().isValid()) { From dd6f95cd330422a69d193f3e423ae35d9b33969a Mon Sep 17 00:00:00 2001 From: Aleric Inglewood Date: Fri, 8 Feb 2013 14:13:54 +0100 Subject: [PATCH 08/16] Make AIList resilient against invalidating iterators. This adds a counter and a 'dead' flag to the data stored in the linked list. The counter counts the number of iterators (still) pointing to an element and the dead flag is set to indicate the element was erased when iterators are still pointing to it. As a result, iterators are NEVER invalidated. Of course, such elements are subsequentially skipped when iterating over the list. Assertions protect against dereferencing an erased iterator, but incrementing or decremention still works: it is still well-defined what the next (non erased) element is, assuming the element wasn't erased (yet), but would be erased delayed - or assuming the iterator would have been incremented (decremented) in advance to erasing the element. --- indra/llui/ailist.h | 580 ++++++++++++++++++++++++++++---- indra/llui/llviewquery.cpp | 9 +- indra/newview/lltextureview.cpp | 4 +- 3 files changed, 524 insertions(+), 69 deletions(-) diff --git a/indra/llui/ailist.h b/indra/llui/ailist.h index 821f679cb..be2142f7e 100644 --- a/indra/llui/ailist.h +++ b/indra/llui/ailist.h @@ -33,83 +33,372 @@ #include +template +class AIList; + template class AIConstListIterator; +/* + * AINode + * + * The actual data stored in a std::list when using AIList. + * T is the template parameter used with AIList. + * count are the number of iterators that currently point to this element. + * dead is 0 or 1, where 1 means that the element was erased from the AIList + * (but not yet from the internal std::list, so that any iterators to it + * are NOT invalidated). + */ +template +struct AINode { + T mElement; // The user visual data. + mutable unsigned short count; // Number of iterators pointing to this element. + mutable unsigned short dead; // Whether or not the element is "erased". + + AINode(void) : count(0), dead(0) { } + explicit AINode(T const& __value) : mElement(__value), count(0), dead(0) { } + + // Equivalence operators. + // __node may not be dead. Dead nodes in the list are "skipped" (obviously), meaning that they are never equal or always unequal. + bool operator==(AINode const& __node) const { llassert(!__node.dead); return mElement == __node.mElement && !dead; } + bool operator!=(AINode const& __node) const { llassert(!__node.dead); return mElement != __node.mElement || dead; } + + // Default ordering for sort(). + bool operator<(AINode const& __node) const { return mElement < __node.mElement; } +}; + +/* + * AIListIterator + * + * A non-const iterator to an element of AIList. + */ template class AIListIterator { private: typedef AIListIterator _Self; - typedef std::list _Container; + typedef std::list > _Container; typedef typename _Container::iterator _Iterator; - _Container* mContainer; - _Iterator mIterator; + _Container* mContainer; // A pointer to the associated container, or NULL when (still) singular. + // Note that this code does not allow a container to be destructed while + // any non-end iterators still exist (although that could be legal). + // If an iterator points to end() and the container is destructed then + // this pointer is NOT reset to NULL. + _Iterator mIterator; // Internal iterator to element of mContainer (or singular when mContainer is NULL). + + // Increment reference counter for mIterator (if not singular and not end()). + void ref(void) + { + if (mContainer && mContainer->end() != mIterator) + { + // It would be bad a new iterator was created that pointed to a dead element. + // Also, as a sanity check, make sure there aren't a ridiculous number of iterators + // pointing to this element. + llassert(!mIterator->dead && mIterator->count < 100); + ++(mIterator->count); + } + } + + // Decrement reference counter for mIterator (if not singular and not end()). + // If this was the last iterator pointing to a dead element, then really erase it. + void unref(void) + { + if (mContainer && mContainer->end() != mIterator) + { + llassert(mIterator->count > 0); + if (--(mIterator->count) == 0 && mIterator->dead) + { + mContainer->erase(mIterator); + } + } + } public: + // Some standard typedefs that have to exist for iterators. typedef typename _Iterator::difference_type difference_type; typedef typename _Iterator::iterator_category iterator_category; - typedef typename _Iterator::value_type value_type; - typedef typename _Iterator::pointer pointer; - typedef typename _Iterator::reference reference; + typedef T value_type; + typedef T* pointer; + typedef T& reference; + // Construct a singular iterator. AIListIterator(void) : mContainer(NULL) { } - AIListIterator(_Container* __c, _Iterator const& __i) : mContainer(__c), mIterator(__i) { } - _Self& operator=(_Self const& __x) { mContainer = __x.mContainer; mIterator = __x.mIterator; return *this; } + // Construct an iterator to a given element of std::list. Only for internal use by AIList. + AIListIterator(_Container* __c, _Iterator const& __i) : mContainer(__c), mIterator(__i) + { + llassert(mContainer); + ref(); + } - reference operator*() const { return *mIterator; } - pointer operator->() const { return mIterator.operator->(); } - _Self& operator++() { ++mIterator; return *this; } - _Self operator++(int) { _Self tmp = *this; ++mIterator; return tmp; } - _Self& operator--() { --mIterator; return *this; } - _Self operator--(int) { _Self tmp = *this; --mIterator; return tmp; } + // Copy constructor. + AIListIterator(AIListIterator const& __i) : mContainer(__i.mContainer), mIterator(__i.mIterator) + { + ref(); + } - bool operator==(_Self const& __x) const { return mIterator == __x.mIterator; } - bool operator!=(_Self const& __x) const { return mIterator != __x.mIterator; } + // Destructor. + ~AIListIterator() + { + unref(); + } - template friend class AIConstListIterator; + // Assignment operator. + _Self& operator=(_Self const& __x) + { + unref(); // We no longer point to whatever we were pointing. + mContainer = __x.mContainer; + mIterator = __x.mIterator; + llassert(mContainer); + ref(); // We now point an(other) element. + return *this; + } + + // Dereference operator. + reference operator*() const + { + // Iterator may not be singular or dead. + llassert(mContainer && !mIterator->dead); + return mIterator->mElement; + } + + // Dereference operator. + pointer operator->() const + { + // Iterator may not be singular or dead. + llassert(mContainer && !mIterator->dead); + return &mIterator->mElement; + } + + // Pre-increment operator (not being singular is implied). + _Self& operator++() + { + _Iterator cur = mIterator; // Make copy of mIterator. + ++cur; // Advance it. + unref(); // We will no longer be pointing to this element. This might invalidate mIterator! + while(cur != mContainer->end() && cur->dead) // Advance till the first non-dead element. + { + ++cur; + } + mIterator = cur; // Put result back into mIterator. + ref(); // We are now pointing to a valid element again. + return *this; + } + + // Post-increment operator (not being singular is implied). + _Self operator++(int) + { + _Self tmp = *this; + this->operator++(); + return tmp; + } + + // Pre-decrement operator (not being singular is implied). + _Self& operator--() + { + _Iterator cur = mIterator; // See operator++(). + --cur; + unref(); + while(cur->dead) + { + --cur; + } + mIterator = cur; + ref(); + return *this; + } + + // Post-decrement operator (not being singular is implied). + _Self operator--(int) + { + _Self tmp = *this; + this->operator--(); + return tmp; + } + + // Equivalence operators. + // We allow comparing with dead iterators, because if one of them is not-dead + // then the result is "unequal" anyway, which is probably what you want. + bool operator==(_Self const& __x) const { return mIterator == __x.mIterator; } + bool operator!=(_Self const& __x) const { return mIterator != __x.mIterator; } + + friend class AIList; + friend class AIConstListIterator; template friend bool operator==(AIListIterator const& __x, AIConstListIterator const& __y); template friend bool operator!=(AIListIterator const& __x, AIConstListIterator const& __y); + + // Return the total number of iterators pointing the element that this iterator is pointing to. + // Unless the iterator points to end, a positive number will be returned since this iterator is + // pointing to the element. If it points to end (or is singular) then 0 is returned. + int count(void) const + { + if (mContainer && mIterator != mContainer->end()) + { + return mIterator->count; + } + return 0; + } }; +/* + * AIConstListIterator + * + * A const iterator to an element of AIList. + * + * Because this class is very simular to AIListIterator, see above for detailed comments. + */ template class AIConstListIterator { private: typedef AIConstListIterator _Self; - typedef std::list _Container; + typedef std::list > _Container; + typedef typename _Container::iterator _Iterator; typedef typename _Container::const_iterator _ConstIterator; typedef AIListIterator iterator; _Container const* mContainer; - _ConstIterator mConstIterator; + _Iterator mConstIterator; // This has to be an _Iterator instead of _ConstIterator, because the compiler doesn't accept a const_iterator for erase yet (C++11 does). + + void ref(void) + { + if (mContainer && mContainer->end() != mConstIterator) + { + llassert(mConstIterator->count < 100); + mConstIterator->count++; + } + } + + void unref(void) + { + if (mContainer && mContainer->end() != mConstIterator) + { + llassert(mConstIterator->count > 0); + mConstIterator->count--; + if (mConstIterator->count == 0 && mConstIterator->dead) + { + const_cast<_Container*>(mContainer)->erase(mConstIterator); + } + } + } public: typedef typename _ConstIterator::difference_type difference_type; typedef typename _ConstIterator::iterator_category iterator_category; - typedef typename _ConstIterator::value_type value_type; - typedef typename _ConstIterator::pointer pointer; - typedef typename _ConstIterator::reference reference; + typedef T value_type; + typedef T const* pointer; + typedef T const& reference; AIConstListIterator(void) : mContainer(NULL) { } - AIConstListIterator(_Container const* __c, _ConstIterator const& __i) : mContainer(__c), mConstIterator(__i) { } - AIConstListIterator(iterator const& __x) : mContainer(__x.mContainer), mConstIterator(__x.mIterator) { } + AIConstListIterator(_Container const* __c, _Iterator const& __i) : mContainer(__c), mConstIterator(__i) + { + llassert(mContainer); + ref(); + } + // Allow to construct a const_iterator from an iterator. + AIConstListIterator(iterator const& __x) : mContainer(__x.mContainer), mConstIterator(__x.mIterator) + { + ref(); + } + AIConstListIterator(AIConstListIterator const& __i) : mContainer(__i.mContainer), mConstIterator(__i.mConstIterator) + { + ref(); + } + ~AIConstListIterator() + { + unref(); + } - _Self& operator=(_Self const& __x) { mContainer = __x.mContainer; mConstIterator = __x.mConstIterator; return *this; } - _Self& operator=(iterator const& __x) { mContainer = __x.mContainer; mConstIterator = __x.mIterator; return *this; } + _Self& operator=(_Self const& __x) + { + unref(); + mContainer = __x.mContainer; + mConstIterator = __x.mConstIterator; + llassert(mContainer); + ref(); + return *this; + } - reference operator*() const { return *mConstIterator; } - pointer operator->() const { return mConstIterator.operator->(); } - _Self& operator++() { ++mConstIterator; return *this; } - _Self operator++(int) { _Self tmp = *this; ++mConstIterator; return tmp; } - _Self& operator--() { --mConstIterator; return *this; } - _Self operator--(int) { _Self tmp = *this; --mConstIterator; return tmp; } + // Allow to assign from a non-const iterator. + _Self& operator=(iterator const& __x) + { + unref(); + mContainer = __x.mContainer; + mConstIterator = __x.mIterator; + llassert(mContainer); + ref(); + return *this; + } - bool operator==(_Self const& __x) const { return mConstIterator == __x.mConstIterator; } - bool operator!=(_Self const& __x) const { return mConstIterator != __x.mConstIterator; } + reference operator*() const + { + llassert(mContainer && !mConstIterator->dead); + return mConstIterator->mElement; + } + + pointer operator->() const + { + llassert(mContainer && !mConstIterator->dead); + return &mConstIterator->mElement; + } + + _Self& operator++() + { + _Iterator cur = mConstIterator; + ++cur; + unref(); + while(cur != mContainer->end() && cur->dead) + { + ++cur; + } + mConstIterator = cur; + ref(); + return *this; + } + + _Self operator++(int) + { + _Self tmp = *this; + this->operator++(); + return tmp; + } + + _Self& operator--() + { + _Iterator cur = mConstIterator; + --cur; + unref(); + while(cur->dead) + { + --cur; + } + mConstIterator = cur; + ref(); + return *this; + } + + _Self operator--(int) + { + _Self tmp = *this; + this->operator--(); + return tmp; + } + + bool operator==(_Self const& __x) const { return mConstIterator == __x.mConstIterator; } + bool operator!=(_Self const& __x) const { return mConstIterator != __x.mConstIterator; } + bool operator==(iterator const& __x) const { return mConstIterator == __x.mIterator; } + bool operator!=(iterator const& __x) const { return mConstIterator != __x.mIterator; } template friend bool operator==(AIListIterator const& __x, AIConstListIterator const& __y); template friend bool operator!=(AIListIterator const& __x, AIConstListIterator const& __y); + + int count(void) const + { + if (mContainer && mConstIterator != mContainer->end()) + { + return mConstIterator->count; + } + return 0; + } }; template @@ -124,10 +413,31 @@ inline bool operator!=(AIListIterator const& __x, AIConstListIterator cons return __x.mIterator != __y.mConstIterator; } +/* + * AIList + * + * A linked list that allows elements to be erased while one or more iterators + * are still pointing to that element, after which pre-increment and pre-decrement + * operators still work. + * + * For example: + * + * for (AIList::iterator i = l.begin(); i != l.end(); ++i) + * { + * int x = *i; + * f(i); // Might erase any element of list l. + * // Should not dereference i anymore here (because it might be erased). + * } + */ template class AIList { private: - std::list mContainer; + typedef std::list > _Container; + typedef typename _Container::iterator _Iterator; + typedef typename _Container::const_iterator _ConstIterator; + + _Container mContainer; + size_t mSize; public: typedef T value_type; @@ -142,50 +452,194 @@ class AIList { typedef size_t size_type; typedef ptrdiff_t difference_type; - AIList(void) { } - //AIList(std::list const& __list) : mContainer(__list) { } + // Default constructor. Create an empty list. + AIList(void) : mSize(0) { } +#ifdef LL_DEBUG + // Destructor calls clear() to check if there are no iterators left pointing to this list. Destructing an empty list is trivial. + ~AIList() { clear(); } +#endif - explicit AIList(size_type __n, value_type const& __value = value_type()) : mContainer(__n, __value) { } - AIList(AIList const& __list) : mContainer(__list.mContainer) { } + // Construct a list with __n elements of __value. + explicit AIList(size_type __n, value_type const& __value = value_type()) : mContainer(__n, AINode(__value)), mSize(__n) { } + // Copy constructor. + AIList(AIList const& __list) : mSize(0) + { + for (_ConstIterator __i = __list.mContainer.begin(); __i != __list.mContainer.end(); ++__i) + { + if (!__i->dead) + { + mContainer.push_back(AINode(__i->mElement)); + ++mSize; + } + } + } + + // Construct a list from the range [__first, __last>. template - AIList(_InputIterator __first, _InputIterator __last) : mContainer(__first, __last) { } + AIList(_InputIterator __first, _InputIterator __last) : mSize(0) + { + for (_InputIterator __i = __first; __i != __last; ++__i) + { + mContainer.push_back(AINode(*__i)); + ++mSize; + } + } - AIList& operator=(AIList const& __list) { mContainer = __list.mContainer; return *this; } + // Assign from another list. + AIList& operator=(AIList const& __list) + { + clear(); + for (_ConstIterator __i = __list.mContainer.begin(); __i != __list.mContainer.end(); ++__i) + { + if (!__i->dead) + { + mContainer.push_back(AINode(__i->mElement)); + ++mSize; + } + } + return *this; + } + + iterator begin() + { + _Iterator __i = mContainer.begin(); + while(__i != mContainer.end() && __i->dead) + { + ++__i; + } + return iterator(&mContainer, __i); + } + + const_iterator begin() const + { + _Iterator __i = const_cast<_Container&>(mContainer).begin(); + while(__i != mContainer.end() && __i->dead) + { + ++__i; + } + return const_iterator(&mContainer, __i); + } - iterator begin() { return iterator(&mContainer, mContainer.begin()); } - const_iterator begin() const { return const_iterator(&mContainer, mContainer.begin()); } iterator end() { return iterator(&mContainer, mContainer.end()); } - const_iterator end() const { return const_iterator(&mContainer, mContainer.end()); } + const_iterator end() const { return const_iterator(&mContainer, const_cast<_Container&>(mContainer).end()); } reverse_iterator rbegin() { return reverse_iterator(end()); } const_reverse_iterator rbegin() const { return const_reverse_iterator(end()); } reverse_iterator rend() { return reverse_iterator(begin()); } const_reverse_iterator rend() const { return const_reverse_iterator(begin()); } - bool empty() const { return mContainer.empty(); } - size_type size() const { return mContainer.size(); } - size_type max_size() const { return mContainer.max_size(); } + bool empty() const { return mSize == 0; } + size_type size() const { return mSize; } + size_type max_size() const { return mContainer.max_size(); } - reference front() { return mContainer.front(); } - const_reference front() const { return mContainer.front(); } - reference back() { return mContainer.back(); } - const_reference back() const { return mContainer.back(); } + reference front() { iterator __i = begin(); return *__i; } + const_reference front() const { const_iterator __i = begin(); return *__i; } + reference back() { iterator __i = end(); --__i; return *__i; } + const_reference back() const { const_iterator __i = end(); --__i; return *__i; } - void push_front(value_type const& __x) { mContainer.push_front(__x); } - void pop_front() { mContainer.pop_front(); } - void push_back(value_type const& __x) { mContainer.push_back(__x); } - void pop_back() { mContainer.pop_back(); } - iterator insert(iterator __position, value_type const& __x) { return iterator(&mContainer, mContainer.insert(__position, __x)); } - iterator erase(iterator __position) { return iterator(&mContainer, mContainer.erase(__position)); } - void clear() { mContainer.clear(); } - void remove(value_type const& __value) { mContainer.remove(__value); } + void push_front(value_type const& __x) + { + mContainer.push_front(AINode(__x)); + ++mSize; + } + void pop_front() + { + iterator __i = begin(); + erase(__i); + } + void push_back(value_type const& __x) + { + mContainer.push_back(AINode(__x)); + ++mSize; + } + void pop_back() + { + iterator __i = end(); + --__i; + erase(__i); + } - // Use this with care. No iterators should be left pointing at elements after the code returns. - std::list const& get_std_list(void) const { return mContainer; } + iterator insert(iterator __position, value_type const& __x) + { + ++mSize; + return iterator(&mContainer, mContainer.insert(__position.mIterator, AINode(__x))); + } - void sort() { mContainer.sort(); } + void clear() + { +#ifdef LL_DEBUG + // There should be no iterators left pointing at any element here. + for (_Iterator __i = mContainer.begin(); __i != mContainer.end(); ++__i) + { + llassert(__i->count == 0); + } +#endif + mContainer.clear(); + mSize = 0; + } + + iterator erase(iterator __position) + { + // Mark the element __position points to as being erased. + // Iterator may not be singular, point to end, or be dead already. + // Obviously count must be larger than zero since __position is still pointing to it. + llassert(__position.mContainer == &mContainer && __position.mIterator != mContainer.end() && __position.mIterator->count > 0 && !__position.mIterator->dead); + __position.mIterator->dead = 1; + --mSize; + return ++__position; + } + + // Remove all elements, designated by the iterator where, for which *where == __value. + void remove(value_type const& __value) + { + _Iterator const __e = mContainer.end(); + for (_Iterator __i = mContainer.begin(); __i != __e;) + { + if (!__i->dead && __i->mElement == __value) + { + --mSize; + if (__i->count == 0) + { + mContainer.erase(__i++); + continue; + } + // Mark the element as being erased. + __i->dead = 1; + } + ++__i; + } + } + + void sort() + { +#ifdef LL_DEBUG + // There should be no iterators left pointing at any element here. + for (_Iterator __i = mContainer.begin(); __i != mContainer.end(); ++__i) + { + llassert(__i->count == 0); + } +#endif + mContainer.sort(); + } template - void sort(_StrictWeakOrdering pred) { mContainer.sort(pred); } + struct PredWrapper + { + _StrictWeakOrdering mPred; + PredWrapper(_StrictWeakOrdering const& pred) : mPred(pred) { } + bool operator()(AINode const& __x, AINode const& __y) const { return mPred(__x.mElement, __y.mElement); } + }; + template + void sort(_StrictWeakOrdering const& pred) + { +#ifdef LL_DEBUG + // There should be no iterators left pointing at any element here. + for (_Iterator __i = mContainer.begin(); __i != mContainer.end(); ++__i) + { + llassert(__i->count == 0); + } +#endif + mContainer.sort(PredWrapper<_StrictWeakOrdering>(pred)); + } }; #endif diff --git a/indra/llui/llviewquery.cpp b/indra/llui/llviewquery.cpp index 7faf53726..bb3bab353 100644 --- a/indra/llui/llviewquery.cpp +++ b/indra/llui/llviewquery.cpp @@ -74,9 +74,10 @@ filterResult_t LLCtrlFilter::operator() (const LLView* const view, const viewLis viewList_t LLViewQuery::run(LLView* view) const { viewList_t result; + viewList_t const child_list(view->getChildList()->begin(), view->getChildList()->end()); // prefilter gets immediate children of view - filterResult_t pre = runFilters(view, view->getChildList()->get_std_list(), mPreFilters); + filterResult_t pre = runFilters(view, child_list, mPreFilters); if(!pre.first && !pre.second) { // not including ourselves or the children @@ -113,14 +114,14 @@ viewList_t LLViewQuery::run(LLView* view) const void LLViewQuery::filterChildren(LLView * view, viewList_t & filtered_children) const { - viewList_t views(view->getChildList()->get_std_list()); + viewList_t views(view->getChildList()->begin(), view->getChildList()->end()); if (mSorterp) { (*mSorterp)(view, views); // sort the children per the sorter } for(viewList_t::iterator iter = views.begin(); iter != views.end(); - iter++) + ++iter) { viewList_t indiv_children = this->run(*iter); filtered_children.splice(filtered_children.end(), indiv_children); @@ -132,7 +133,7 @@ filterResult_t LLViewQuery::runFilters(LLView* view, viewList_t const& children, filterResult_t result = filterResult_t(TRUE, TRUE); for(filterList_const_iter_t iter = filters.begin(); iter != filters.end(); - iter++) + ++iter) { filterResult_t filtered = (**iter)(view, children); result.first = result.first && filtered.first; diff --git a/indra/newview/lltextureview.cpp b/indra/newview/lltextureview.cpp index 0a0eb5f08..e57fbc3a2 100644 --- a/indra/newview/lltextureview.cpp +++ b/indra/newview/lltextureview.cpp @@ -101,7 +101,7 @@ public: // Used for sorting struct sort { - bool operator()(const LLView* i1, const LLView* i2) + bool operator()(const LLView* i1, const LLView* i2) const { LLTextureBar* bar1p = (LLTextureBar*)i1; LLTextureBar* bar2p = (LLTextureBar*)i2; @@ -120,7 +120,7 @@ public: struct sort_fetch { - bool operator()(const LLView* i1, const LLView* i2) + bool operator()(const LLView* i1, const LLView* i2) const { LLTextureBar* bar1p = (LLTextureBar*)i1; LLTextureBar* bar2p = (LLTextureBar*)i2; From 2d7ab61c6e496fbee7fd03d015da37c9338c4174 Mon Sep 17 00:00:00 2001 From: Aleric Inglewood Date: Fri, 8 Feb 2013 18:40:14 +0100 Subject: [PATCH 09/16] Make keyboard focus more robust under closed floaters. --- indra/llui/llfloater.cpp | 2 ++ indra/llui/llfocusmgr.cpp | 35 +++++++++++++++++++++++++++-- indra/llui/llfocusmgr.h | 5 ++++- indra/newview/llfloatersnapshot.cpp | 5 +---- 4 files changed, 40 insertions(+), 7 deletions(-) diff --git a/indra/llui/llfloater.cpp b/indra/llui/llfloater.cpp index 1cbb52dab..b91b0df43 100644 --- a/indra/llui/llfloater.cpp +++ b/indra/llui/llfloater.cpp @@ -911,6 +911,8 @@ void LLFloater::setMinimized(BOOL minimize) // Lose keyboard focus when minimized releaseFocus(); + // Also reset mLockedView and mLastKeyboardFocus, to avoid that we get focus back somehow. + gFocusMgr.removeKeyboardFocusWithoutCallback(this); for (S32 i = 0; i < 4; i++) { diff --git a/indra/llui/llfocusmgr.cpp b/indra/llui/llfocusmgr.cpp index 66834a6a1..7817fd0d5 100644 --- a/indra/llui/llfocusmgr.cpp +++ b/indra/llui/llfocusmgr.cpp @@ -55,6 +55,8 @@ BOOL LLFocusableElement::handleUnicodeChar(llwchar uni_char, BOOL called_from_pa // virtual LLFocusableElement::~LLFocusableElement() { + // Make sure nothing is pointing to us anymore! + gFocusMgr.removeKeyboardFocusWithoutCallback(this); delete mFocusLostCallback; delete mFocusReceivedCallback; delete mFocusChangedCallback; @@ -131,6 +133,7 @@ LLFocusMgr::LLFocusMgr() mKeyboardFocus( NULL ), mLastKeyboardFocus( NULL ), mDefaultKeyboardFocus( NULL ), + mLastDefaultKeyboardFocus( NULL ), mKeystrokesOnly(FALSE), mTopCtrl( NULL ), mAppHasFocus(TRUE), // Macs don't seem to notify us that we've gotten focus, so default to true @@ -171,6 +174,23 @@ void LLFocusMgr::releaseFocusIfNeeded( const LLView* view ) } } +void LLFocusMgr::restoreDefaultKeyboardFocus(LLFocusableElement* current_default_focus) +{ + if (current_default_focus && mDefaultKeyboardFocus == current_default_focus) + { + setDefaultKeyboardFocus(mLastDefaultKeyboardFocus); + mLastDefaultKeyboardFocus = NULL; + } +} + +void LLFocusMgr::restoreKeyboardFocus(LLFocusableElement* current_focus) +{ + if (current_focus && mKeyboardFocus == current_focus) + { + setKeyboardFocus(mLastKeyboardFocus); + mLastKeyboardFocus = NULL; + } +} void LLFocusMgr::setKeyboardFocus(LLFocusableElement* new_focus, BOOL lock, BOOL keystrokes_only) { @@ -328,11 +348,22 @@ void LLFocusMgr::removeKeyboardFocusWithoutCallback( const LLFocusableElement* f { mLockedView = NULL; } - - if( mKeyboardFocus == focus ) + if (mKeyboardFocus == focus) { mKeyboardFocus = NULL; } + if (mLastKeyboardFocus == focus) + { + mLastKeyboardFocus = NULL; + } + if (mDefaultKeyboardFocus == focus) + { + mDefaultKeyboardFocus = NULL; + } + if (mLastDefaultKeyboardFocus == focus) + { + mLastDefaultKeyboardFocus = NULL; + } } diff --git a/indra/llui/llfocusmgr.h b/indra/llui/llfocusmgr.h index 14c711b5e..5f1466f5d 100644 --- a/indra/llui/llfocusmgr.h +++ b/indra/llui/llfocusmgr.h @@ -84,6 +84,7 @@ public: // Keyboard Focus void setKeyboardFocus(LLFocusableElement* new_focus, BOOL lock = FALSE, BOOL keystrokes_only = FALSE); // new_focus = NULL to release the focus. + void restoreKeyboardFocus(LLFocusableElement* current_focus); LLFocusableElement* getKeyboardFocus() const { return mKeyboardFocus; } LLFocusableElement* getLastKeyboardFocus() const { return mLastKeyboardFocus; } BOOL childHasKeyboardFocus( const LLView* parent ) const; @@ -103,7 +104,8 @@ public: // If setKeyboardFocus(NULL) is called, and there is a non-NULL default // keyboard focus view, focus goes there. JC - void setDefaultKeyboardFocus(LLFocusableElement* default_focus) { mDefaultKeyboardFocus = default_focus; } + void setDefaultKeyboardFocus(LLFocusableElement* default_focus) { mLastDefaultKeyboardFocus = mDefaultKeyboardFocus; mDefaultKeyboardFocus = default_focus; } + void restoreDefaultKeyboardFocus(LLFocusableElement* current_default_focus); LLFocusableElement* getDefaultKeyboardFocus() const { return mDefaultKeyboardFocus; } @@ -132,6 +134,7 @@ private: LLFocusableElement* mKeyboardFocus; // Keyboard events are preemptively routed to this object LLFocusableElement* mLastKeyboardFocus; // who last had focus LLFocusableElement* mDefaultKeyboardFocus; + LLFocusableElement* mLastDefaultKeyboardFocus; BOOL mKeystrokesOnly; // Top View diff --git a/indra/newview/llfloatersnapshot.cpp b/indra/newview/llfloatersnapshot.cpp index 735b033b1..d3f4ed3f6 100644 --- a/indra/newview/llfloatersnapshot.cpp +++ b/indra/newview/llfloatersnapshot.cpp @@ -345,7 +345,6 @@ public: LLToolset* mLastToolset; boost::signals2::connection mQualityMouseUpConnection; - LLFocusableElement* mPrevDefaultKeyboardFocus; }; //---------------------------------------------------------------------------- @@ -1716,14 +1715,12 @@ void LLFloaterSnapshot::Impl::freezeTime(bool on) } // Make sure the floater keeps focus so that pressing ESC stops Freeze Time mode. - sInstance->impl.mPrevDefaultKeyboardFocus = gFocusMgr.getDefaultKeyboardFocus(); gFocusMgr.setDefaultKeyboardFocus(sInstance); } else if (gSavedSettings.getBOOL("FreezeTime")) // turning off freeze frame mode { // Restore default keyboard focus. - gFocusMgr.setDefaultKeyboardFocus(sInstance->impl.mPrevDefaultKeyboardFocus); - sInstance->impl.mPrevDefaultKeyboardFocus = NULL; + gFocusMgr.restoreDefaultKeyboardFocus(sInstance); gSnapshotFloaterView->setMouseOpaque(FALSE); From 2a177c6614cb3be21728923c9f128b663e36bbee Mon Sep 17 00:00:00 2001 From: Aleric Inglewood Date: Sun, 10 Feb 2013 18:18:50 +0100 Subject: [PATCH 10/16] Fix for libcwd configured with --disable-location --- indra/cwdebug/debug.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/indra/cwdebug/debug.h b/indra/cwdebug/debug.h index 329f0448e..6c6d71908 100644 --- a/indra/cwdebug/debug.h +++ b/indra/cwdebug/debug.h @@ -173,8 +173,8 @@ extern LL_COMMON_API fake_channel const notice; #include #if CWDEBUG_LOCATION #include // Needed for 'backtrace'. -#include "llpreprocessor.h" #endif +#include "llpreprocessor.h" // LL_COMMON_API #include #define CWD_API __attribute__ ((visibility("default"))) From c18b156d8b5df7668b305e32b47652bdbdd329ca Mon Sep 17 00:00:00 2001 From: Aleric Inglewood Date: Mon, 11 Feb 2013 03:36:46 +0100 Subject: [PATCH 11/16] Bug fix for scrolling folder views. An LLFolderView is added as child to LLScrollableContainerView, but also adds the LLScrollableContainerView to it's mScrollContainer. As a result, when scrolling inside a LLFolderView the event is passed to the mScrollContainer, which then passes it first on to it's children (see the "Bad UI design" remark in the code), causing an infinite loop. This patch breaks that loop for those objects that have a mScrollContainer: LLFolderView and LLContainerView. --- indra/llui/llscrollcontainer.cpp | 5 +++-- indra/llui/llscrollcontainer.h | 2 ++ indra/newview/llcontainerview.cpp | 7 +++++++ indra/newview/llcontainerview.h | 2 +- indra/newview/llfolderview.cpp | 6 ++++++ indra/newview/llfolderview.h | 2 +- 6 files changed, 20 insertions(+), 4 deletions(-) diff --git a/indra/llui/llscrollcontainer.cpp b/indra/llui/llscrollcontainer.cpp index 8e740384a..1206d3b76 100644 --- a/indra/llui/llscrollcontainer.cpp +++ b/indra/llui/llscrollcontainer.cpp @@ -77,7 +77,8 @@ LLScrollableContainerView::LLScrollableContainerView( const std::string& name, mReserveScrollCorner( FALSE ), mMinAutoScrollRate( MIN_AUTO_SCROLL_RATE ), mMaxAutoScrollRate( MAX_AUTO_SCROLL_RATE ), - mScrolledView( scrolled_view ) + mScrolledView( scrolled_view ), + mPassBackToChildren(true) { if( mScrolledView ) { @@ -218,7 +219,7 @@ BOOL LLScrollableContainerView::handleScrollWheel( S32 x, S32 y, S32 clicks ) { // Give event to my child views - they may have scroll bars // (Bad UI design, but technically possible.) - if (LLUICtrl::handleScrollWheel(x,y,clicks)) + if (mPassBackToChildren && LLUICtrl::handleScrollWheel(x,y,clicks)) return TRUE; // When the vertical scrollbar is visible, scroll wheel diff --git a/indra/llui/llscrollcontainer.h b/indra/llui/llscrollcontainer.h index a5002a341..2e1cf1dd3 100644 --- a/indra/llui/llscrollcontainer.h +++ b/indra/llui/llscrollcontainer.h @@ -70,6 +70,7 @@ public: virtual void setValue(const LLSD& value) { mInnerRect.setValue(value); } void setBorderVisible( BOOL b ); + void setPassBackToChildren(bool b) { mPassBackToChildren = b; } void scrollToShowRect( const LLRect& rect, const LLRect& constraint); void scrollToShowRect( const LLRect& rect) { scrollToShowRect(rect, LLRect(0, mInnerRect.getHeight(), mInnerRect.getWidth(), 0)); } @@ -128,6 +129,7 @@ private: F32 mMinAutoScrollRate; F32 mMaxAutoScrollRate; bool mHideScrollbar; + bool mPassBackToChildren; }; diff --git a/indra/newview/llcontainerview.cpp b/indra/newview/llcontainerview.cpp index be7be72c7..e6d9b0119 100644 --- a/indra/newview/llcontainerview.cpp +++ b/indra/newview/llcontainerview.cpp @@ -278,3 +278,10 @@ void LLContainerView::setDisplayChildren(const BOOL displayChildren) childp->setVisible(mDisplayChildren); } } + +void LLContainerView::setScrollContainer(LLScrollableContainerView* scroll) +{ + mScrollContainer = scroll; + scroll->setPassBackToChildren(false); +} + diff --git a/indra/newview/llcontainerview.h b/indra/newview/llcontainerview.h index 37c2d97f3..1b337b09b 100644 --- a/indra/newview/llcontainerview.h +++ b/indra/newview/llcontainerview.h @@ -61,7 +61,7 @@ public: void showLabel(BOOL show) { mShowLabel = show; } void setDisplayChildren(const BOOL displayChildren); BOOL getDisplayChildren() { return mDisplayChildren; } - void setScrollContainer(LLScrollableContainerView* scroll) {mScrollContainer = scroll;} + void setScrollContainer(LLScrollableContainerView* scroll); private: LLScrollableContainerView* mScrollContainer; diff --git a/indra/newview/llfolderview.cpp b/indra/newview/llfolderview.cpp index bea49eb6b..e474dec9e 100644 --- a/indra/newview/llfolderview.cpp +++ b/indra/newview/llfolderview.cpp @@ -2068,6 +2068,12 @@ void LLFolderView::scrollToShowItem(LLFolderViewItem* item, const LLRect& constr } } +void LLFolderView::setScrollContainer(LLScrollableContainerView* parent) +{ + mScrollContainer = parent; + parent->setPassBackToChildren(false); +} + LLRect LLFolderView::getVisibleRect() { S32 visible_height = mScrollContainer->getRect().getHeight(); diff --git a/indra/newview/llfolderview.h b/indra/newview/llfolderview.h index 0d36bfe3b..c2771ed77 100644 --- a/indra/newview/llfolderview.h +++ b/indra/newview/llfolderview.h @@ -217,7 +217,7 @@ public: void scrollToShowSelection(); void scrollToShowItem(LLFolderViewItem* item, const LLRect& constraint_rect); - void setScrollContainer( LLScrollableContainerView* parent ) { mScrollContainer = parent; } + void setScrollContainer(LLScrollableContainerView* parent); LLRect getVisibleRect(); BOOL search(LLFolderViewItem* first_item, const std::string &search_string, BOOL backward); From b1892eb238a646e2b540b59b517e8a4fde5b1819 Mon Sep 17 00:00:00 2001 From: Aleric Inglewood Date: Mon, 11 Feb 2013 20:08:58 +0100 Subject: [PATCH 12/16] DoutEntering was hiding variable 'on' --- indra/cwdebug/debug.h | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/indra/cwdebug/debug.h b/indra/cwdebug/debug.h index 6c6d71908..9ca1886da 100644 --- a/indra/cwdebug/debug.h +++ b/indra/cwdebug/debug.h @@ -387,21 +387,21 @@ void InstanceTracker::dump(void) // Print "Entering " << \a data to channel \a cntrl and increment // debugging output indentation until the end of the current scope. #define DoutEntering(cntrl, data) \ - int __slviewer_debug_indentation = 2; \ + int __slviewer_debug_indentation = 2; \ { \ LIBCWD_TSD_DECLARATION; \ if (LIBCWD_DO_TSD_MEMBER_OFF(::libcwd::libcw_do) < 0) \ { \ ::libcwd::channel_set_bootstrap_st __libcwd_channel_set(LIBCWD_DO_TSD(::libcwd::libcw_do) LIBCWD_COMMA_TSD); \ - bool on; \ + bool __slviewer_debug_on; \ { \ using namespace LIBCWD_DEBUGCHANNELS; \ - on = (__libcwd_channel_set|cntrl).on; \ + __slviewer_debug_on = (__libcwd_channel_set|cntrl).on; \ } \ - if (on) \ + if (__slviewer_debug_on) \ Dout(cntrl, "Entering " << data); \ else \ - __slviewer_debug_indentation = 0; \ + __slviewer_debug_indentation = 0; \ } \ } \ debug::Indent __slviewer_debug_indent(__slviewer_debug_indentation); From b40e8fb508ea0f5350c8b66f1eae49f2406bc185 Mon Sep 17 00:00:00 2001 From: Aleric Inglewood Date: Tue, 12 Feb 2013 22:21:39 +0100 Subject: [PATCH 13/16] Add checkbox "Keep aspect ratio" to snapshot floater. --- indra/newview/app_settings/settings.xml | 46 +- indra/newview/llfloatersnapshot.cpp | 462 ++++++++++-------- .../default/xui/en-us/floater_snapshot.xml | 28 +- 3 files changed, 312 insertions(+), 224 deletions(-) diff --git a/indra/newview/app_settings/settings.xml b/indra/newview/app_settings/settings.xml index 75eea9830..aef7f4e17 100644 --- a/indra/newview/app_settings/settings.xml +++ b/indra/newview/app_settings/settings.xml @@ -8732,7 +8732,7 @@ This should be as low as possible, but too low may break functionality LastSnapshotType Comment - Select this as next type of snapshot to take (0 = postcard, 1 = texture, 2 = local image) + Select this as next type of snapshot to take (0 = feed, 1 = postcard, 2 = texture, 3 = local image) Persist 1 Type @@ -14185,6 +14185,50 @@ This should be as low as possible, but too low may break functionality Value 0 + SnapshotFeedKeepAspect + + Comment + When adjusting feed resolution keep its aspect ratio constant and equal to the target aspect. + Persist + 1 + Type + Boolean + Value + 0 + + SnapshotPostcardKeepAspect + + Comment + When adjusting postcard resolution keep its aspect ratio constant and equal to the target aspect. + Persist + 1 + Type + Boolean + Value + 0 + + SnapshotTextureKeepAspect + + Comment + When adjusting texture resolution keep its aspect ratio constant and equal to the target aspect. + Persist + 1 + Type + Boolean + Value + 0 + + SnapshotLocalKeepAspect + + Comment + When adjusting local resolution keep its aspect ratio constant and equal to the target aspect. + Persist + 1 + Type + Boolean + Value + 0 + SnapshotOpenFreezeTime Comment diff --git a/indra/newview/llfloatersnapshot.cpp b/indra/newview/llfloatersnapshot.cpp index d3f4ed3f6..1ef53b269 100644 --- a/indra/newview/llfloatersnapshot.cpp +++ b/indra/newview/llfloatersnapshot.cpp @@ -91,8 +91,8 @@ ///---------------------------------------------------------------------------- /// Local function declarations, constants, enums, and typedefs ///---------------------------------------------------------------------------- -S32 LLFloaterSnapshot::sUIWinHeightLong = 619 ; -S32 LLFloaterSnapshot::sUIWinHeightShort = LLFloaterSnapshot::sUIWinHeightLong - 260 ; +S32 LLFloaterSnapshot::sUIWinHeightLong = 625 ; +S32 LLFloaterSnapshot::sUIWinHeightShort = LLFloaterSnapshot::sUIWinHeightLong - 266 ; S32 LLFloaterSnapshot::sUIWinWidth = 219 ; S32 const THUMBHEIGHT = 159; @@ -111,6 +111,8 @@ S32 BORDER_WIDTH = 6; const S32 MAX_POSTCARD_DATASIZE = 1024 * 1024; // one megabyte const S32 MAX_TEXTURE_SIZE = 512 ; //max upload texture size 512 * 512 +static std::string snapshotKeepAspectName(); + ///---------------------------------------------------------------------------- /// Class LLSnapshotLivePreview ///---------------------------------------------------------------------------- @@ -172,6 +174,7 @@ public: BOOL getThumbnailUpToDate() const { return mThumbnailUpToDate ;} bool getShowFreezeFrameSnapshot() const { return mShowFreezeFrameSnapshot; } LLViewerTexture* getCurrentImage(); + char const* resolutionComboName() const; char const* aspectComboName() const; void setSnapshotType(ESnapshotType type) { mSnapshotType = type; } @@ -305,6 +308,7 @@ public: static void onClickUICheck(LLUICtrl *ctrl, void* data); static void onClickHUDCheck(LLUICtrl *ctrl, void* data); static void onClickKeepOpenCheck(LLUICtrl *ctrl, void* data); + static void onClickKeepAspect(LLUICtrl* ctrl, void* data); static void onCommitQuality(LLUICtrl* ctrl, void* data); static void onCommitFeedResolution(LLUICtrl* ctrl, void* data); static void onCommitPostcardResolution(LLUICtrl* ctrl, void* data); @@ -327,10 +331,14 @@ public: static LLSnapshotLivePreview* getPreviewView(void); static void setResolution(LLFloaterSnapshot* floater, const std::string& comboname, bool visible, bool update_controls = true); static void setAspect(LLFloaterSnapshot* floater, const std::string& comboname, bool update_controls = true); + static void storeAspectSetting(LLComboBox* combo, const std::string& comboname); + static void enforceAspect(LLFloaterSnapshot* floater, F32 new_aspect); + static void enforceResolution(LLFloaterSnapshot* floater, F32 new_aspect); static void updateControls(LLFloaterSnapshot* floater, bool delayed_formatted = false); static void resetFeedAndPostcardAspect(LLFloaterSnapshot* floater); static void updateLayout(LLFloaterSnapshot* floater); static void freezeTime(bool on); + static void keepAspect(LLFloaterSnapshot* view, bool on, bool force = false); static LLHandle sPreviewHandle; @@ -403,7 +411,7 @@ LLSnapshotLivePreview::LLSnapshotLivePreview (const LLRect& rect) : mNeedsFlash(TRUE), mSnapshotQuality(gSavedSettings.getS32("SnapshotQuality")), mFormattedDataSize(0), - mSnapshotType(SNAPSHOT_FEED), + mSnapshotType((ESnapshotType)gSavedSettings.getS32("LastSnapshotType")), mSnapshotFormat(LLFloaterSnapshot::ESnapshotFormat(gSavedSettings.getS32("SnapshotFormat"))), mShowFreezeFrameSnapshot(FALSE), mCameraPos(LLViewerCamera::getInstance()->getOrigin()), @@ -1800,7 +1808,6 @@ void LLFloaterSnapshot::Impl::updateControls(LLFloaterSnapshot* floater, bool de LLSnapshotLivePreview* previewp = getPreviewView(); if (previewp) { - previewp->setSnapshotType(shot_type); LLViewerWindow::ESnapshotType layer_type = (shot_type == LLSnapshotLivePreview::SNAPSHOT_LOCAL) ? (LLViewerWindow::ESnapshotType)gSavedSettings.getS32("SnapshotLayerType") : @@ -1841,6 +1848,7 @@ void LLFloaterSnapshot::Impl::updateControls(LLFloaterSnapshot* floater, bool de floater->childSetVisible("more_btn", !is_advance); // the only item hidden in advanced mode floater->childSetVisible("less_btn", is_advance); floater->childSetVisible("type_label2", is_advance); + floater->childSetVisible("keep_aspect", is_advance); floater->childSetVisible("type_label3", is_advance); floater->childSetVisible("format_label", is_advance && is_local); floater->childSetVisible("local_format_combo", is_local); @@ -1896,6 +1904,8 @@ void LLFloaterSnapshot::Impl::updateControls(LLFloaterSnapshot* floater, bool de shot_type == LLSnapshotLivePreview::SNAPSHOT_POSTCARD && got_bytes && previewp->getDataSize() > MAX_POSTCARD_DATASIZE ? LLColor4::red : gColors.getColor( "LabelTextColor" )); + std::string target_size_str = gSavedSettings.getBOOL(snapshotKeepAspectName()) ? floater->getString("sourceAR") : floater->getString("targetAR"); + floater->childSetValue("type_label3", target_size_str); bool up_to_date = previewp && previewp->getSnapshotUpToDate(); bool can_upload = up_to_date && !previewp->isUsedBy(shot_type); @@ -1984,6 +1994,11 @@ void LLFloaterSnapshot::Impl::onCommitFeedAspect(LLUICtrl* ctrl, void* data) previewp->addManualOverride(LLSnapshotLivePreview::SNAPSHOT_FEED); } updateAspect(ctrl, data); + LLFloaterSnapshot* floater = (LLFloaterSnapshot*)data; + if (floater && previewp && gSavedSettings.getBOOL(snapshotKeepAspectName())) + { + enforceResolution(floater, previewp->getAspect()); + } } // static @@ -1997,6 +2012,11 @@ void LLFloaterSnapshot::Impl::onCommitPostcardAspect(LLUICtrl* ctrl, void* data) previewp->addManualOverride(LLSnapshotLivePreview::SNAPSHOT_POSTCARD); } updateAspect(ctrl, data); + LLFloaterSnapshot* floater = (LLFloaterSnapshot*)data; + if (floater && previewp && gSavedSettings.getBOOL(snapshotKeepAspectName())) + { + enforceResolution(floater, previewp->getAspect()); + } } // static @@ -2023,6 +2043,11 @@ void LLFloaterSnapshot::Impl::onCommitLocalAspect(LLUICtrl* ctrl, void* data) previewp->addManualOverride(LLSnapshotLivePreview::SNAPSHOT_LOCAL); } updateAspect(ctrl, data); + LLFloaterSnapshot* floater = (LLFloaterSnapshot*)data; + if (floater && previewp && gSavedSettings.getBOOL(snapshotKeepAspectName())) + { + enforceResolution(floater, previewp->getAspect()); + } } // static @@ -2249,6 +2274,18 @@ void LLFloaterSnapshot::Impl::onClickKeepOpenCheck(LLUICtrl* ctrl, void* data) gSavedSettings.setBOOL( "CloseSnapshotOnKeep", !check->get() ); } +// static +void LLFloaterSnapshot::Impl::onClickKeepAspect(LLUICtrl* ctrl, void* data) +{ + LLFloaterSnapshot* view = (LLFloaterSnapshot*)data; + if (view) + { + LLCheckBoxCtrl* check = (LLCheckBoxCtrl*)ctrl; + keepAspect(view, check->get()); + updateControls(view); + } +} + // static void LLFloaterSnapshot::Impl::onCommitQuality(LLUICtrl* ctrl, void* data) { @@ -2318,6 +2355,46 @@ static std::string lastSnapshotAspectName() } } +static std::string snapshotKeepAspectName() +{ + switch(gSavedSettings.getS32("LastSnapshotType")) + { + case LLSnapshotLivePreview::SNAPSHOT_FEED: return "SnapshotFeedKeepAspect"; + case LLSnapshotLivePreview::SNAPSHOT_POSTCARD: return "SnapshotPostcardKeepAspect"; + case LLSnapshotLivePreview::SNAPSHOT_TEXTURE: return "SnapshotTextureKeepAspect"; + default: return "SnapshotLocalKeepAspect"; + } +} + +// static +void LLFloaterSnapshot::Impl::keepAspect(LLFloaterSnapshot* view, bool on, bool force) +{ + DoutEntering(dc::snapshot, "LLFloaterSnapshot::Impl::keepAspect(view, " << on << ", " << force << ")"); + bool cur_on = gSavedSettings.getBOOL(snapshotKeepAspectName()); + if ((!force && cur_on == on) || + gSavedSettings.getBOOL("RenderUIInSnapshot") || + gSavedSettings.getBOOL("RenderHUDInSnapshot")) + { + // No change. + return; + } + view->childSetValue("keep_aspect", on); + gSavedSettings.setBOOL(snapshotKeepAspectName(), on); + if (on) + { + LLSnapshotLivePreview* previewp = getPreviewView(); + if (previewp) + { + S32 w = llround(view->childGetValue("snapshot_width").asReal(), 1.0); + S32 h = llround(view->childGetValue("snapshot_height").asReal(), 1.0); + gSavedSettings.setS32(lastSnapshotWidthName(), w); + gSavedSettings.setS32(lastSnapshotHeightName(), h); + comboSetCustom(view, previewp->resolutionComboName()); + enforceAspect(view, (F32)w / h); + } + } +} + // static void LLFloaterSnapshot::Impl::updateResolution(LLUICtrl* ctrl, void* data, bool update_controls) { @@ -2329,192 +2406,12 @@ void LLFloaterSnapshot::Impl::updateResolution(LLUICtrl* ctrl, void* data, bool return; } - S32 new_width = 0; - S32 new_height = 0; - F32 new_aspect = 0; LLSnapshotLivePreview* previewp = getPreviewView(); -#if 0 // Broken -- not doing this for now. - LLSnapshotLivePreview::ESnapshotType shot_type = (LLSnapshotLivePreview::ESnapshotType)gSavedSettings.getS32("LastSnapshotType"); - // Is the snapshot was already used (saved or uploaded) and no manual changes - // have been made since to the current destination type, and the raw snapshot - // is still up to date with regard to visibility of UI, HUD and BufferType etc? - if (previewp && previewp->isUsed() && !previewp->isOverriddenBy(shot_type) && previewp->getRawSnapshotUpToDate()) - { - previewp->getSize(new_width, new_height); - new_aspect = previewp->getAspect(); - S32 const old_width = new_width; - S32 const old_height = new_height; - F32 const old_aspect = new_aspect; - S32 raw_width; - S32 raw_height; - previewp->getRawSize(raw_width, raw_height); - F32 const raw_aspect = (F32)raw_width / raw_height; - bool fixed_crop = false; - bool fixed_size = false; - bool fixed_scale = false; - bool done = false; - bool fail = false; - while (!done) - { - // Attempt to change the size and aspect so the raw snapshot can also be used for this new destination. - S32 w, h, crop_offset; - bool crop_vertically; - previewp->setSize(new_width, new_height); - previewp->setAspect(new_aspect); - LLSnapshotLivePreview::EAspectSizeProblem ret = previewp->getAspectSizeProblem(w, h, crop_vertically, crop_offset); - switch(ret) - { - case LLSnapshotLivePreview::ASPECTSIZE_OK: - done = true; // Don't change anything (else) if the current settings are usable. - break; - case LLSnapshotLivePreview::CANNOT_CROP_HORIZONTALLY: - case LLSnapshotLivePreview::CANNOT_CROP_VERTICALLY: - if (fixed_crop) - { - done = fail = true; - } - else - { - // Set target aspect to aspect of the raw snapshot we have (no reason to crop anything). - new_aspect = raw_aspect; - fixed_crop = true; - } - break; - case LLSnapshotLivePreview::SIZE_TOO_LARGE: - if (fixed_size) - { - done = fail = true; - } - else - { - if (new_width > w) - { - new_width = w; - new_height = llround(new_width / new_aspect); - } - if (new_height > h) - { - new_width = llmin(w, llround(h * new_aspect)); - new_height = llmin(h, llround(new_width / new_aspect)); - } - fixed_size = true; - } - break; - case LLSnapshotLivePreview::CANNOT_RESIZE: - if (fixed_scale) - { - done = fail = true; - } - else - { - F32 ratio = llmin((F32)w / new_width, (F32)h / new_height); - new_width = llround(new_width * ratio); - new_height = llround(new_height * ratio); - fixed_scale = true; - } - break; - } - } - previewp->setAspect(old_aspect); - previewp->setSize(old_width, old_height); - if (fail) - { - new_aspect = 0; - new_width = new_height = 0; - } - else - { - LLComboBox* size_combo_box = NULL; - LLComboBox* aspect_combo_box = NULL; - switch(shot_type) - { - case LLSnapshotLivePreview::SNAPSHOT_FEED: - size_combo_box = view->getChild("feed_size_combo"); - aspect_combo_box = view->getChild("feed_aspect_combo"); - break; - case LLSnapshotLivePreview::SNAPSHOT_POSTCARD: - size_combo_box = view->getChild("postcard_size_combo"); - aspect_combo_box = view->getChild("postcard_aspect_combo"); - break; - case LLSnapshotLivePreview::SNAPSHOT_TEXTURE: - size_combo_box = view->getChild("texture_size_combo"); - aspect_combo_box = view->getChild("texture_aspect_combo"); - break; - case LLSnapshotLivePreview::SNAPSHOT_LOCAL: - size_combo_box = view->getChild("local_size_combo"); - aspect_combo_box = view->getChild("local_aspect_combo"); - break; - } - S32 index = 0; - S32 const size_custom = size_combo_box->getItemCount() - (shot_type == LLSnapshotLivePreview::SNAPSHOT_TEXTURE ? 0 : 1); // Texture does not end on 'Custom'. - while (index < size_custom) - { - size_combo_box->setCurrentByIndex(index); - std::string sdstring = size_combo_box->getSelectedValue(); - LLSD sdres; - std::stringstream sstream(sdstring); - LLSDSerialize::fromNotation(sdres, sstream, sdstring.size()); - S32 width = sdres[0]; - S32 height = sdres[1]; - if (width == 0 || height == 0) - { - width = gViewerWindow->getWindowDisplayWidth(); - height = gViewerWindow->getWindowDisplayHeight(); - } - if (width == new_width && height == new_height) - { - break; - } - ++index; - } - if (index == size_custom && shot_type != LLSnapshotLivePreview::SNAPSHOT_TEXTURE) - { - size_combo_box->setCurrentByIndex(index); - } - index = 0; - S32 const aspect_custom = aspect_combo_box->getItemCount() - 1; - while (index < aspect_custom) - { - aspect_combo_box->setCurrentByIndex(index); - std::string sdstring = aspect_combo_box->getSelectedValue(); - std::stringstream sstream; - sstream << sdstring; - F32 aspect; - sstream >> aspect; - if (aspect == -2) // Default - { - aspect = (F32)new_width / new_height; - } - if (aspect == 0) // Current window - { - aspect = (F32)gViewerWindow->getWindowDisplayWidth() / gViewerWindow->getWindowDisplayHeight(); - } - if (llabs(aspect - new_aspect) < 0.0001) - { - break; - } - ++index; - } - if (index == aspect_custom) - { - aspect_combo_box->setCurrentByIndex(index); - setAspect(view, previewp->aspectComboName(), update_controls); - } - } - } - else -#endif - { - view->getChild("feed_size_combo")->selectNthItem(gSavedSettings.getS32("SnapshotFeedLastResolution")); - view->getChild("feed_aspect_combo")->selectNthItem(gSavedSettings.getS32("SnapshotFeedLastAspect")); - view->getChild("postcard_size_combo")->selectNthItem(gSavedSettings.getS32("SnapshotPostcardLastResolution")); - view->getChild("postcard_aspect_combo")->selectNthItem(gSavedSettings.getS32("SnapshotPostcardLastAspect")); - view->getChild("texture_size_combo")->selectNthItem(gSavedSettings.getS32("SnapshotTextureLastResolution")); - view->getChild("texture_aspect_combo")->selectNthItem(gSavedSettings.getS32("SnapshotTextureLastAspect")); - view->getChild("local_size_combo")->selectNthItem(gSavedSettings.getS32("SnapshotLocalLastResolution")); - view->getChild("local_aspect_combo")->selectNthItem(gSavedSettings.getS32("SnapshotLocalLastAspect")); - } + view->getChild("feed_size_combo")->selectNthItem(gSavedSettings.getS32("SnapshotFeedLastResolution")); + view->getChild("postcard_size_combo")->selectNthItem(gSavedSettings.getS32("SnapshotPostcardLastResolution")); + view->getChild("texture_size_combo")->selectNthItem(gSavedSettings.getS32("SnapshotTextureLastResolution")); + view->getChild("local_size_combo")->selectNthItem(gSavedSettings.getS32("SnapshotLocalLastResolution")); std::string sdstring = combobox->getSelectedValue(); LLSD sdres; @@ -2523,6 +2420,12 @@ void LLFloaterSnapshot::Impl::updateResolution(LLUICtrl* ctrl, void* data, bool S32 width = sdres[0]; S32 height = sdres[1]; + + if (width != -1 && height != -1) + { + // Not "Custom". + keepAspect(view, false); + } if (previewp && combobox->getCurrentIndex() >= 0) { @@ -2535,15 +2438,8 @@ void LLFloaterSnapshot::Impl::updateResolution(LLUICtrl* ctrl, void* data, bool } else if (width == -1 || height == -1) { - if (previewp->isUsed() && new_width != 0 && new_height != 0) - { - previewp->setSize(new_width, new_height); - } - else - { - // load last custom value - previewp->setSize(gSavedSettings.getS32(lastSnapshotWidthName()), gSavedSettings.getS32(lastSnapshotHeightName())); - } + // load last custom value + previewp->setSize(gSavedSettings.getS32(lastSnapshotWidthName()), gSavedSettings.getS32(lastSnapshotHeightName())); } else if (height == -2) { @@ -2594,17 +2490,8 @@ void LLFloaterSnapshot::Impl::updateResolution(LLUICtrl* ctrl, void* data, bool if (previewp) { - if (new_aspect == 0) - { - // In case the aspect is 'Default', need to update aspect (which will call updateControls, if necessary). - setAspect(view, previewp->aspectComboName(), update_controls); - } - else - { - LLSpinCtrl* aspect_spinner = view->getChild("aspect_ratio"); - aspect_spinner->set(new_aspect); - previewp->setAspect(new_aspect); - } + // In case the aspect is 'Default', need to update aspect (which will call updateControls, if necessary). + setAspect(view, previewp->aspectComboName(), update_controls); } } @@ -2637,6 +2524,8 @@ void LLFloaterSnapshot::Impl::updateAspect(LLUICtrl* ctrl, void* data, bool upda S32 width, height; previewp->getSize(width, height); aspect = (F32)width / height; + // Turn off "Keep aspect" when aspect is set to Default. + keepAspect(view, false); } else if (aspect == -1) // Custom { @@ -2648,6 +2537,7 @@ void LLFloaterSnapshot::Impl::updateAspect(LLUICtrl* ctrl, void* data, bool upda } LLSpinCtrl* aspect_spinner = view->getChild("aspect_ratio"); + LLCheckBoxCtrl* keep_aspect = view->getChild("keep_aspect"); // Set whether or not the spinners can be changed. if (gSavedSettings.getBOOL("RenderUIInSnapshot") || @@ -2657,11 +2547,13 @@ void LLFloaterSnapshot::Impl::updateAspect(LLUICtrl* ctrl, void* data, bool upda // Disable without making label gray. aspect_spinner->setAllowEdit(FALSE); aspect_spinner->setIncrement(0); + keep_aspect->setEnabled(FALSE); } else { aspect_spinner->setAllowEdit(TRUE); aspect_spinner->setIncrement(llmax(0.01f, lltrunc(aspect) / 100.0f)); + keep_aspect->setEnabled(TRUE); } // Sync the spinner and cache value. @@ -2682,6 +2574,77 @@ void LLFloaterSnapshot::Impl::updateAspect(LLUICtrl* ctrl, void* data, bool upda } } +// static +void LLFloaterSnapshot::Impl::enforceAspect(LLFloaterSnapshot* floater, F32 new_aspect) +{ + LLSnapshotLivePreview* previewp = getPreviewView(); + if (previewp) + { + LLComboBox* combo = floater->getChild(previewp->aspectComboName()); + S32 const aspect_custom = combo->getItemCount() - 1; + for (S32 index = 0; index <= aspect_custom; ++index) + { + combo->setCurrentByIndex(index); + if (index == aspect_custom) + { + gSavedSettings.setF32(lastSnapshotAspectName(), new_aspect); + break; + } + std::string sdstring = combo->getSelectedValue(); + std::stringstream sstream; + sstream << sdstring; + F32 aspect; + sstream >> aspect; + if (aspect == -2) // Default + { + continue; + } + if (aspect == 0) // Current window + { + aspect = (F32)gViewerWindow->getWindowDisplayWidth() / gViewerWindow->getWindowDisplayHeight(); + } + if (llabs(aspect - new_aspect) < 0.0001) + { + break; + } + } + storeAspectSetting(combo, previewp->aspectComboName()); + updateAspect(combo, floater, true); + } +} + +void LLFloaterSnapshot::Impl::enforceResolution(LLFloaterSnapshot* floater, F32 new_aspect) +{ + LLSnapshotLivePreview* previewp = getPreviewView(); + if (previewp) + { + // Get current size. + S32 w, h; + previewp->getSize(w, h); + // Do all calculation in floating point. + F32 cw = w; + F32 ch = h; + // Get the current raw size. + previewp->getRawSize(w, h); + F32 rw = w; + F32 rh = h; + // Fit rectangle with aspect new_aspect, around current rectangle, but cropped to raw size. + F32 nw = llmin(llmax(cw, ch * new_aspect), rw); + F32 nh = llmin(llmax(ch, cw / new_aspect), rh); + // Fit rectangle with aspect new_aspect inside that rectangle (in case it was cropped). + nw = llmin(nw, nh * new_aspect); + nh = llmin(nh, nw / new_aspect); + // Round off to nearest integer. + S32 new_width = llround(nw); + S32 new_height = llround(nh); + + gSavedSettings.setS32(lastSnapshotWidthName(), new_width); + gSavedSettings.setS32(lastSnapshotHeightName(), new_height); + comboSetCustom(floater, previewp->resolutionComboName()); + updateResolution(floater->getChild(previewp->resolutionComboName()), floater, false); + } +} + // static void LLFloaterSnapshot::Impl::onCommitLayerTypes(LLUICtrl* ctrl, void*data) { @@ -2700,13 +2663,20 @@ void LLFloaterSnapshot::Impl::onCommitSnapshotType(LLUICtrl* ctrl, void* data) LLFloaterSnapshot *view = (LLFloaterSnapshot *)data; if (view) { - gSavedSettings.setS32("LastSnapshotType", getTypeIndex(view)); + LLSnapshotLivePreview::ESnapshotType snapshot_type = getTypeIndex(view); + gSavedSettings.setS32("LastSnapshotType", snapshot_type); + LLSnapshotLivePreview* previewp = getPreviewView(); + if (previewp) + { + previewp->setSnapshotType(snapshot_type); + } + keepAspect(view, gSavedSettings.getBOOL(snapshotKeepAspectName()), true); updateControls(view); } } -//static +//static void LLFloaterSnapshot::Impl::onCommitSnapshotFormat(LLUICtrl* ctrl, void* data) { LLFloaterSnapshot *view = (LLFloaterSnapshot *)data; @@ -2726,7 +2696,12 @@ void LLFloaterSnapshot::Impl::comboSetCustom(LLFloaterSnapshot* floater, const s LLComboBox* combo = floater->getChild(comboname); combo->setCurrentByIndex(combo->getItemCount() - 1); // "custom" is always the last index + storeAspectSetting(combo, comboname); +} +// static +void LLFloaterSnapshot::Impl::storeAspectSetting(LLComboBox* combo, const std::string& comboname) +{ if(comboname == "feed_size_combo") { gSavedSettings.setS32("SnapshotFeedLastResolution", combo->getCurrentIndex()); @@ -2763,8 +2738,11 @@ void LLFloaterSnapshot::Impl::onCommitCustomResolution(LLUICtrl *ctrl, void* dat LLFloaterSnapshot *view = (LLFloaterSnapshot *)data; if (view) { - S32 w = llround(view->childGetValue("snapshot_width").asReal(), 1.0); - S32 h = llround(view->childGetValue("snapshot_height").asReal(), 1.0); + LLSpinCtrl* width_spinner = view->getChild("snapshot_width"); + LLSpinCtrl* height_spinner = view->getChild("snapshot_height"); + + S32 w = llround((F32)width_spinner->getValue().asReal(), 1.0f); + S32 h = llround((F32)height_spinner->getValue().asReal(), 1.0f); LLSnapshotLivePreview* previewp = getPreviewView(); if (previewp) @@ -2774,6 +2752,33 @@ void LLFloaterSnapshot::Impl::onCommitCustomResolution(LLUICtrl *ctrl, void* dat if (w != curw || h != curh) { + // Enforce multiple of 32 if the step was 32. + Dout(dc::snapshot, "w = " << w << "; curw = " << curw); + if (llabs(w - curw) == 32) + { + w = (w + 16) & -32; + Dout(dc::snapshot, "w = (w + 16) & -32 = " << w); + } + if (llabs(h - curh) == 32) + { + h = (h + 16) & -32; + } + if (gSavedSettings.getBOOL(snapshotKeepAspectName())) + { + F32 aspect = previewp->getAspect(); + if (h == curh) + { + // Width was changed. Change height to keep aspect constant. + h = llround(w / aspect); + } + else + { + // Height was changed. Change width to keep aspect constant. + w = llround(h * aspect); + } + } + width_spinner->forceSetValue(LLSD::Real(w)); + height_spinner->forceSetValue(LLSD::Real(h)); previewp->setMaxImageSize((S32)((LLSpinCtrl *)ctrl)->getMaxValue()) ; previewp->setSize(w,h); checkAutoSnapshot(previewp, FALSE); @@ -2791,6 +2796,30 @@ void LLFloaterSnapshot::Impl::onCommitCustomResolution(LLUICtrl *ctrl, void* dat } } +char const* LLSnapshotLivePreview::resolutionComboName() const +{ + char const* result; + switch(mSnapshotType) + { + case SNAPSHOT_FEED: + result = "feed_size_combo"; + break; + case SNAPSHOT_POSTCARD: + result = "postcard_size_combo"; + break; + case SNAPSHOT_TEXTURE: + result = "texture_size_combo"; + break; + case SNAPSHOT_LOCAL: + result = "local_size_combo"; + break; + default: + result= ""; + break; + } + return result; +} + char const* LLSnapshotLivePreview::aspectComboName() const { char const* result; @@ -2838,6 +2867,11 @@ void LLFloaterSnapshot::Impl::onCommitCustomAspect(LLUICtrl *ctrl, void* data) gSavedSettings.setF32(lastSnapshotAspectName(), a); + if (gSavedSettings.getBOOL(snapshotKeepAspectName())) + { + enforceResolution(view, a); + } + updateControls(view, true); } } @@ -2899,6 +2933,7 @@ BOOL LLFloaterSnapshot::postBuild() childSetCommitCallback("snapshot_height", Impl::onCommitCustomResolution, this); childSetCommitCallback("aspect_ratio", Impl::onCommitCustomAspect, this); + childSetCommitCallback("keep_aspect", Impl::onClickKeepAspect, this); childSetCommitCallback("ui_check", Impl::onClickUICheck, this); childSetValue("ui_check", gSavedSettings.getBOOL("RenderUIInSnapshot")); @@ -2949,8 +2984,9 @@ BOOL LLFloaterSnapshot::postBuild() Impl::sPreviewHandle = previewp->getHandle(); - impl.updateControls(this); + impl.keepAspect(sInstance, gSavedSettings.getBOOL(snapshotKeepAspectName()), true); impl.freezeTime(gSavedSettings.getBOOL("SnapshotOpenFreezeTime")); + impl.updateControls(this); return TRUE; } diff --git a/indra/newview/skins/default/xui/en-us/floater_snapshot.xml b/indra/newview/skins/default/xui/en-us/floater_snapshot.xml index 8142c0f2b..cb9d8cfda 100644 --- a/indra/newview/skins/default/xui/en-us/floater_snapshot.xml +++ b/indra/newview/skins/default/xui/en-us/floater_snapshot.xml @@ -53,16 +53,16 @@