Mercurial > hg > svgui
changeset 1090:c8c747783110 spectrogram-minor-refactor
Cut over to using the renderer (though it's very incomplete) and fix some cache problems and pointer lifetime misunderstandings
| author | Chris Cannam |
|---|---|
| date | Tue, 05 Jul 2016 17:48:26 +0100 |
| parents | c8683d94442a |
| children | ac10a087e045 |
| files | layer/Colour3DPlotRenderer.cpp layer/Colour3DPlotRenderer.h layer/LayerGeometryProvider.h layer/ScrollableImageCache.cpp layer/ScrollableImageCache.h layer/SpectrogramLayer.cpp |
| diffstat | 6 files changed, 105 insertions(+), 59 deletions(-) [+] |
line wrap: on
line diff
--- a/layer/Colour3DPlotRenderer.cpp Tue Jul 05 12:20:56 2016 +0100 +++ b/layer/Colour3DPlotRenderer.cpp Tue Jul 05 17:48:26 2016 +0100 @@ -28,25 +28,22 @@ using namespace std; Colour3DPlotRenderer::RenderResult -Colour3DPlotRenderer::render(QPainter &paint, QRect rect) +Colour3DPlotRenderer::render(LayerGeometryProvider *v, QPainter &paint, QRect rect) { - return render(paint, rect, false); + return render(v, paint, rect, false); } Colour3DPlotRenderer::RenderResult -Colour3DPlotRenderer::renderTimeConstrained(QPainter &paint, QRect rect) +Colour3DPlotRenderer::renderTimeConstrained(LayerGeometryProvider *v, + QPainter &paint, QRect rect) { - return render(paint, rect, true); + return render(v, paint, rect, true); } Colour3DPlotRenderer::RenderResult -Colour3DPlotRenderer::render(QPainter &paint, QRect rect, bool timeConstrained) +Colour3DPlotRenderer::render(LayerGeometryProvider *v, + QPainter &paint, QRect rect, bool timeConstrained) { - LayerGeometryProvider *v = m_sources.geometryProvider; - if (!v) { - throw std::logic_error("no LayerGeometryProvider provided"); - } - sv_frame_t startFrame = v->getStartFrame(); int x0 = v->getXForViewX(rect.x()); @@ -57,22 +54,35 @@ m_cache.resize(v->getPaintSize()); m_cache.setZoomLevel(v->getZoomLevel()); + cerr << "cache start " << m_cache.getStartFrame() + << " view start " << startFrame + << " valid left " << m_cache.getValidLeft() + << " valid right " << m_cache.getValidRight() + << " x0 " << x0 + << " x1 " << x1 + << endl; + + if (m_cache.isValid()) { // some part of the cache is valid if (v->getXForFrame(m_cache.getStartFrame()) == v->getXForFrame(startFrame) && m_cache.getValidLeft() <= x0 && m_cache.getValidRight() >= x1) { - + + cerr << "cache hit" << endl; + // cache is valid for the complete requested area paint.drawImage(rect, m_cache.getImage(), rect); return { rect, {} }; } else { + cerr << "cache partial hit" << endl; + // cache doesn't begin at the right frame or doesn't // contain the complete view, but might be scrollable or // partially usable - m_cache.scrollTo(startFrame); + m_cache.scrollTo(v, startFrame); // if we are not time-constrained, then we want to paint // the whole area in one go; we don't return a partial @@ -87,6 +97,9 @@ } } } + } else { + // cache completely invalid + m_cache.setStartFrame(startFrame); } bool rightToLeft = false; @@ -104,6 +117,8 @@ } if (m_cache.isValid()) { + cerr << "cache somewhat valid" << endl; + // When rendering only a part of the cache, we need to make // sure that the part we're rendering is adjacent to (or // overlapping) a valid area of cache, if we have one. The @@ -125,7 +140,7 @@ rightToLeft = isLeftOfValidArea; } - renderToCache(x0, x1 - x0, rightToLeft, timeConstrained); + renderToCache(v, x0, x1 - x0, rightToLeft, timeConstrained); QRect pr = rect & m_cache.getValidArea(); paint.drawImage(pr.x(), pr.y(), m_cache.getImage(), @@ -161,7 +176,8 @@ } void -Colour3DPlotRenderer::renderToCache(int x0, int repaintWidth, +Colour3DPlotRenderer::renderToCache(LayerGeometryProvider *v, + int x0, int repaintWidth, bool rightToLeft, bool timeConstrained) { // Draw to the draw buffer, and then scale-copy from there. @@ -171,8 +187,6 @@ throw std::logic_error("no source model provided, or model not ready"); } - LayerGeometryProvider *v = m_sources.geometryProvider; // already checked - // The draw buffer contains a fragment at either our pixel // resolution (if there is more than one time-bin per pixel) or // time-bin resolution (if a time-bin spans more than one pixel). @@ -251,7 +265,7 @@ } else { for (int x = 0; x < drawWidth; ++x) { - sv_frame_t f0 = v->getFrameForX(x); + sv_frame_t f0 = v->getFrameForX(x0 + x); double s0 = double(f0 - model->getStartFrame()) / binResolution; binforx[x] = int(s0 + 0.0001); } @@ -269,8 +283,7 @@ } for (int y = 0; y < h; ++y) { - binfory[y] = - m_sources.verticalBinLayer->getBinForY(m_sources.geometryProvider, y); + binfory[y] = m_sources.verticalBinLayer->getBinForY(v, h - y - 1); } int attainedWidth = renderDrawBuffer(repaintWidth, @@ -335,7 +348,6 @@ } } - int Colour3DPlotRenderer::renderDrawBuffer(int w, int h, const vector<int> &binforx, @@ -416,6 +428,10 @@ // distribute/interpolate ColumnOp::Column fullColumn = sourceModel->getColumn(sx); + + cerr << "x " << x << ", sx " << sx << ", col height " << fullColumn.size() + << ", minbin " << minbin << ", maxbin " << maxbin << endl; + ColumnOp::Column column = vector<float>(fullColumn.data() + minbin, fullColumn.data() + maxbin + 1);
--- a/layer/Colour3DPlotRenderer.h Tue Jul 05 12:20:56 2016 +0100 +++ b/layer/Colour3DPlotRenderer.h Tue Jul 05 17:48:26 2016 +0100 @@ -47,11 +47,9 @@ }; struct Sources { - Sources() : geometryProvider(0), verticalBinLayer(0), - source(0), peaks(0), fft(0) { } + Sources() : verticalBinLayer(0), source(0), peaks(0), fft(0) { } // These must all outlive this class - LayerGeometryProvider *geometryProvider; // always const VerticalBinLayer *verticalBinLayer; // always DenseThreeDimensionalModel *source; // always Dense3DModelPeakCache *peaks; // optionally @@ -98,14 +96,21 @@ /** * Render the requested area using the given painter, obtaining - * geometry (e.g. start frame) from the stored + * geometry (e.g. start frame) from the given * LayerGeometryProvider. * - * The whole rect will be rendered and the returned QRect will be - * equal to the passed QRect. (See renderTimeConstrained for an - * alternative that may render only part of the rect in cases - * where obtaining source data is slow and retaining - * responsiveness is important.) + * The whole of the supplied rect will be rendered and the + * returned QRect will be equal to the supplied QRect. (See + * renderTimeConstrained for an alternative that may render only + * part of the rect in cases where obtaining source data is slow + * and retaining responsiveness is important.) + * + * Note that Colour3DPlotRenderer retains internal cache state + * related to the size and position of the supplied + * LayerGeometryProvider. Although it is valid to call render() + * successively on the same Colour3DPlotRenderer with different + * LayerGeometryProviders, it will be much faster to use a + * dedicated Colour3DPlotRenderer for each LayerGeometryProvider. * * If the model to render from is not ready, this will throw a * std::logic_error exception. The model must be ready and the @@ -113,7 +118,7 @@ * that the LayerGeometryProvider returns valid results; it is the * caller's responsibility to ensure these. */ - RenderResult render(QPainter &paint, QRect rect); + RenderResult render(LayerGeometryProvider *v, QPainter &paint, QRect rect); /** * Render the requested area using the given painter, obtaining @@ -127,13 +132,21 @@ * rendered. Note that we always render the full requested height, * it's only width that is time-constrained. * + * Note that Colour3DPlotRenderer retains internal cache state + * related to the size and position of the supplied + * LayerGeometryProvider. Although it is valid to call render() + * successively on the same Colour3DPlotRenderer with different + * LayerGeometryProviders, it will be much faster to use a + * dedicated Colour3DPlotRenderer for each LayerGeometryProvider. + * * If the model to render from is not ready, this will throw a * std::logic_error exception. The model must be ready and the * layer requesting the render must not be dormant in its view, so * that the LayerGeometryProvider returns valid results; it is the * caller's responsibility to ensure these. */ - RenderResult renderTimeConstrained(QPainter &paint, QRect rect); + RenderResult renderTimeConstrained(LayerGeometryProvider *v, + QPainter &paint, QRect rect); private: Sources m_sources; @@ -154,8 +167,9 @@ // then repainting from cache to the requested painter. ScrollableImageCache m_cache; - RenderResult render(QPainter &paint, QRect rect, bool timeConstrained); - void renderToCache(int x0, int repaintWidth, + RenderResult render(LayerGeometryProvider *v, + QPainter &paint, QRect rect, bool timeConstrained); + void renderToCache(LayerGeometryProvider *v, int x0, int repaintWidth, bool rightToLeft, bool timeConstrained); int renderDrawBuffer(int w, int h, const std::vector<int> &binforx,
--- a/layer/LayerGeometryProvider.h Tue Jul 05 12:20:56 2016 +0100 +++ b/layer/LayerGeometryProvider.h Tue Jul 05 17:48:26 2016 +0100 @@ -25,6 +25,22 @@ class View; class Layer; +/** + * Interface for classes that provide geometry information (such as + * size, start frame, and a large number of other properties) about + * the disposition of a layer. The main implementor of this interface + * is the View class, but other implementations may be used in + * different circumstances, e.g. as a proxy to handle hi-dpi + * coordinate mapping. + * + * Note it is expected that some implementations of this may be + * disposable, created on-the-fly for a single use. Code that receives + * a LayerGeometryProvider pointer as an argument to something should + * not, in general, store that pointer as it may be invalidated before + * the next use. Use getId() to instead obtain a persistent identifier + * for a LayerGeometryProvider, for example to establish whether the + * same one is being provided in two separate calls. + */ class LayerGeometryProvider { protected:
--- a/layer/ScrollableImageCache.cpp Tue Jul 05 12:20:56 2016 +0100 +++ b/layer/ScrollableImageCache.cpp Tue Jul 05 17:48:26 2016 +0100 @@ -17,20 +17,18 @@ #include <iostream> using namespace std; -//#define DEBUG_SCROLLABLE_IMAGE_CACHE 1 +#define DEBUG_SCROLLABLE_IMAGE_CACHE 1 void -ScrollableImageCache::scrollTo(sv_frame_t newStartFrame) +ScrollableImageCache::scrollTo(LayerGeometryProvider *v, sv_frame_t newStartFrame) { - if (!m_v) throw std::logic_error("ScrollableImageCache: not associated with a LayerGeometryProvider"); - if (m_startFrame == newStartFrame) { // haven't moved return; } - int dx = (m_v->getXForFrame(m_startFrame) - - m_v->getXForFrame(newStartFrame)); + int dx = (v->getXForFrame(m_startFrame) - + v->getXForFrame(newStartFrame)); #ifdef DEBUG_SCROLLABLE_IMAGE_CACHE cerr << "ScrollableImageCache::scrollTo: start frame " << m_startFrame
--- a/layer/ScrollableImageCache.h Tue Jul 05 12:20:56 2016 +0100 +++ b/layer/ScrollableImageCache.h Tue Jul 05 17:48:26 2016 +0100 @@ -37,8 +37,7 @@ class ScrollableImageCache { public: - ScrollableImageCache(const LayerGeometryProvider *v = 0) : - m_v(v), + ScrollableImageCache() : m_left(0), m_width(0), m_startFrame(0), @@ -112,11 +111,12 @@ } /** - * Set the new start frame for the cache, if possible also moving - * along any existing valid data within the cache so that it - * continues to be valid for the new start frame. + * Set the new start frame for the cache, according to the + * geometry of the supplied LayerGeometryProvider, if possible + * also moving along any existing valid data within the cache so + * that it continues to be valid for the new start frame. */ - void scrollTo(sv_frame_t newStartFrame); + void scrollTo(LayerGeometryProvider *v, sv_frame_t newStartFrame); /** * Take a left coordinate and width describing a region, and @@ -142,7 +142,6 @@ int imageWidth); private: - const LayerGeometryProvider *m_v; QImage m_image; int m_left; // of valid region int m_width; // of valid region
--- a/layer/SpectrogramLayer.cpp Tue Jul 05 12:20:56 2016 +0100 +++ b/layer/SpectrogramLayer.cpp Tue Jul 05 17:48:26 2016 +0100 @@ -52,6 +52,7 @@ #include <alloca.h> #endif +#define DEBUG_SPECTROGRAM 1 #define DEBUG_SPECTROGRAM_REPAINT 1 using namespace std; @@ -1158,7 +1159,7 @@ } double -SpectrogramLayer::getYForBin(LayerGeometryProvider *, double bin) const { +SpectrogramLayer::getYForBin(LayerGeometryProvider *, double) const { //!!! not implemented throw std::logic_error("not implemented"); } @@ -1525,7 +1526,7 @@ SpectrogramLayer::getImageCacheReference(const LayerGeometryProvider *view) const { if (m_imageCaches.find(view->getId()) == m_imageCaches.end()) { - m_imageCaches[view->getId()] = ScrollableImageCache(view); + m_imageCaches[view->getId()] = ScrollableImageCache(); } return m_imageCaches.at(view->getId()); } @@ -1535,7 +1536,11 @@ { Colour3DPlotRenderer *renderer = getRenderer(v); - + //!!! not time-constrained for now + Colour3DPlotRenderer::RenderResult result = renderer->render(v, paint, rect); + + //!!! do + (void)result; } Colour3DPlotRenderer * @@ -1544,11 +1549,10 @@ if (m_renderers.find(v->getId()) == m_renderers.end()) { Colour3DPlotRenderer::Sources sources; - sources.geometryProvider = v; sources.verticalBinLayer = this; - sources.source = m_fftModel; - sources.peaks = m_peakCache; - sources.fft = m_fftModel; + sources.fft = getFFTModel(); + sources.source = sources.fft; + sources.peaks = getPeakCache(); ::ColourScale::Parameters cparams; //!!! todo @@ -1588,8 +1592,8 @@ SVDEBUG << "SpectrogramLayer::paint(): Layer is dormant, making it undormant again" << endl; } -// paintAlternative(v, paint, rect); -// return; + paintAlternative(v, paint, rect); + return; //!!! @@ -1671,7 +1675,7 @@ cerr << "SpectrogramLayer: scrolling the image cache if applicable" << endl; #endif - cache.scrollTo(startFrame); + cache.scrollTo(v, startFrame); #ifdef DEBUG_SPECTROGRAM_REPAINT cerr << "SpectrogramLayer: after scrolling, cache valid from " @@ -2595,7 +2599,7 @@ } int -SpectrogramLayer::getCompletion(LayerGeometryProvider *v) const +SpectrogramLayer::getCompletion(LayerGeometryProvider *) const { if (!m_fftModel) return 100; int completion = m_fftModel->getCompletion(); @@ -2606,9 +2610,8 @@ } QString -SpectrogramLayer::getError(LayerGeometryProvider *v) const +SpectrogramLayer::getError(LayerGeometryProvider *) const { - const View *view = v->getView(); if (!m_fftModel) return ""; return m_fftModel->getError(); }