From 9af23d9c8561bd47d3add417ea0ae53304917101 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jaakko=20Ker=C3=A4nen?= Date: Sun, 8 Jun 2014 20:01:45 +0300 Subject: [PATCH 01/10] libcore|Range: Making a range with a start point and size --- doomsday/libcore/include/de/core/range.h | 1 + 1 file changed, 1 insertion(+) diff --git a/doomsday/libcore/include/de/core/range.h b/doomsday/libcore/include/de/core/range.h index 7922b081f8..5fa6be0028 100644 --- a/doomsday/libcore/include/de/core/range.h +++ b/doomsday/libcore/include/de/core/range.h @@ -41,6 +41,7 @@ struct Range Type end; explicit Range(Type const &a = 0, Type const &b = 0) : start(a), end(b) {} + static Range fromSize(Type const &a, Type const &size) { return Range(a, a + size); } inline bool isEmpty() const { return end == start; } inline Type size() const { return end - start; } inline bool contains(Type const &i) const { return i >= start && i < end; } From 51acaddc6edf4a3c534653b2efbae7f14d89bad8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jaakko=20Ker=C3=A4nen?= Date: Sun, 8 Jun 2014 20:04:33 +0300 Subject: [PATCH 02/10] Refactor|libappfw: Use TextDrawable in LogWidget Instead of custom code for wrapping text and preparing text for GL drawing, LogWidget now uses TextDrawable. Only the potentially visible lines are allocated on the atlas. Todo: Fix rewrapping, optimize and clean up. --- .../include/de/framework/textdrawable.h | 1 + doomsday/libappfw/src/gltextcomposer.cpp | 2 +- doomsday/libappfw/src/textdrawable.cpp | 5 + doomsday/libappfw/src/widgets/logwidget.cpp | 323 ++++++++++++++---- 4 files changed, 264 insertions(+), 67 deletions(-) diff --git a/doomsday/libappfw/include/de/framework/textdrawable.h b/doomsday/libappfw/include/de/framework/textdrawable.h index 5eadf1163b..9f753174fd 100644 --- a/doomsday/libappfw/include/de/framework/textdrawable.h +++ b/doomsday/libappfw/include/de/framework/textdrawable.h @@ -96,6 +96,7 @@ class LIBAPPFW_PUBLIC TextDrawable : public GLTextComposer String text() const; String plainText() const; bool isBeingWrapped() const; + Font const &font() const; private: DENG2_PRIVATE(d) diff --git a/doomsday/libappfw/src/gltextcomposer.cpp b/doomsday/libappfw/src/gltextcomposer.cpp index 61487e05dc..6e48d90d23 100644 --- a/doomsday/libappfw/src/gltextcomposer.cpp +++ b/doomsday/libappfw/src/gltextcomposer.cpp @@ -154,7 +154,7 @@ DENG2_PIMPL(GLTextComposer) if(i < lines.size()) { // Is the rasterized copy up to date? - if(!isLineVisible(i) || matchingSegments(i, info)) + if(/*!isLineVisible(i) ||*/ matchingSegments(i, info)) { // This line can be kept as is. continue; diff --git a/doomsday/libappfw/src/textdrawable.cpp b/doomsday/libappfw/src/textdrawable.cpp index a93ae9fa84..53c29c0e73 100644 --- a/doomsday/libappfw/src/textdrawable.cpp +++ b/doomsday/libappfw/src/textdrawable.cpp @@ -290,4 +290,9 @@ bool TextDrawable::isBeingWrapped() const return !d->tasks.isDone(); } +Font const &TextDrawable::font() const +{ + return d->backWrap->font(); +} + } // namespace de diff --git a/doomsday/libappfw/src/widgets/logwidget.cpp b/doomsday/libappfw/src/widgets/logwidget.cpp index 61b8373879..c96b3ceb25 100644 --- a/doomsday/libappfw/src/widgets/logwidget.cpp +++ b/doomsday/libappfw/src/widgets/logwidget.cpp @@ -22,8 +22,9 @@ */ #include "de/LogWidget" -#include "de/FontLineWrapping" -#include "de/GLTextComposer" +//#include "de/FontLineWrapping" +//#include "de/GLTextComposer" +#include "de/TextDrawable" #include "de/Style" #include @@ -58,28 +59,35 @@ public Font::RichFormat::IStyle * when drawing and by RewrapTask when wrapping the cached entries to a * resized content width. */ - class CacheEntry : public Lockable + class CacheEntry // : public Lockable { int _height; ///< Current height of the entry, in pixels. + //bool _unknownHeight; ///< Cannot be drawn yet, or new content is being prepared. public: - int sinkIndex; ///< Index of the corresponding entry in the Sink (for sorting). - Font::RichFormat format; - FontLineWrapping wraps; - GLTextComposer composer; - - CacheEntry(int index, Font const &font, Font::RichFormat::IStyle &richStyle, Atlas &atlas) - : _height(0), sinkIndex(index), format(richStyle) + //int sinkIndex; ///< Index of the corresponding entry in the Sink (for sorting). + //Font::RichFormat format; + //FontLineWrapping wraps; + //GLTextComposer composer; + TextDrawable drawable; + + CacheEntry(/*int index, */Font const &font, Font::RichFormat::IStyle &richStyle, Atlas &atlas) + : _height(0) + //, _dirty(true) + //, sinkIndex(index), format(richStyle) { - wraps.setFont(font); - composer.setAtlas(atlas); + drawable.init(atlas, font, &richStyle); + drawable.setRange(Rangei()); // Determined later. + //wraps.setFont(font); + //composer.setAtlas(atlas); } ~CacheEntry() { - DENG2_GUARD(this); + //DENG2_GUARD(this); // Free atlas allocations. - composer.release(); + drawable.deinit(); + //composer.release(); } int height() const @@ -87,51 +95,138 @@ public Font::RichFormat::IStyle return _height; } - bool needWrap() const + bool isReady() const { - DENG2_GUARD(this); - return wraps.isEmpty(); + return drawable.isReady(); } void wrap(String const &richText, int width) { - DENG2_GUARD(this); - String plain = format.initFromStyledText(richText); + //DENG2_GUARD(this); + //_dirty = true; + /*String plain = format.initFromStyledText(richText); wraps.wrapTextToWidth(plain, format, width); composer.setText(plain, format); composer.setWrapping(wraps); - recalculateHeight(); + recalculateHeight();*/ + drawable.setLineWrapWidth(width); + drawable.setText(richText); } - /// Returns the difference in the entry's height (in pixels). - int rewrap(int width) + void rewrap(int width) { - DENG2_GUARD(this); - int oldHeight = _height; - wraps.wrapTextToWidth(wraps.text(), format, width); - recalculateHeight(); - return _height - oldHeight; + //DENG2_GUARD(this); + //int oldHeight = _height; + //wraps.wrapTextToWidth(wraps.text(), format, width); + drawable.setLineWrapWidth(width); + //recalculateHeight(); + //return _height - oldHeight; } void recalculateHeight() { - _height = wraps.height() * wraps.font().lineSpacing().valuei(); + //_height = wraps.height() * wraps.font().lineSpacing().valuei(); + _height = drawable.wraps().height() * drawable.font().lineSpacing().valuei(); + } + + /*bool needsUpdate() const + { + return _dirty; + }*/ + + /// Returns the possible delta in the height of the entry. + /// Does not block even though a long wrapping task is in progress. + int updateHeightOnly() + { + int const oldHeight = _height; + if(drawable.update()) + { + recalculateHeight(); + return _height - oldHeight; + } + return 0; + } + + void update(int yBottom, Rangei const &visiblePixels) + { + if(!_height) return; + + // Determine which lines might be visible. + int const lineSpacing = drawable.font().lineSpacing().value(); + int const yTop = yBottom - _height; + Rangei range; + + if(yBottom < visiblePixels.start || yTop > visiblePixels.end) + { + // Completely outside. + } + else if(yTop >= visiblePixels.start && yBottom <= visiblePixels.end) + { + // Completely inside. + range = Rangei(0, drawable.wraps().height()); + } + else if(yTop < visiblePixels.start && yBottom > visiblePixels.end) + { + // Extends over the whole range and beyond. + range = Rangei::fromSize((visiblePixels.start - yTop) / lineSpacing, + (visiblePixels.end - visiblePixels.start) / lineSpacing + 1); + } + else if(yBottom > visiblePixels.end) + { + DENG2_ASSERT(yTop >= visiblePixels.start); + + // Partially inside. + range = Rangei(0, (visiblePixels.end - yTop) / lineSpacing); + } + else + { + DENG2_ASSERT(yBottom <= visiblePixels.end); + + // Partially inside. + int count = (yBottom - visiblePixels.start) / lineSpacing; + range = Rangei(drawable.wraps().height() - count, + drawable.wraps().height()); + } + + //qDebug() << yBottom << visiblePixels.asText() << "=>" << range.asText(); + + drawable.setRange(range); + + //if(!needsUpdate()) return 0; + + //int const oldHeight = _height; + + // Prepare the visible lines for drawing. + drawable.update(); + + /*{ + //_dirty = false; + //recalculateHeight(); + return _height - oldHeight; + } + return 0;*/ } void make(GLTextComposer::Vertices &verts, int y) { - DENG2_GUARD(this); - composer.update(); - composer.makeVertices(verts, Vector2i(0, y), AlignLeft); + DENG2_ASSERT(isReady()); + //DENG2_GUARD(this); + //composer.update(); + //if(isReady()) + { + drawable.makeVertices(verts, Vector2i(0, y), AlignLeft); + } } void clear() { - DENG2_GUARD(this); + //DENG2_GUARD(this); + drawable.setRange(Rangei()); // Nothing visible. + /* if(composer.isReady()) { composer.release(); - } + }*/ } }; @@ -149,26 +244,27 @@ public Font::RichFormat::IStyle { public: WrappingMemoryLogSink(LogWidget::Instance *wd) - : d(wd), - _maxEntries(1000), - _next(0), - _width(0) + : d(wd) + , _maxEntries(1000) + , _next(0) + , _width(0) { // Whenever the pool is idle, we'll check if pruning should be done. - QObject::connect(&_pool, SIGNAL(allTasksDone()), - d->thisPublic, SLOT(pruneExcessEntries())); + /*QObject::connect(&_pool, SIGNAL(allTasksDone()), + d->thisPublic, SLOT(pruneExcessEntries()));*/ } ~WrappingMemoryLogSink() { - _pool.waitForDone(); + //_pool.waitForDone(); clear(); } + /* bool isBusy() const { return !_pool.isDone(); - } + }*/ int maxEntries() const { return _maxEntries; } @@ -204,6 +300,7 @@ public Font::RichFormat::IStyle return _wrappedEntries.takeFirst(); } +#if 0 protected: /** * Asynchronous task for wrapping an incoming entry as rich text in the @@ -236,6 +333,7 @@ public Font::RichFormat::IStyle int _index; String _styledText; }; +#endif /** * Schedules wrapping tasks for all incoming entries. @@ -251,7 +349,17 @@ public Font::RichFormat::IStyle LogEntry const &ent = entry(_next); String const styled = d->formatter->logEntryToTextLines(ent).at(0); - _pool.start(new WrapTask(*this, _next, styled)); + //_pool.start(new WrapTask(*this, _next, styled)); + + CacheEntry *cached = new CacheEntry(/*_next, */*d->font, *d, *d->entryAtlas); + //cached->wrap(_styledText, _sink._width); + cached->wrap(styled, _width); + + //usleep(75000); // testing aid + + DENG2_GUARD(_wrappedEntries); + _wrappedEntries << cached; + _next++; } } @@ -272,6 +380,7 @@ public Font::RichFormat::IStyle QList cache; ///< Indices match entry indices in sink. int cacheWidth; +#if 0 /** * Asynchronous task that iterates through the cached entries in reverse * order and rewraps their existing content to a new maximum width. The @@ -318,6 +427,7 @@ public Font::RichFormat::IStyle volatile duint32 cancelRewrap; enum { CancelAllRewraps = 0xffffffff }; +#endif // State. Rangei visibleRange; @@ -355,7 +465,7 @@ public Font::RichFormat::IStyle : Base(i) , sink(this) , cacheWidth(0) - , cancelRewrap(0) + //, cancelRewrap(0) , visibleRange(Rangei(-1, -1)) , formatter(0) , font(0) @@ -386,9 +496,13 @@ public Font::RichFormat::IStyle void cancelRewraps() { + /* cancelRewrap = CancelAllRewraps; rewrapPool.waitForDone(); - cancelRewrap = 0; + cancelRewrap = 0;*/ + + // Cancel all wraps. + } void clearCache() @@ -526,22 +640,37 @@ public Font::RichFormat::IStyle return self.maximumScrollY().valuei(); } - void fetchNewCachedEntries() + void modifyContentHeight(float delta) { - int oldHeight = self.contentHeight(); + self.modifyContentHeight(delta); //cached->height()); + + // Adjust visible offset so it remains fixed in relation to + // existing entries. + if(self.scrollPositionY().animation().target() > 0) + { + self.scrollPositionY().shift(delta); + + //emit self.scrollPositionChanged(visibleOffset.target()); + } + } + void fetchNewCachedEntries() + { while(CacheEntry *cached = sink.nextCachedEntry()) { // Find a suitable place according to the original index in the sink; // the task pool may output the entries slightly out of order as // multiple threads may be in use. - int pos = cache.size(); + /*int pos = cache.size(); while(pos > 0 && cache.at(pos - 1)->sinkIndex > cached->sinkIndex) { --pos; } - cache.insert(pos, cached); + cache.insert(pos, cached);*/ + + cache << cached; +#if 0 self.modifyContentHeight(cached->height()); // Adjust visible offset so it remains fixed in relation to @@ -552,17 +681,13 @@ public Font::RichFormat::IStyle //emit self.scrollPositionChanged(visibleOffset.target()); } - } - - if(self.contentHeight() > oldHeight) - { - emit self.contentHeightIncreased(self.contentHeight() - oldHeight); +#endif } } void rewrapCache() { - if(cache.isEmpty()) return; + /*if(cache.isEmpty()) return; if(isRewrapping()) { @@ -572,13 +697,21 @@ public Font::RichFormat::IStyle // Start a rewrapping task that goes through all the existing entries, // starting from the latest entry. - rewrapPool.start(new RewrapTask(this, cache.size() - 1, contentWidth())); + rewrapPool.start(new RewrapTask(this, cache.size() - 1, contentWidth()));*/ + + for(int idx = cache.size() - 1; idx >= 0; --idx) + { + cache[idx]->rewrap(contentWidth()); + } } + /* bool isRewrapping() const { - return !rewrapPool.isDone(); + //return !rewrapPool.isDone(); + return numberOfUnwrappedEntries > 0; } + */ void releaseExcessComposedEntries() { @@ -621,7 +754,7 @@ public Font::RichFormat::IStyle void prune() { DENG2_ASSERT_IN_MAIN_THREAD(); - +#if 0 if(isRewrapping()) { // Rewrapper is busy, let's not do this right now. @@ -631,15 +764,24 @@ public Font::RichFormat::IStyle // We must lock the sink so no new entries are added. DENG2_GUARD(sink); - if(sink.isBusy()) + /*if(sink.isBusy()) { // New entries are being added, prune later. return; - } + }*/ fetchNewCachedEntries(); DENG2_ASSERT(sink.entryCount() == cache.size()); +#endif +#if 0 + // We must lock the sink so no new entries are added. + DENG2_GUARD(sink); + + fetchNewCachedEntries(); + + // There has to be a cache entry for each sink entry. + DENG2_ASSERT(sink.entryCount() == cache.size()); int num = sink.entryCount() - sink.maxEntries(); if(num > 0) @@ -650,12 +792,14 @@ public Font::RichFormat::IStyle self.modifyContentHeight(-cache[0]->height()); delete cache.takeFirst(); } + /* // Adjust existing indices to match. for(int i = 0; i < cache.size(); ++i) { cache[i]->sinkIndex -= num; - } + }*/ } +#endif } void updateProjection() @@ -665,6 +809,34 @@ public Font::RichFormat::IStyle uBgMvpMatrix = projMatrix; } + void updateEntries() + { + int oldHeight = self.contentHeight(); + + for(int idx = cache.size() - 1; idx >= 0; --idx) + { + CacheEntry *entry = cache[idx]; + + int delta = entry->updateHeightOnly(); + if(delta) + { + // The new height will be effective on the next frame. + modifyContentHeight(delta); + } + } + + if(self.contentHeight() > oldHeight) + { + emit self.contentHeightIncreased(self.contentHeight() - oldHeight); + } + } + + Rangei extendPixelRangeWithPadding(Rangei const &range) + { + int const padding = range.size() / 2; + return Rangei(range.start - padding, range.end + padding); + } + void updateGeometry() { Vector2i const contentSize = self.viewportSize(); @@ -677,11 +849,20 @@ public Font::RichFormat::IStyle cacheWidth = contentSize.x; } + updateEntries(); + // If the atlas becomes full, we'll retry once. entryAtlasFull = false; VertexBuf::Builder verts; + // Draw in reverse, as much as we need. + int initialYBottom = contentSize.y + self.scrollPositionY().valuei(); + contentOffsetForDrawing = std::ceil(contentOffset.value()); + + Rangei const visiblePixelRange = extendPixelRangeWithPadding( + Rangei(-contentOffsetForDrawing, contentSize.y - contentOffsetForDrawing)); + for(int attempt = 0; attempt < 2; ++attempt) { if(entryAtlasFull) @@ -690,21 +871,31 @@ public Font::RichFormat::IStyle entryAtlasFull = false; } - // Draw in reverse, as much as we need. - int yBottom = contentSize.y + self.scrollPositionY().valuei(); + int yBottom = initialYBottom; visibleRange = Rangei(-1, -1); entryAtlasLayoutChanged = false; - contentOffsetForDrawing = std::ceil(contentOffset.value()); + bool gotReady = false; // Find the visible range and update all visible entries. for(int idx = cache.size() - 1; yBottom >= -contentOffsetForDrawing && idx >= 0; --idx) { CacheEntry *entry = cache[idx]; + + entry->update(yBottom, visiblePixelRange); + + if(gotReady && !entry->isReady()) + { + // Anything above this has an undefined position, so we must stop here. + break; + } + yBottom -= entry->height(); - if(yBottom + contentOffsetForDrawing <= contentSize.y) + if(entry->isReady() && yBottom + contentOffsetForDrawing <= contentSize.y) { + gotReady = true; + // Rasterize and allocate if needed. entry->make(verts, yBottom); @@ -748,7 +939,6 @@ public Font::RichFormat::IStyle bgBuf->setVertices(gl::TriangleStrip, bgVerts, gl::Static); } - // Draw the background. background.draw(); Rectanglei vp = self.viewport(); @@ -786,7 +976,7 @@ LogWidget::LogWidget(String const &name) { setOrigin(Bottom); - connect(&d->rewrapPool, SIGNAL(allTasksDone()), this, SLOT(pruneExcessEntries())); + //connect(&d->rewrapPool, SIGNAL(allTasksDone()), this, SLOT(pruneExcessEntries())); LogBuffer::appBuffer().addSink(d->sink); } @@ -837,6 +1027,7 @@ void LogWidget::update() d->sink.setWidth(d->contentWidth()); d->fetchNewCachedEntries(); + d->prune(); // The log widget's geometry is fully dynamic -- regenerated on every frame. d->updateGeometry(); From 0734577a6989047134835744568b72e52eaa79d8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jaakko=20Ker=C3=A4nen?= Date: Sun, 8 Jun 2014 21:27:42 +0300 Subject: [PATCH 03/10] Fixed|libappfw: LogWidget's rewrapping behavior after refactoring Every call to TextDrawable::update() potentially changes the visible contents of the drawable, so its return value always should be acted upon. --- doomsday/libappfw/src/widgets/logwidget.cpp | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/doomsday/libappfw/src/widgets/logwidget.cpp b/doomsday/libappfw/src/widgets/logwidget.cpp index c96b3ceb25..e0721b6fdb 100644 --- a/doomsday/libappfw/src/widgets/logwidget.cpp +++ b/doomsday/libappfw/src/widgets/logwidget.cpp @@ -149,7 +149,7 @@ public Font::RichFormat::IStyle void update(int yBottom, Rangei const &visiblePixels) { - if(!_height) return; + if(!_height || drawable.isBeingWrapped()) return; // Determine which lines might be visible. int const lineSpacing = drawable.font().lineSpacing().value(); @@ -197,7 +197,7 @@ public Font::RichFormat::IStyle //int const oldHeight = _height; // Prepare the visible lines for drawing. - drawable.update(); + //drawable.update(); /*{ //_dirty = false; @@ -812,20 +812,26 @@ public Font::RichFormat::IStyle void updateEntries() { int oldHeight = self.contentHeight(); + bool needNotify = false; for(int idx = cache.size() - 1; idx >= 0; --idx) { CacheEntry *entry = cache[idx]; + int prevHeight = entry->height(); + int delta = entry->updateHeightOnly(); if(delta) { + // We won't notify when content height changes because of rewrapping. + if(!prevHeight) needNotify = true; + // The new height will be effective on the next frame. modifyContentHeight(delta); } } - if(self.contentHeight() > oldHeight) + if(needNotify && self.contentHeight() > oldHeight) { emit self.contentHeightIncreased(self.contentHeight() - oldHeight); } From 9fc10e9ca97494b4262d69c01621b8e9d0df7772 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jaakko=20Ker=C3=A4nen?= Date: Sun, 8 Jun 2014 21:29:32 +0300 Subject: [PATCH 04/10] Fixed|libappfw|GLTextComposer: Showing a portion of tabbed content The visible line range was not taken properly into account when laying out tabbed content for drawing. Also, when lines outside the range are released, their metadata is still kept intact even though the rasterized version is released from the atlas. This allows the text layout to still be calculated. --- doomsday/libappfw/src/gltextcomposer.cpp | 25 +++++++++++++++++++----- 1 file changed, 20 insertions(+), 5 deletions(-) diff --git a/doomsday/libappfw/src/gltextcomposer.cpp b/doomsday/libappfw/src/gltextcomposer.cpp index 6e48d90d23..5c68ffe3d8 100644 --- a/doomsday/libappfw/src/gltextcomposer.cpp +++ b/doomsday/libappfw/src/gltextcomposer.cpp @@ -83,12 +83,14 @@ DENG2_PIMPL(GLTextComposer) { if(!isLineVisible(i)) { - releaseLine(i); + releaseLine(i, ReleaseButKeepSegs); } } } - void releaseLine(int index) + enum ReleaseBehavior { ReleaseFully, ReleaseButKeepSegs }; + + void releaseLine(int index, ReleaseBehavior behavior = ReleaseFully) { Line &ln = lines[index]; for(int i = 0; i < ln.segs.size(); ++i) @@ -96,9 +98,13 @@ DENG2_PIMPL(GLTextComposer) if(!ln.segs[i].id.isNone()) { atlas->release(ln.segs[i].id); + ln.segs[i].id = Id::None; } } - ln.segs.clear(); + if(behavior == ReleaseFully) + { + ln.segs.clear(); + } } bool isLineVisible(int line) const @@ -201,6 +207,8 @@ DENG2_PIMPL(GLTextComposer) } line.segs << seg; } + + DENG2_ASSERT(line.segs.size() == info.segs.size()); } // Remove the excess lines. @@ -286,7 +294,8 @@ DENG2_PIMPL(GLTextComposer) // Set segment X coordinates by stacking them left-to-right on each line. for(int i = lineRange.start; i < rangeEnd; ++i) { - if(lines[i].segs.isEmpty()) continue; + if(lines[i].segs.isEmpty() || i >= visibleLineRange.end) + continue; lines[i].segs[0].x = wraps->lineInfo(i).indent; @@ -305,7 +314,11 @@ DENG2_PIMPL(GLTextComposer) // Find the maximum right edge for this spot. for(int i = lineRange.start; i < rangeEnd; ++i) { - FontLineWrapping::LineInfo const &info = wraps->lineInfo(i); + if(i >= visibleLineRange.end) break; + + FontLineWrapping::LineInfo const &info = wraps->lineInfo(i); + + DENG2_ASSERT(info.segs.size() == lines[i].segs.size()); for(int k = 0; k < info.segs.size(); ++k) { Instance::Line::Segment &seg = lines[i].segs[k]; @@ -319,6 +332,8 @@ DENG2_PIMPL(GLTextComposer) // Move the segments to this position. for(int i = lineRange.start; i < rangeEnd; ++i) { + if(i >= visibleLineRange.end) break; + int localRight = maxRight; FontLineWrapping::LineInfo const &info = wraps->lineInfo(i); From a98dd7cbd84c8b216d4dd6f765a4ddc0eba3403a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jaakko=20Ker=C3=A4nen?= Date: Mon, 9 Jun 2014 18:32:47 +0300 Subject: [PATCH 05/10] Refactor|libappfw|LogWidget: Updating a cached entry Entries are updated on the fly. --- doomsday/libappfw/src/widgets/logwidget.cpp | 112 +++++++++++++------- 1 file changed, 75 insertions(+), 37 deletions(-) diff --git a/doomsday/libappfw/src/widgets/logwidget.cpp b/doomsday/libappfw/src/widgets/logwidget.cpp index e0721b6fdb..95fda10bf3 100644 --- a/doomsday/libappfw/src/widgets/logwidget.cpp +++ b/doomsday/libappfw/src/widgets/logwidget.cpp @@ -62,6 +62,7 @@ public Font::RichFormat::IStyle class CacheEntry // : public Lockable { int _height; ///< Current height of the entry, in pixels. + int _oldHeight; //bool _unknownHeight; ///< Cannot be drawn yet, or new content is being prepared. public: @@ -73,6 +74,7 @@ public Font::RichFormat::IStyle CacheEntry(/*int index, */Font const &font, Font::RichFormat::IStyle &richStyle, Atlas &atlas) : _height(0) + , _oldHeight(0) //, _dirty(true) //, sinkIndex(index), format(richStyle) { @@ -95,6 +97,11 @@ public Font::RichFormat::IStyle return _height; } + int oldHeight() const + { + return _oldHeight; + } + bool isReady() const { return drawable.isReady(); @@ -123,33 +130,46 @@ public Font::RichFormat::IStyle //return _height - oldHeight; } - void recalculateHeight() - { - //_height = wraps.height() * wraps.font().lineSpacing().valuei(); - _height = drawable.wraps().height() * drawable.font().lineSpacing().valuei(); - } - - /*bool needsUpdate() const - { - return _dirty; - }*/ - /// Returns the possible delta in the height of the entry. /// Does not block even though a long wrapping task is in progress. - int updateHeightOnly() + int update() { - int const oldHeight = _height; + int const old = _height; if(drawable.update()) { - recalculateHeight(); - return _height - oldHeight; + _height = drawable.wraps().height() * drawable.font().lineSpacing().valuei(); + return _height - old; } return 0; } - void update(int yBottom, Rangei const &visiblePixels) + /// Returns the difference in height. + int updateVisibility(int yBottom, Rangei const &visiblePixels) { - if(!_height || drawable.isBeingWrapped()) return; + int heightDelta = 0; + + // Remember the height we had prior to any updating. + _oldHeight = _height; + + /* + * At this point: + * - we may have no content ready yet (_height is 0) + * - wrapping may have completed for the first time + * - wrapping may be ongoing for rewrapping, but we can still update the + * current content's visibility + * - wrapping may have completed for an updated content + */ + + if(!drawable.isBeingWrapped()) + { + // We may now have the number of wrapped lines. + heightDelta += update(); + } + if(!_height) + { + // Content not ready yet. + return 0; + } // Determine which lines might be visible. int const lineSpacing = drawable.font().lineSpacing().value(); @@ -196,8 +216,8 @@ public Font::RichFormat::IStyle //int const oldHeight = _height; - // Prepare the visible lines for drawing. - //drawable.update(); + // Updating will prepare the visible lines for drawing. + return update() + heightDelta; /*{ //_dirty = false; @@ -809,33 +829,29 @@ public Font::RichFormat::IStyle uBgMvpMatrix = projMatrix; } - void updateEntries() + /* + bool updateEntries() { - int oldHeight = self.contentHeight(); - bool needNotify = false; - + bool notify = false; for(int idx = cache.size() - 1; idx >= 0; --idx) { CacheEntry *entry = cache[idx]; int prevHeight = entry->height(); - int delta = entry->updateHeightOnly(); + int delta = entry->update(); if(delta) { // We won't notify when content height changes because of rewrapping. - if(!prevHeight) needNotify = true; + if(!prevHeight) notify = true; // The new height will be effective on the next frame. modifyContentHeight(delta); } } - - if(needNotify && self.contentHeight() > oldHeight) - { - emit self.contentHeightIncreased(self.contentHeight() - oldHeight); - } + return notify; } + */ Rangei extendPixelRangeWithPadding(Rangei const &range) { @@ -845,6 +861,8 @@ public Font::RichFormat::IStyle void updateGeometry() { + bool needHeightNotify = false; // if changed as entries are updated + int heightDelta = 0; Vector2i const contentSize = self.viewportSize(); // If the width of the widget changes, text needs to be reflowed with the @@ -855,8 +873,6 @@ public Font::RichFormat::IStyle cacheWidth = contentSize.x; } - updateEntries(); - // If the atlas becomes full, we'll retry once. entryAtlasFull = false; @@ -881,26 +897,37 @@ public Font::RichFormat::IStyle visibleRange = Rangei(-1, -1); entryAtlasLayoutChanged = false; - bool gotReady = false; + //bool gotReady = false; // Find the visible range and update all visible entries. for(int idx = cache.size() - 1; yBottom >= -contentOffsetForDrawing && idx >= 0; --idx) { CacheEntry *entry = cache[idx]; - entry->update(yBottom, visiblePixelRange); + if(int delta = entry->updateVisibility(yBottom, visiblePixelRange)) + { + heightDelta += delta; + if(!entry->oldHeight()) + { + // A bit of the kludge: height was changed due to a first-time + // update of content (new content appears rather than being a rewrap). + needHeightNotify = true; + } + } + + /* if(gotReady && !entry->isReady()) { - // Anything above this has an undefined position, so we must stop here. + // Everything above this likely has an undefined position, so we must stop here. break; - } + }*/ yBottom -= entry->height(); if(entry->isReady() && yBottom + contentOffsetForDrawing <= contentSize.y) { - gotReady = true; + //gotReady = true; // Rasterize and allocate if needed. entry->make(verts, yBottom); @@ -932,6 +959,17 @@ public Font::RichFormat::IStyle self.glMakeScrollIndicatorGeometry(verts); buf->setVertices(gl::TriangleStrip, verts, gl::Dynamic); + + // Apply changes to content height that may have occurred as text becomes + // available for drawing. + if(heightDelta) + { + modifyContentHeight(heightDelta); + if(needHeightNotify && heightDelta > 0) + { + emit self.contentHeightIncreased(heightDelta); + } + } } void draw() From c6c6efd6cd00075037f61cb0f87b2416d64349db Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jaakko=20Ker=C3=A4nen?= Date: Mon, 9 Jun 2014 18:43:12 +0300 Subject: [PATCH 06/10] libappfw|FoldPanelWidget: Removed frame around fold indicator Looks cleaner this way and is not confused with a button. --- doomsday/libappfw/src/widgets/foldpanelwidget.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/doomsday/libappfw/src/widgets/foldpanelwidget.cpp b/doomsday/libappfw/src/widgets/foldpanelwidget.cpp index 5eca14a07e..3dab3c7081 100644 --- a/doomsday/libappfw/src/widgets/foldpanelwidget.cpp +++ b/doomsday/libappfw/src/widgets/foldpanelwidget.cpp @@ -85,8 +85,8 @@ DENG2_PIMPL_NOREF(FoldPanelWidget) ColorBank::Colorf const &textColor = fold.title().textColorf(); // Frame. - verts.makeFlexibleFrame(rect.toRectanglei(), 5, textColor, - atlas.imageRectf(root.roundCorners())); + /*verts.makeFlexibleFrame(rect.toRectanglei(), 5, textColor, + atlas.imageRectf(root.roundCorners()));*/ Rectanglef uv = atlas.imageRectf(root.styleTexture("fold")); Matrix4f const turn = Matrix4f::rotateAround(rect.middle(), angle); From d9ad2ff435a30b9ca12ade3dddf20dedc603f055 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jaakko=20Ker=C3=A4nen?= Date: Mon, 9 Jun 2014 18:43:51 +0300 Subject: [PATCH 07/10] Fixed|libappfw|LogWidget: Layout glitch when new entries first appear --- doomsday/libappfw/src/widgets/logwidget.cpp | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/doomsday/libappfw/src/widgets/logwidget.cpp b/doomsday/libappfw/src/widgets/logwidget.cpp index 95fda10bf3..998464e9ab 100644 --- a/doomsday/libappfw/src/widgets/logwidget.cpp +++ b/doomsday/libappfw/src/widgets/logwidget.cpp @@ -913,6 +913,12 @@ public Font::RichFormat::IStyle // A bit of the kludge: height was changed due to a first-time // update of content (new content appears rather than being a rewrap). needHeightNotify = true; + + // Don't draw this entry yet; we'll need to fire the notification + // first to ensure that offsets are properly set so that the + // new entry's height is taken into account. The new entry will + // be visible on the next frame. + continue; } } From 20dd20e3fb5dca746dd49f5646a150a0ec527117 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jaakko=20Ker=C3=A4nen?= Date: Mon, 9 Jun 2014 20:27:58 +0300 Subject: [PATCH 08/10] Refactor|libappfw|LogWidget: Fixed pruning, overall cleanup --- .../libappfw/include/de/widgets/logwidget.h | 3 - doomsday/libappfw/src/widgets/logwidget.cpp | 302 ++---------------- .../libcore/include/de/core/memorylogsink.h | 5 + 3 files changed, 23 insertions(+), 287 deletions(-) diff --git a/doomsday/libappfw/include/de/widgets/logwidget.h b/doomsday/libappfw/include/de/widgets/logwidget.h index 3c9b4b6334..10931d0b26 100644 --- a/doomsday/libappfw/include/de/widgets/logwidget.h +++ b/doomsday/libappfw/include/de/widgets/logwidget.h @@ -69,9 +69,6 @@ class LIBAPPFW_PUBLIC LogWidget : public ScrollAreaWidget void drawContent(); bool handleEvent(Event const &event); -protected slots: - void pruneExcessEntries(); - signals: //void scrollPositionChanged(int pos); //void scrollMaxChanged(int maximum); diff --git a/doomsday/libappfw/src/widgets/logwidget.cpp b/doomsday/libappfw/src/widgets/logwidget.cpp index 998464e9ab..0fa7192e28 100644 --- a/doomsday/libappfw/src/widgets/logwidget.cpp +++ b/doomsday/libappfw/src/widgets/logwidget.cpp @@ -22,8 +22,6 @@ */ #include "de/LogWidget" -//#include "de/FontLineWrapping" -//#include "de/GLTextComposer" #include "de/TextDrawable" #include "de/Style" @@ -63,33 +61,22 @@ public Font::RichFormat::IStyle { int _height; ///< Current height of the entry, in pixels. int _oldHeight; - //bool _unknownHeight; ///< Cannot be drawn yet, or new content is being prepared. public: - //int sinkIndex; ///< Index of the corresponding entry in the Sink (for sorting). - //Font::RichFormat format; - //FontLineWrapping wraps; - //GLTextComposer composer; TextDrawable drawable; - CacheEntry(/*int index, */Font const &font, Font::RichFormat::IStyle &richStyle, Atlas &atlas) + CacheEntry(Font const &font, Font::RichFormat::IStyle &richStyle, Atlas &atlas) : _height(0) , _oldHeight(0) - //, _dirty(true) - //, sinkIndex(index), format(richStyle) { drawable.init(atlas, font, &richStyle); drawable.setRange(Rangei()); // Determined later. - //wraps.setFont(font); - //composer.setAtlas(atlas); } ~CacheEntry() { - //DENG2_GUARD(this); // Free atlas allocations. drawable.deinit(); - //composer.release(); } int height() const @@ -109,25 +96,13 @@ public Font::RichFormat::IStyle void wrap(String const &richText, int width) { - //DENG2_GUARD(this); - //_dirty = true; - /*String plain = format.initFromStyledText(richText); - wraps.wrapTextToWidth(plain, format, width); - composer.setText(plain, format); - composer.setWrapping(wraps); - recalculateHeight();*/ drawable.setLineWrapWidth(width); drawable.setText(richText); } void rewrap(int width) { - //DENG2_GUARD(this); - //int oldHeight = _height; - //wraps.wrapTextToWidth(wraps.text(), format, width); drawable.setLineWrapWidth(width); - //recalculateHeight(); - //return _height - oldHeight; } /// Returns the possible delta in the height of the entry. @@ -208,45 +183,21 @@ public Font::RichFormat::IStyle drawable.wraps().height()); } - //qDebug() << yBottom << visiblePixels.asText() << "=>" << range.asText(); - drawable.setRange(range); - //if(!needsUpdate()) return 0; - - //int const oldHeight = _height; - // Updating will prepare the visible lines for drawing. return update() + heightDelta; - - /*{ - //_dirty = false; - //recalculateHeight(); - return _height - oldHeight; - } - return 0;*/ } void make(GLTextComposer::Vertices &verts, int y) { DENG2_ASSERT(isReady()); - //DENG2_GUARD(this); - //composer.update(); - //if(isReady()) - { - drawable.makeVertices(verts, Vector2i(0, y), AlignLeft); - } + drawable.makeVertices(verts, Vector2i(0, y), AlignLeft); } - void clear() + void releaseFromAtlas() { - //DENG2_GUARD(this); drawable.setRange(Rangei()); // Nothing visible. - /* - if(composer.isReady()) - { - composer.release(); - }*/ } }; @@ -268,24 +219,13 @@ public Font::RichFormat::IStyle , _maxEntries(1000) , _next(0) , _width(0) - { - // Whenever the pool is idle, we'll check if pruning should be done. - /*QObject::connect(&_pool, SIGNAL(allTasksDone()), - d->thisPublic, SLOT(pruneExcessEntries()));*/ - } + {} ~WrappingMemoryLogSink() { - //_pool.waitForDone(); clear(); } - /* - bool isBusy() const - { - return !_pool.isDone(); - }*/ - int maxEntries() const { return _maxEntries; } void clear() @@ -320,41 +260,6 @@ public Font::RichFormat::IStyle return _wrappedEntries.takeFirst(); } -#if 0 - protected: - /** - * Asynchronous task for wrapping an incoming entry as rich text in the - * background. WrapTask is responsible for creating the CacheEntry - * instances for the LogWidget's entry cache. - */ - class WrapTask : public Task - { - public: - WrapTask(WrappingMemoryLogSink &owner, int index, String const &styledText) - : _sink(owner), - _index(index), - _styledText(styledText) - {} - - void runTask() - { - CacheEntry *cached = new CacheEntry(_index, *_sink.d->font, *_sink.d, - *_sink.d->entryAtlas); - cached->wrap(_styledText, _sink._width); - - //usleep(75000); // testing aid - - DENG2_GUARD_FOR(_sink._wrappedEntries, G); - _sink._wrappedEntries << cached; - } - - private: - WrappingMemoryLogSink &_sink; - int _index; - String _styledText; - }; -#endif - /** * Schedules wrapping tasks for all incoming entries. */ @@ -369,14 +274,9 @@ public Font::RichFormat::IStyle LogEntry const &ent = entry(_next); String const styled = d->formatter->logEntryToTextLines(ent).at(0); - //_pool.start(new WrapTask(*this, _next, styled)); - - CacheEntry *cached = new CacheEntry(/*_next, */*d->font, *d, *d->entryAtlas); - //cached->wrap(_styledText, _sink._width); + CacheEntry *cached = new CacheEntry(*d->font, *d, *d->entryAtlas); cached->wrap(styled, _width); - //usleep(75000); // testing aid - DENG2_GUARD(_wrappedEntries); _wrappedEntries << cached; @@ -392,63 +292,14 @@ public Font::RichFormat::IStyle int _width; struct WrappedEntries : public QList, public Lockable {}; - WrappedEntries _wrappedEntries; + WrappedEntries _wrappedEntries; ///< New entries possibly created in background threads. }; WrappingMemoryLogSink sink; - QList cache; ///< Indices match entry indices in sink. + QList cache; ///< Cached entries in use when drawing. int cacheWidth; -#if 0 - /** - * Asynchronous task that iterates through the cached entries in reverse - * order and rewraps their existing content to a new maximum width. The - * task is cancellable because an existing wrap should be abandoned if the - * widget content width changes again during a rewrap. - * - * The total height of the entries is updated as the entries are rewrapped. - */ - class RewrapTask : public Task - { - LogWidget::Instance *d; - duint32 _cancelLevel; - int _next; - int _width; - - public: - RewrapTask(LogWidget::Instance *wd, int startFrom, int width) - : d(wd), _cancelLevel(wd->cancelRewrap), _next(startFrom), _width(width) - {} - - void runTask() - { - while(_next >= 0 && _cancelLevel == d->cancelRewrap) - { - CacheEntry *e = d->cache[_next--]; - - // Rewrap and update total height. - int delta = e->rewrap(_width); - d->self.modifyContentHeight(delta); - - /// @todo Adjust the scroll position if this entry is below it - /// (would cause a visible scroll to occur). - - if(_next < d->visibleRange.end) - { - // Above the visible range, no need to rush. - TimeDelta(.001).sleep(); - } - } - } - }; - - TaskPool rewrapPool; ///< Used when rewrapping existing cached entries. - volatile duint32 cancelRewrap; - - enum { CancelAllRewraps = 0xffffffff }; -#endif - // State. Rangei visibleRange; Animation contentOffset; ///< Additional vertical offset to apply when drawing content. @@ -485,7 +336,6 @@ public Font::RichFormat::IStyle : Base(i) , sink(this) , cacheWidth(0) - //, cancelRewrap(0) , visibleRange(Rangei(-1, -1)) , formatter(0) , font(0) @@ -516,13 +366,8 @@ public Font::RichFormat::IStyle void cancelRewraps() { - /* - cancelRewrap = CancelAllRewraps; - rewrapPool.waitForDone(); - cancelRewrap = 0;*/ - // Cancel all wraps. - + /// @todo TextDrawable does not support cancelling. } void clearCache() @@ -662,15 +507,13 @@ public Font::RichFormat::IStyle void modifyContentHeight(float delta) { - self.modifyContentHeight(delta); //cached->height()); + self.modifyContentHeight(delta); // Adjust visible offset so it remains fixed in relation to // existing entries. if(self.scrollPositionY().animation().target() > 0) { self.scrollPositionY().shift(delta); - - //emit self.scrollPositionChanged(visibleOffset.target()); } } @@ -678,60 +521,21 @@ public Font::RichFormat::IStyle { while(CacheEntry *cached = sink.nextCachedEntry()) { - // Find a suitable place according to the original index in the sink; - // the task pool may output the entries slightly out of order as - // multiple threads may be in use. - /*int pos = cache.size(); - while(pos > 0 && cache.at(pos - 1)->sinkIndex > cached->sinkIndex) - { - --pos; - } - cache.insert(pos, cached);*/ - cache << cached; - -#if 0 - self.modifyContentHeight(cached->height()); - - // Adjust visible offset so it remains fixed in relation to - // existing entries. - if(self.scrollPositionY().animation().target() > 0) - { - self.scrollPositionY().shift(cached->height()); - - //emit self.scrollPositionChanged(visibleOffset.target()); - } -#endif } } void rewrapCache() { - /*if(cache.isEmpty()) return; - - if(isRewrapping()) - { - // Cancel an existing rewrap. - cancelRewrap++; - } - - // Start a rewrapping task that goes through all the existing entries, - // starting from the latest entry. - rewrapPool.start(new RewrapTask(this, cache.size() - 1, contentWidth()));*/ - + // Resize all the existing entries, starting from the latest visible entry. for(int idx = cache.size() - 1; idx >= 0; --idx) { cache[idx]->rewrap(contentWidth()); } - } - /* - bool isRewrapping() const - { - //return !rewrapPool.isDone(); - return numberOfUnwrappedEntries > 0; + // TODO - Resize the rest of the items (below the visible range). + } - */ void releaseExcessComposedEntries() { @@ -743,14 +547,14 @@ public Font::RichFormat::IStyle int excess = visibleRange.start - len; for(int i = 0; i <= excess && i < cache.size(); ++i) { - cache[i]->clear(); + cache[i]->releaseFromAtlas(); } // Excess entries after the visible range. excess = visibleRange.end + len; for(int i = excess; i < cache.size(); ++i) { - cache[i]->clear(); + cache[i]->releaseFromAtlas(); } } @@ -763,7 +567,7 @@ public Font::RichFormat::IStyle { if(!visibleRange.contains(i)) { - cache[i]->clear(); + cache[i]->releaseFromAtlas(); } } } @@ -774,52 +578,22 @@ public Font::RichFormat::IStyle void prune() { DENG2_ASSERT_IN_MAIN_THREAD(); -#if 0 - if(isRewrapping()) - { - // Rewrapper is busy, let's not do this right now. - return; - } - // We must lock the sink so no new entries are added. + // We must lock the sink during this so no new entries are added. DENG2_GUARD(sink); - /*if(sink.isBusy()) - { - // New entries are being added, prune later. - return; - }*/ - - fetchNewCachedEntries(); - - DENG2_ASSERT(sink.entryCount() == cache.size()); -#endif -#if 0 - // We must lock the sink so no new entries are added. - DENG2_GUARD(sink); - - fetchNewCachedEntries(); - - // There has to be a cache entry for each sink entry. - DENG2_ASSERT(sink.entryCount() == cache.size()); - + // Remove oldest excess entries. int num = sink.entryCount() - sink.maxEntries(); if(num > 0) { + // There is one sink entry and one cached entry for each log entry. sink.remove(0, num); for(int i = 0; i < num; ++i) { self.modifyContentHeight(-cache[0]->height()); delete cache.takeFirst(); } - /* - // Adjust existing indices to match. - for(int i = 0; i < cache.size(); ++i) - { - cache[i]->sinkIndex -= num; - }*/ } -#endif } void updateProjection() @@ -829,30 +603,6 @@ public Font::RichFormat::IStyle uBgMvpMatrix = projMatrix; } - /* - bool updateEntries() - { - bool notify = false; - for(int idx = cache.size() - 1; idx >= 0; --idx) - { - CacheEntry *entry = cache[idx]; - - int prevHeight = entry->height(); - - int delta = entry->update(); - if(delta) - { - // We won't notify when content height changes because of rewrapping. - if(!prevHeight) notify = true; - - // The new height will be effective on the next frame. - modifyContentHeight(delta); - } - } - return notify; - } - */ - Rangei extendPixelRangeWithPadding(Rangei const &range) { int const padding = range.size() / 2; @@ -897,8 +647,6 @@ public Font::RichFormat::IStyle visibleRange = Rangei(-1, -1); entryAtlasLayoutChanged = false; - //bool gotReady = false; - // Find the visible range and update all visible entries. for(int idx = cache.size() - 1; yBottom >= -contentOffsetForDrawing && idx >= 0; --idx) { @@ -922,13 +670,6 @@ public Font::RichFormat::IStyle } } - /* - if(gotReady && !entry->isReady()) - { - // Everything above this likely has an undefined position, so we must stop here. - break; - }*/ - yBottom -= entry->height(); if(entry->isReady() && yBottom + contentOffsetForDrawing <= contentSize.y) @@ -1026,8 +767,6 @@ LogWidget::LogWidget(String const &name) { setOrigin(Bottom); - //connect(&d->rewrapPool, SIGNAL(allTasksDone()), this, SLOT(pruneExcessEntries())); - LogBuffer::appBuffer().addSink(d->sink); } @@ -1093,11 +832,6 @@ bool LogWidget::handleEvent(Event const &event) return ScrollAreaWidget::handleEvent(event); } -void LogWidget::pruneExcessEntries() -{ - d->prune(); -} - void LogWidget::glInit() { d->glInit(); diff --git a/doomsday/libcore/include/de/core/memorylogsink.h b/doomsday/libcore/include/de/core/memorylogsink.h index 5f386f5f28..a758cc727e 100644 --- a/doomsday/libcore/include/de/core/memorylogsink.h +++ b/doomsday/libcore/include/de/core/memorylogsink.h @@ -47,6 +47,11 @@ class DENG2_PUBLIC MemoryLogSink : public LogSink, public Lockable void clear(); protected: + /** + * Called after a new entry has been appended to the end of the entries list. + * + * @param entry Added entry. + */ virtual void addedNewEntry(LogEntry &entry); private: From e8e6a5879ede41c9100360810f695a05890cf6fb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jaakko=20Ker=C3=A4nen?= Date: Mon, 9 Jun 2014 21:59:05 +0300 Subject: [PATCH 09/10] libcore|TaskPool: Ensure pool is not deleted when the last task finishes In taskFinished(), between remove() and isEmpty() the mutex guarding 'd' was released. This would allow ~TaskPool to finish before the 'd' is accessed again in taskFinished(). --- doomsday/libcore/src/concurrency/taskpool.cpp | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/doomsday/libcore/src/concurrency/taskpool.cpp b/doomsday/libcore/src/concurrency/taskpool.cpp index 852d5e0a2f..3c9e5ae3a1 100644 --- a/doomsday/libcore/src/concurrency/taskpool.cpp +++ b/doomsday/libcore/src/concurrency/taskpool.cpp @@ -55,14 +55,17 @@ DENG2_PIMPL(TaskPool), public Lockable, public Waitable tasks.insert(t); } - void remove(Task *t) + /// Returns @c true, if the pool became empty as result of the remove. + bool remove(Task *t) { DENG2_GUARD(this); tasks.remove(t); if(tasks.isEmpty()) { post(); + return true; } + return false; } void waitForEmpty() const @@ -70,7 +73,7 @@ DENG2_PIMPL(TaskPool), public Lockable, public Waitable wait(); DENG2_GUARD(this); DENG2_ASSERT(tasks.isEmpty()); - post(); + post(); // When empty, the semaphore is available. } bool isEmpty() const @@ -104,11 +107,10 @@ bool TaskPool::isDone() const void TaskPool::taskFinished(Task &task) { - d->remove(&task); - - if(d->isEmpty()) + DENG2_GUARD(d); + if(d->remove(&task)) { - allTasksDone(); + emit allTasksDone(); } } From 739216e28d5c4ea595f02c1c9f0b17158ef1aacc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jaakko=20Ker=C3=A4nen?= Date: Tue, 10 Jun 2014 20:51:47 +0300 Subject: [PATCH 10/10] libappfw|LogWidget: Start rewrap from visible range Also updated comments in the source file. --- doomsday/libappfw/src/widgets/logwidget.cpp | 64 +++++++++++++-------- 1 file changed, 39 insertions(+), 25 deletions(-) diff --git a/doomsday/libappfw/src/widgets/logwidget.cpp b/doomsday/libappfw/src/widgets/logwidget.cpp index 0fa7192e28..b27e3b433f 100644 --- a/doomsday/libappfw/src/widgets/logwidget.cpp +++ b/doomsday/libappfw/src/widgets/logwidget.cpp @@ -1,11 +1,6 @@ /** @file widgets/logwidget.cpp Widget for output message log. * - * @todo Refactor: Separate the non-Log-related functionality into its own - * class that handles long text documents with a background thread used for - * preparing it for showing on the screen. Such a class can be used as the - * foundation of DocumentWidget as well. - * - * @authors Copyright © 2013 Jaakko Keränen + * @authors Copyright © 2013-2014 Jaakko Keränen * * @par License * LGPL: http://www.gnu.org/licenses/lgpl.html @@ -50,17 +45,20 @@ public Font::RichFormat::IStyle typedef GLBufferT VertexBuf; /** - * Cached log entry ready for drawing. The styled text of the entry is - * wrapped onto multiple lines according to the available content width. + * Cached log entry ready for drawing. TextDrawable takes the styled text of the + * entry and wraps it onto multiple lines according to the available content width. + * + * The height of the entry is initially zero. When TextDrawable has finished + * laying out and preparing the text, the real height is then updated and the + * content height of the log increases. * - * CacheEntry is Lockable because it is accessed both by the main thread - * when drawing and by RewrapTask when wrapping the cached entries to a - * resized content width. + * CacheEntry is accessed only from the main thread. However, instances may be + * initially created also in background threads (if they happen to flush the log). */ - class CacheEntry // : public Lockable + class CacheEntry { - int _height; ///< Current height of the entry, in pixels. - int _oldHeight; + int _height; ///< Current height of the entry, in pixels. + int _oldHeight; ///< Previous height, before calling updateVisibility(). public: TextDrawable drawable; @@ -118,7 +116,15 @@ public Font::RichFormat::IStyle return 0; } - /// Returns the difference in height. + /** + * Updates the entry's visibility: which lines might be visible to the user + * and thus need to be allocated on an atlas and ready to draw. + * + * @param yBottom Bottom coordinate for the entry. + * @param visibleRange Full range of currently visible pixels in the widget. + * + * @return Possible change in the height of the entry. + */ int updateVisibility(int yBottom, Rangei const &visiblePixels) { int heightDelta = 0; @@ -202,14 +208,13 @@ public Font::RichFormat::IStyle }; /** - * Log sink that wraps log entries as rich text to multiple lines before - * they are shown by LogWidget. The wrapping is done by a TaskPool in the - * background. + * Log sink that where all entries that will be visible in the widget will + * be received. For each entry, a CacheEntry is created and its TextDrawable + * will start processing the entry contents in the background. * - * Whenever all the wrapping tasks are complete, LogWidget will be notified - * and it will check if excess entries should be removed. Entries are only - * removed from the sink (and cache) during a prune, in the main thread, - * and during this no background tasks are running. + * LogWidget will periodically check if excess entries should be removed. Entries are + * only removed from the sink (and cache) during a prune, in the main thread, + * during which the sink is locked. */ class WrappingMemoryLogSink : public MemoryLogSink { @@ -527,14 +532,23 @@ public Font::RichFormat::IStyle void rewrapCache() { + int startFrom = cache.size() - 1; + if(visibleRange >= 0) + { + startFrom = min(startFrom, visibleRange.end + 1); + } + // Resize all the existing entries, starting from the latest visible entry. - for(int idx = cache.size() - 1; idx >= 0; --idx) + for(int idx = startFrom; idx >= 0; --idx) { cache[idx]->rewrap(contentWidth()); } - // TODO - Resize the rest of the items (below the visible range). - + // Resize the rest of the items (below the visible range). + for(int idx = startFrom + 1; idx < cache.size(); ++idx) + { + cache[idx]->rewrap(contentWidth()); + } } void releaseExcessComposedEntries()