From 81e6cb0c1d21d767a795fa54f980526ddb560310 Mon Sep 17 00:00:00 2001 From: Igor Kushnir Date: Thu, 4 Feb 2021 20:52:18 +0200 Subject: [PATCH 1/4] YACReaderFlowGL::remove(): adjust the last index too Adjusting index of the item that is about to be removed is pointless. Not adjusting the last index caused an assertion failure (a crash) in Library built in Debug mode - when a comic other than the last in the current folder was deleted in ClassicComicsView with hardware acceleration enabled. --- common/gl/yacreader_flow_gl.cpp | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/common/gl/yacreader_flow_gl.cpp b/common/gl/yacreader_flow_gl.cpp index ad59daf73..c1db65b05 100644 --- a/common/gl/yacreader_flow_gl.cpp +++ b/common/gl/yacreader_flow_gl.cpp @@ -713,11 +713,8 @@ void YACReaderFlowGL::remove(int item) QOpenGLTexture *texture = images[item].texture; - int count = item; - while (count <= numObjects - 2) { - images[count].index--; - count++; - } + for (int i = item + 1; i < numObjects; ++i) + --images[i].index; images.removeAt(item); if (texture != defaultTexture) From 315a3e0f3f17f75000d799850084b3ce52b92097 Mon Sep 17 00:00:00 2001 From: Igor Kushnir Date: Thu, 4 Feb 2021 21:25:04 +0200 Subject: [PATCH 2/4] Remove unused parameter from YACReaderFlowGL::insert() The implementation when item != -1 seems to leak images[item].texture. Let us remove rather than fix this unused branch. Make YACReaderFlowGL::insert()'s name argument const. --- common/gl/yacreader_flow_gl.cpp | 16 +++++++--------- common/gl/yacreader_flow_gl.h | 4 +--- 2 files changed, 8 insertions(+), 12 deletions(-) diff --git a/common/gl/yacreader_flow_gl.cpp b/common/gl/yacreader_flow_gl.cpp index c1db65b05..f50362e9f 100644 --- a/common/gl/yacreader_flow_gl.cpp +++ b/common/gl/yacreader_flow_gl.cpp @@ -673,21 +673,19 @@ void YACReaderFlowGL::updatePositions() stopAnimationTimer(); } -void YACReaderFlowGL::insert(char *name, QOpenGLTexture *texture, float x, float y, int item) +void YACReaderFlowGL::insert(const char *name, QOpenGLTexture *texture, float x, float y) { startAnimationTimer(); Q_UNUSED(name) - //set a new entry - if (item == -1) { - images.push_back(YACReader3DImage()); - item = numObjects; - numObjects++; + images.push_back(YACReader3DImage()); - calcVector(images[item].current, item); - images[item].current.z = images[item].current.z - 1; - } + const auto item = numObjects; + numObjects++; + + calcVector(images[item].current, item); + images[item].current.z = images[item].current.z - 1; images[item].texture = texture; images[item].width = x; diff --git a/common/gl/yacreader_flow_gl.h b/common/gl/yacreader_flow_gl.h index 6fc965a78..5d305ffd6 100644 --- a/common/gl/yacreader_flow_gl.h +++ b/common/gl/yacreader_flow_gl.h @@ -194,9 +194,7 @@ class YACReaderFlowGL : public QOpenGLWidget, public ScrollManagement //updates the coverflow void updatePositions(); //inserts a new item to the coverflow - //if item is set to a value > -1 it updates a already set value - //otherwise a new entry is set - void insert(char *name, QOpenGLTexture *texture, float x, float y, int item = -1); + void insert(const char *name, QOpenGLTexture *texture, float x, float y); //removes a item virtual void remove(int item); //replaces the texture of the item 'item' with Tex From 91986cd4c36910487095a868fbe27121825df5e4 Mon Sep 17 00:00:00 2001 From: Igor Kushnir Date: Sat, 6 Feb 2021 18:51:43 +0200 Subject: [PATCH 3/4] Remove YACReader3DImage::index data member This index was used only in YACReaderFlowGL::drawCover(). A simple change of this function eliminates the use. The benefits of the removal: * sizeof(YACReader3DImage) is 8 bytes less now, which improves CPU cache utilization and saves a little RAM; * the code, which ensured that this index member matches the index of the image in YACReaderFlowGL::images, can be removed now; * the index member can no longer get out of sync with the index of the image. Simplify the implementation of YACReaderFlowGL::draw(). For some (most likely obsolete) reason YACReaderFlowGL::replace() checked if the index member matches the index of the image and handled the mismatch differently. I don't see how the mismatch could happen with the current implementation of YACReaderFlowGL and I couldn't trigger it during debugging. So I think this special case can be safely removed. --- common/gl/yacreader_flow_gl.cpp | 48 +++++++++++---------------------- common/gl/yacreader_flow_gl.h | 4 +-- 2 files changed, 16 insertions(+), 36 deletions(-) diff --git a/common/gl/yacreader_flow_gl.cpp b/common/gl/yacreader_flow_gl.cpp index f50362e9f..a76a08733 100644 --- a/common/gl/yacreader_flow_gl.cpp +++ b/common/gl/yacreader_flow_gl.cpp @@ -425,8 +425,10 @@ bool YACReaderFlowGL::animate(YACReader3DVector ¤tVector, YACReader3DVecto return false; } -void YACReaderFlowGL::drawCover(const YACReader3DImage &image) +void YACReaderFlowGL::drawCover(int index) { + const YACReader3DImage &image = images[index]; + float w = image.width; float h = image.height; @@ -507,9 +509,9 @@ void YACReaderFlowGL::drawCover(const YACReader3DImage &image) glEnd(); glDisable(GL_TEXTURE_2D); - if (showMarks && loaded[image.index] && marks[image.index] != Unread) { + if (showMarks && loaded[index] && marks[index] != Unread) { glEnable(GL_TEXTURE_2D); - if (marks[image.index] == Read) + if (marks[index] == Read) markTexture->bind(); else readingTexture->bind(); @@ -551,25 +553,15 @@ void YACReaderFlowGL::cleanupAnimation() void YACReaderFlowGL::draw() { - int CS = currentSelected; - int count; - + const int CS = currentSelected; //Draw right Covers - for (count = numObjects - 1; count > -1; count--) { - if (count > CS) { - drawCover(images[count]); - } - } - + for (int i = numObjects - 1; i > CS; --i) + drawCover(i); //Draw left Covers - for (count = 0; count < numObjects - 1; count++) { - if (count < CS) { - drawCover(images[count]); - } - } - + for (int i = 0; i < CS; ++i) + drawCover(i); //Draw Center Cover - drawCover(images[CS]); + drawCover(CS); } void YACReaderFlowGL::showPrevious() @@ -690,7 +682,6 @@ void YACReaderFlowGL::insert(const char *name, QOpenGLTexture *texture, float x, images[item].texture = texture; images[item].width = x; images[item].height = y; - images[item].index = item; //strcpy(cfImages[item].name,name); } @@ -710,11 +701,7 @@ void YACReaderFlowGL::remove(int item) } QOpenGLTexture *texture = images[item].texture; - - for (int i = item + 1; i < numObjects; ++i) - --images[i].index; images.removeAt(item); - if (texture != defaultTexture) delete (texture); @@ -732,13 +719,10 @@ void YACReaderFlowGL::replace(char *name, QOpenGLTexture *texture, float x, floa startAnimationTimer(); Q_UNUSED(name) - if (images[item].index == item) { - images[item].texture = texture; - images[item].width = x; - images[item].height = y; - loaded[item] = true; - } else - loaded[item] = false; + images[item].texture = texture; + images[item].width = x; + images[item].height = y; + loaded[item] = true; } void YACReaderFlowGL::populate(int n) @@ -1241,13 +1225,11 @@ void YACReaderComicFlowGL::resortCovers(QList newOrder) QVector marksNew; QVector imagesNew; - int index = 0; foreach (int i, newOrder) { pathsNew << paths.at(i); loadedNew << loaded.at(i); marksNew << marks.at(i); imagesNew << images.at(i); - imagesNew.last().index = index++; } paths = pathsNew; diff --git a/common/gl/yacreader_flow_gl.h b/common/gl/yacreader_flow_gl.h index 5d305ffd6..ac39bc404 100644 --- a/common/gl/yacreader_flow_gl.h +++ b/common/gl/yacreader_flow_gl.h @@ -36,8 +36,6 @@ struct YACReader3DImage { float width; float height; - int index; - YACReader3DVector current; YACReader3DVector animEnd; }; @@ -112,7 +110,7 @@ class YACReaderFlowGL : public QOpenGLWidget, public ScrollManagement void calcVector(YACReader3DVector &vector, int pos); //returns true if the animation is finished for Current bool animate(YACReader3DVector ¤tVector, YACReader3DVector &toVector); - void drawCover(const YACReader3DImage &image); + void drawCover(int index); void udpatePerspective(int width, int height); From ba1567260d53807ac90a32f6858c2ee021e8d998 Mon Sep 17 00:00:00 2001 From: Igor Kushnir Date: Sat, 6 Feb 2021 20:44:45 +0200 Subject: [PATCH 4/4] YACReaderPageFlowGL: remove a redundant assignment loaded[idx] is unconditionally set to true in the call to YACReaderFlowGL::replace() just before the removed line. --- common/gl/yacreader_flow_gl.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/common/gl/yacreader_flow_gl.cpp b/common/gl/yacreader_flow_gl.cpp index a76a08733..afbfa086c 100644 --- a/common/gl/yacreader_flow_gl.cpp +++ b/common/gl/yacreader_flow_gl.cpp @@ -1303,7 +1303,6 @@ void YACReaderPageFlowGL::updateImageData() float y = 1 * (float(img.height()) / img.width()); QString s = "cover"; replace(s.toLocal8Bit().data(), texture, x, y, idx); - loaded[idx] = true; } }