From 6986ad357c622328064a5c1247678ed17b3fac76 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jaakko=20Kera=CC=88nen?= Date: Wed, 29 Jun 2016 11:32:40 +0300 Subject: [PATCH] Refactor|libappfw: More robust child widget virtualization for menus Since filtering is no longer a concern for ChildWidgetOrganizer, it can now use simpler and more robust virtualization logic. This corrects any issues when scrolling to arbitrary locations in the item set. However, handling varying item heights still requires a bit more finesse. --- .../de/framework/childwidgetorganizer.h | 44 +-- .../sdk/libappfw/src/childwidgetorganizer.cpp | 356 +++++------------- .../sdk/libappfw/src/widgets/menuwidget.cpp | 2 + doomsday/sdk/libcore/include/de/core/range.h | 1 - 4 files changed, 93 insertions(+), 310 deletions(-) diff --git a/doomsday/sdk/libappfw/include/de/framework/childwidgetorganizer.h b/doomsday/sdk/libappfw/include/de/framework/childwidgetorganizer.h index a0176eb48f..2bc62c2587 100644 --- a/doomsday/sdk/libappfw/include/de/framework/childwidgetorganizer.h +++ b/doomsday/sdk/libappfw/include/de/framework/childwidgetorganizer.h @@ -76,28 +76,6 @@ class LIBAPPFW_PUBLIC ChildWidgetOrganizer virtual void updateItemWidget(GuiWidget &widget, ui::Item const &item) = 0; }; - /** - * Filters out data items. - */ - class IFilter - { - public: - virtual ~IFilter() {} - - /** - * Determines whether an item should be accepted or ignored by the organizer. - * - * @param organizer Which organizer is asking the question. - * @param data Data context. - * @param item Item to check. - * - * @return @c true to accept item, @c false to ignore it. - */ - virtual bool isItemAccepted(ChildWidgetOrganizer const &organizer, - ui::Data const &data, - ui::Item const &item) const = 0; - }; - public: ChildWidgetOrganizer(GuiWidget &container); @@ -112,15 +90,6 @@ class LIBAPPFW_PUBLIC ChildWidgetOrganizer IWidgetFactory &widgetFactory() const; - /** - * Sets the object that decides whether items are accepted or ignored. - * - * @param filter Filtering object. - */ - void setFilter(IFilter const &filter); - - void unsetFilter(); - /** * Sets the data context of the organizer. If there was a previous context, * all widgets created for it are deleted from the container. The widgets @@ -140,17 +109,6 @@ class LIBAPPFW_PUBLIC ChildWidgetOrganizer ui::Item const *findItemForWidget(GuiWidget const &widget) const; - /** - * Filters all items according to the defined IFilter. Widgets are - * created and removed as needed according to the filter. - */ - void refilter(); - - /** - * Number of items currently being organized. This is affected by the filtering. - */ - dsize itemCount() const; - //- Child Widget Virtualization --------------------------------------------------------- /** @@ -166,6 +124,8 @@ class LIBAPPFW_PUBLIC ChildWidgetOrganizer */ void setVirtualizationEnabled(bool enabled); + void setVirtualTopEdge(Rule const &topEdge); + void setVisibleArea(Rule const &minimum, Rule const &maximum); bool virtualizationEnabled() const; diff --git a/doomsday/sdk/libappfw/src/childwidgetorganizer.cpp b/doomsday/sdk/libappfw/src/childwidgetorganizer.cpp index 0e32eae85e..75af7495e9 100644 --- a/doomsday/sdk/libappfw/src/childwidgetorganizer.cpp +++ b/doomsday/sdk/libappfw/src/childwidgetorganizer.cpp @@ -31,7 +31,6 @@ namespace internal { AlwaysAppend = 0x1, AlwaysPrepend = 0x2, - IgnoreFilter = 0x4, DefaultBehavior = 0, }; @@ -51,24 +50,24 @@ struct ChildWidgetOrganizer::Instance , DENG2_OBSERVES(ui::Data, OrderChange) , DENG2_OBSERVES(ui::Item, Change ) { + typedef Range PvsRange; + ui::Data const *dataItems = nullptr; - IFilter const *filter = nullptr; GuiWidget *container; IWidgetFactory *factory; - ui::DataPos firstAcceptedPos = 0; typedef QMap Mapping; typedef QMutableMapIterator MutableMappingIterator; Mapping mapping; ///< Maps items to corresponding widgets. bool virtualEnabled = false; + Rule const *virtualTop = nullptr; Rule const *virtualMin = nullptr; Rule const *virtualMax = nullptr; ConstantRule *virtualStrut = nullptr; ConstantRule *estimatedHeight = nullptr; int averageItemHeight = 0; - dsize virtualItemCount = 0; - Ranges virtualPvsRange; + PvsRange virtualPvs; QSet pendingStrutAdjust; Instance(Public *i, GuiWidget *c) @@ -79,6 +78,7 @@ struct ChildWidgetOrganizer::Instance ~Instance() { + releaseRef(virtualTop); releaseRef(virtualMin); releaseRef(virtualMax); releaseRef(virtualStrut); @@ -109,10 +109,10 @@ struct ChildWidgetOrganizer::Instance } } - Ranges itemRange() const + PvsRange itemRange() const { - Ranges range(0, dataItems->size()); - if (virtualEnabled) range = range.intersection(virtualPvsRange); + PvsRange range(0, dataItems->size()); + if (virtualEnabled) range = range.intersection(virtualPvs); return range; } @@ -129,15 +129,6 @@ struct ChildWidgetOrganizer::Instance ui::Item const &item = dataItems->at(pos); - if (filter && !behavior.testFlag(IgnoreFilter)) - { - if (!filter->isItemAccepted(self, *dataItems, item)) - { - // Skip this one. - return nullptr; - } - } - GuiWidget *w = factory->makeItemWidget(item, container); if (!w) return nullptr; // Unpresentable. @@ -249,37 +240,27 @@ struct ChildWidgetOrganizer::Instance } } - void dataItemAdded(ui::Data::Pos pos, ui::Item const &item) + void dataItemAdded(ui::Data::Pos pos, ui::Item const &) { - addItemWidget(pos); - - if (!filter) + if (!virtualEnabled) { - virtualItemCount = dataItems->size(); - updateVirtualHeight(); + addItemWidget(pos); } else { - if (filter->isItemAccepted(self, *dataItems, item)) + // Items added after the PVS can be handled purely virtually. Items + // before the PVS will cause the PVS range to be re-estimated. + if (pos < virtualPvs.end) { - ++virtualItemCount; - updateVirtualHeight(); + clearWidgets(); + virtualPvs = PvsRange(); } + updateVirtualHeight(); } } void dataItemRemoved(ui::Data::Pos pos, ui::Item &item) { - if (pos < firstAcceptedPos) - { - --firstAcceptedPos; - } - if (!filter || filter->isItemAccepted(self, *dataItems, item)) - { - --virtualItemCount; - updateVirtualHeight(); - } - Mapping::iterator found = mapping.find(&item); if (found != mapping.end()) { @@ -287,6 +268,17 @@ struct ChildWidgetOrganizer::Instance deleteWidget(found.value()); mapping.erase(found); } + + if (virtualEnabled) + { + if (virtualPvs.contains(pos)) + { + clearWidgets(); + virtualPvs = PvsRange(); + } + // Virtual total height changes even if the item was not represented by a widget. + updateVirtualHeight(); + } } void dataItemOrderChanged() @@ -354,64 +346,13 @@ struct ChildWidgetOrganizer::Instance return 0; } - void refilter() - { - if (!filter) - { - firstAcceptedPos = 0; - return; - } - - firstAcceptedPos = ui::Data::InvalidPos; - - if (virtualEnabled) - { - virtualPvsRange = Ranges(); - virtualItemCount = 0; - virtualStrut->set(0); - clearWidgets(); - } - - for (ui::DataPos i = 0; i < dataItems->size(); ++i) - { - ui::Item const *item = &dataItems->at(i); - bool const accepted = filter->isItemAccepted(self, *dataItems, *item); - - if (!virtualEnabled) - { - if (!accepted && mapping.contains(item)) - { - // This widget needs to be removed. - removeItemWidget(i); - } - else if (accepted && !mapping.contains(item)) - { - // This widget may need to be created. - addItemWidget(i, IgnoreFilter); - } - } - - if (accepted) - { - ++virtualItemCount; - - if (firstAcceptedPos == ui::Data::InvalidPos) - { - firstAcceptedPos = i; - } - } - } - - updateVirtualHeight(); - } - //- Child Widget Virtualization --------------------------------------------------------- void updateVirtualHeight() { if (virtualEnabled) { - estimatedHeight->set(virtualItemCount * float(averageItemHeight)); + estimatedHeight->set(dataItems->size() * float(averageItemHeight)); } } @@ -437,199 +378,95 @@ struct ChildWidgetOrganizer::Instance duint maxVisibleItems() const { if (!virtualMin) return 0; + // TODO: Include some additional items beyond the visible area. return duint(std::ceil((virtualMax->value() - virtualMin->value()) / float(averageItemHeight))); } void updateVirtualization() { - if (!virtualEnabled || !virtualMin || !virtualMax || + if (!virtualEnabled || !virtualMin || !virtualMax || !virtualTop || virtualMin->valuei() >= virtualMax->valuei()) { return; } - // Apply the pending strut reductions, if the widget heights are now known. - // TODO: Changes in item heights should be observed and immediately applied... - { - QMutableSetIterator iter(pendingStrutAdjust); - while (iter.hasNext()) - { - iter.next(); - if (iter.value()->rule().height().value() > 0) - { - // Adjust based on the difference to the average height. - virtualStrut->set(de::max(0.f, virtualStrut->value() - - (iter.value()->rule().height().value() - - averageItemHeight))); - iter.remove(); - } - } - } + // Estimate a new PVS range based on the average item height and the visible area. + PvsRange const oldPvs = virtualPvs; - Rangef estimatedExtents; // Range covered by items (estimated). - if (container->childCount() > 0) - { - estimatedExtents = Rangef(firstChild()->rule().top() .value(), - lastChild()->rule().bottom().value()); - } - else - { - estimatedExtents = Rangef(virtualMin->value(), virtualMin->value()); - } - - duint const maxVisible = maxVisibleItems(); - bool changed = true; + float const y1 = de::max(0.f, virtualMin->value() - virtualTop->value()); + float const y2 = de::max(0.f, virtualMax->value() - virtualTop->value()); + PvsRange estimated = PvsRange(y1 / averageItemHeight, y2 / averageItemHeight) + .intersection(PvsRange(0, dataItems->size())); - while (changed) + // If this range is completely different than the current range, recreate + // all visible widgets. + if (oldPvs.isEmpty() || + estimated.start >= oldPvs.end || + estimated.end <= oldPvs.start) { - changed = false; - - // Can we remove widgets? - while (container->childCount() > 1 && - lastChild()->rule().top().value() > virtualMax->value()) - { - // Remove from the bottom. - ui::DataPos const pos = virtualPvsRange.end - 1; - if (reduceVirtualPvs(ui::Down)) - { - estimatedExtents.end -= virtualItemHeight(lastChild()); - removeItemWidget(pos); - changed = true; - } - else break; - } + clearWidgets(); - while (container->childCount() > 1 && - firstChild()->rule().bottom().value() < virtualMin->value()) - { - // Remove from the top. - ui::DataPos const pos = virtualPvsRange.start; - if (reduceVirtualPvs(ui::Up)) - { - float const itemHeight = virtualItemHeight(firstChild()); - estimatedExtents.start += itemHeight; - virtualStrut->set(de::max(0.f, virtualStrut->value() + itemHeight)); - removeItemWidget(pos); - changed = true; - } - else break; - } + // Set up a fully estimated strut. + virtualPvs = estimated; + virtualStrut->set(averageItemHeight * virtualPvs.start); - // Can we add more widgets in the bottom? - while (container->childCount() < virtualItemCount && - estimatedExtents.end < virtualMax->value() && - container->childCount() < maxVisible) + for (auto pos = virtualPvs.start; pos < virtualPvs.end; ++pos) { - ui::DataPos pos = extendVirtualPvs(ui::Down); - if (pos != ui::Data::InvalidPos) - { - estimatedExtents.end += averageItemHeight; - addItemWidget(pos, AlwaysAppend | IgnoreFilter); - changed = true; - } - else break; + addItemWidget(pos, AlwaysAppend); } - - // Add widgets to the top? - while (container->childCount() < virtualItemCount && - estimatedExtents.start > virtualMin->value() && - container->childCount() < maxVisible) + DENG2_ASSERT(virtualPvs.size() == container->childCount()); + } + else if (estimated.end > oldPvs.end) // Extend downward. + { + virtualPvs.end = estimated.end; + for (auto pos = oldPvs.end; pos < virtualPvs.end; ++pos) { - ui::DataPos pos = extendVirtualPvs(ui::Up); - if (pos != ui::Data::InvalidPos) - { - GuiWidget *w = addItemWidget(pos, AlwaysPrepend | IgnoreFilter); - pendingStrutAdjust.insert(w); - estimatedExtents.start -= averageItemHeight; - virtualStrut->set(de::max(0.f, virtualStrut->value() - averageItemHeight)); - changed = true; - } - else break; + addItemWidget(pos, AlwaysAppend); } + DENG2_ASSERT(virtualPvs.size() == container->childCount()); } - - if (virtualPvsRange.start == firstAcceptedPos) + else if (estimated.start < oldPvs.start) // Extend upward. { - virtualStrut->set(0); - pendingStrutAdjust.clear(); - } - } - - /** - * Extends the potentially visible range by one item. - * @param dir Direction for expansion. - * @return Position of the newly added item. - */ - ui::DataPos extendVirtualPvs(ui::Direction dir) - { - ui::DataPos pos; + // Reduce strut length to make room for new items. + virtualStrut->set(de::max(0.f, virtualStrut->value() - averageItemHeight * + (oldPvs.start - estimated.start))); - if (dir == ui::Down) - { - pos = virtualPvsRange.end; - do + virtualPvs.start = estimated.start; + for (auto pos = oldPvs.start - 1; + pos >= virtualPvs.start && pos < dataItems->size(); + --pos) { - if (pos == dataItems->size()) - { - return ui::Data::InvalidPos; // Out of items. - } - ++pos; + addItemWidget(pos, AlwaysPrepend); } - while (!filter || !filter->isItemAccepted(self, *dataItems, dataItems->at(pos - 1))); - - virtualPvsRange.end = pos; - //qDebug() << "PVS extended (down):" << virtualPvsRange.asText(); - return pos - 1; + DENG2_ASSERT(virtualPvs.size() == container->childCount()); } - else + + if (container->childCount() > 0) { - pos = virtualPvsRange.start; - do + // Remove excess widgets from the top and extend the strut accordingly. + while (virtualPvs.start < estimated.start) { - if (pos == 0) + float height = firstChild()->rule().height().value(); + if (!fequal(height, 0.f)) { - return ui::Data::InvalidPos; // Out of items. + // Actual height is not yet known, so use estimate. + height = averageItemHeight; } - --pos; + removeItemWidget(virtualPvs.start++); + virtualStrut->set(virtualStrut->value() + height); } - while (!filter || !filter->isItemAccepted(self, *dataItems, dataItems->at(pos))); - - virtualPvsRange.start = pos; - //qDebug() << "PVS extended (up):" << virtualPvsRange.asText(); - return virtualPvsRange.start; - } - } - - bool reduceVirtualPvs(ui::Direction dir) - { - if (virtualPvsRange.isEmpty()) return false; + DENG2_ASSERT(virtualPvs.size() == container->childCount()); - if (dir == ui::Down) - { - do + // Remove excess widgets from the bottom. + while (virtualPvs.end > estimated.end) { - --virtualPvsRange.end; + removeItemWidget(--virtualPvs.end); } - while (!virtualPvsRange.isEmpty() && - (!filter || - !filter->isItemAccepted(self, *dataItems, - dataItems->at(virtualPvsRange.end - 1)))); - //qDebug() << "PVS reduced (down):" << virtualPvsRange.asText(); + DENG2_ASSERT(virtualPvs.size() == container->childCount()); } - else if (dir == ui::Up) - { - do - { - ++virtualPvsRange.start; - } - while (!virtualPvsRange.isEmpty() && - (!filter || - !filter->isItemAccepted(self, *dataItems, - dataItems->at(virtualPvsRange.start)))); - //qDebug() << "PVS reduced (up):" << virtualPvsRange.asText(); - } - return true; + + DENG2_ASSERT(virtualPvs.size() == container->childCount()); } DENG2_PIMPL_AUDIENCE(WidgetCreation) @@ -675,16 +512,6 @@ ChildWidgetOrganizer::IWidgetFactory &ChildWidgetOrganizer::widgetFactory() cons return *d->factory; } -void ChildWidgetOrganizer::setFilter(IFilter const &filter) -{ - d->filter = &filter; -} - -void ChildWidgetOrganizer::unsetFilter() -{ - d->filter = nullptr; -} - GuiWidget *ChildWidgetOrganizer::itemWidget(ui::Item const &item) const { return d->find(item); @@ -700,20 +527,10 @@ ui::Item const *ChildWidgetOrganizer::findItemForWidget(GuiWidget const &widget) return d->findByWidget(widget); } -void ChildWidgetOrganizer::refilter() -{ - d->refilter(); -} - -dsize ChildWidgetOrganizer::itemCount() const -{ - return d->virtualItemCount; -} - void ChildWidgetOrganizer::setVirtualizationEnabled(bool enabled) { d->virtualEnabled = enabled; - d->virtualPvsRange = Ranges(0, 0); + d->virtualPvs = Instance::PvsRange(); if (d->virtualEnabled) { @@ -727,6 +544,11 @@ void ChildWidgetOrganizer::setVirtualizationEnabled(bool enabled) } } +void ChildWidgetOrganizer::setVirtualTopEdge(Rule const &topEdge) +{ + changeRef(d->virtualTop, topEdge); +} + void ChildWidgetOrganizer::setVisibleArea(Rule const &minimum, Rule const &maximum) { changeRef(d->virtualMin, minimum); diff --git a/doomsday/sdk/libappfw/src/widgets/menuwidget.cpp b/doomsday/sdk/libappfw/src/widgets/menuwidget.cpp index c7a05cf2bd..84dc0deac0 100644 --- a/doomsday/sdk/libappfw/src/widgets/menuwidget.cpp +++ b/doomsday/sdk/libappfw/src/widgets/menuwidget.cpp @@ -596,6 +596,8 @@ void MenuWidget::setVirtualizationEnabled(bool enabled, int averageItemHeight) { d->organizer.setVirtualizationEnabled(enabled); d->organizer.setAverageChildHeight(averageItemHeight); + d->organizer.setVirtualTopEdge(contentRule().top()); + d->organizer.setVisibleArea(rule().top(), rule().bottom()); d->needLayout = true; } diff --git a/doomsday/sdk/libcore/include/de/core/range.h b/doomsday/sdk/libcore/include/de/core/range.h index 37582584c6..24a48696a2 100644 --- a/doomsday/sdk/libcore/include/de/core/range.h +++ b/doomsday/sdk/libcore/include/de/core/range.h @@ -155,7 +155,6 @@ typedef Range Rangei; typedef Range Rangeui; typedef Range Rangei64; typedef Range Rangeui64; -typedef Range Ranges; typedef Range Rangef; typedef Range Ranged;