Skip to content

Commit

Permalink
Re-work animated image handling so that we don't open the image twice…
Browse files Browse the repository at this point in the history 
… just to learn if the filetype could be animated. This was killing performance for remote images since we would redownload them before first checking if they were cached. The list of animatable types is once again hardcoded to the gif/mng/apng extensions. There is probably a better way to handle but it would probably involve re-writing the image caching and there's no time left for that before the 0.25 freeze.
  • Loading branch information
@stuartm
stuartm committed Jan 28, 2012
1 parent 95e495a commit d25fad2
Show file tree
Hide file tree
Showing 3 changed files with 42 additions and 39 deletions.
8 changes: 4 additions & 4 deletions mythtv/libs/libmythui/mythimage.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -248,14 +248,14 @@ void MythImage::ToGreyscale()
}
}

bool MythImage::Load(MythImageReader &reader)
bool MythImage::Load(MythImageReader *reader)
{
if (!reader.canRead())
if (!reader || !reader->canRead())
return false;

QImage *im = new QImage;

if (im && reader.read(im))
if (im && reader->read(im))
{
Assign(*im);
delete im;
Expand Down Expand Up @@ -290,7 +290,7 @@ bool MythImage::Load(const QString &filename, bool scale)
im->loadFromData(data);
#if 0
else
LOG(VB_GENERAL, LOG_ERR,
LOG(VB_GENERAL, LOG_ERR,
QString("MythImage::Load failed to load remote image %1")
.arg(filename));
#endif
Expand Down
2 changes: 1 addition & 1 deletion mythtv/libs/libmythui/mythimage.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ class MUI_PUBLIC MythImage : public QImage
void Assign(const QImage &img);
void Assign(const QPixmap &pix);

bool Load(MythImageReader &reader);
bool Load(MythImageReader *reader);
bool Load(const QString &filename, bool scale = true);

void Resize(const QSize &newSize, bool preserveAspect = false);
Expand Down
71 changes: 37 additions & 34 deletions mythtv/libs/libmythui/mythuiimage.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -158,6 +158,18 @@ class ImageLoader
m_loadingImagesLock.unlock();
}

static bool SupportsAnimation(const QString &filename)
{
QString extension = filename.section('.', -1);
if (!filename.startsWith("myth://") &&
(extension == "gif" ||
extension == "apng" ||
extension == "mng"))
return true;

return false;
}

/**
* \brief Generates a unique identifying string for this image which is used
* as a key in the image cache.
Expand Down Expand Up @@ -198,16 +210,16 @@ class ImageLoader
return imagelabel;
}

static MythImage *LoadImage(MythImageReader &imageReader,
MythPainter *painter,
static MythImage *LoadImage(MythPainter *painter,
// Must be a copy for thread safety
ImageProperties imProps,
ImageCacheMode cacheMode,
// Included only to check address, could be
// replaced by generating a unique value for
// each MythUIImage object?
const MythUIImage *parent,
bool &aborted)
bool &aborted,
MythImageReader *imageReader = NULL)
{
QString cacheKey = GenImageLabel(imProps);
if (!PreLoad(cacheKey, parent))
Expand Down Expand Up @@ -236,7 +248,7 @@ class ImageLoader
bForceResize = true;
}

if (!imageReader.supportsAnimation())
if (!imageReader)
{
image = GetMythUI()->LoadCacheImage(filename, cacheKey,
painter, cacheMode);
Expand Down Expand Up @@ -266,7 +278,7 @@ class ImageLoader
image->UpRef();
bool ok = false;

if (imageReader.supportsAnimation())
if (imageReader)
ok = image->Load(imageReader);
else
ok = image->Load(filename);
Expand Down Expand Up @@ -313,7 +325,7 @@ class ImageLoader
if (imProps.isGreyscale)
image->ToGreyscale();

if (!imageReader.supportsAnimation())
if (!imageReader)
GetMythUI()->CacheImage(cacheKey, image);
}

Expand All @@ -335,8 +347,7 @@ class ImageLoader
return image;
}

static AnimationFrames *LoadAnimatedImage(MythImageReader &imageReader,
MythPainter *painter,
static AnimationFrames *LoadAnimatedImage(MythPainter *painter,
// Must be a copy for thread safety
ImageProperties imProps,
ImageCacheMode cacheMode,
Expand All @@ -350,26 +361,30 @@ class ImageLoader
QString frameFilename;
int imageCount = 1;

MythImageReader *imageReader = new MythImageReader(filename);

AnimationFrames *images = new AnimationFrames();

while (imageReader.canRead() && !aborted)
while (imageReader->canRead() && !aborted)
{
frameFilename = filename.arg(imageCount);

ImageProperties frameProps = imProps;
frameProps.filename = frameFilename;

MythImage *im = ImageLoader::LoadImage(imageReader, painter,
frameProps, cacheMode,
parent, aborted);
MythImage *im = ImageLoader::LoadImage(painter, frameProps,
cacheMode, parent, aborted,
imageReader);

if (!im)
aborted = true;

images->append(AnimationFrame(im, imageReader.nextImageDelay()));
images->append(AnimationFrame(im, imageReader->nextImageDelay()));
imageCount++;
}

delete imageReader;

return images;
}

Expand Down Expand Up @@ -446,20 +461,15 @@ class ImageLoadThread : public QRunnable
{
threadRegister("ImageLoad");
bool aborted = false;
QString tmpFilename;

if (!(m_imageProperties.filename.startsWith("myth://")))
tmpFilename = m_imageProperties.filename;

QString cacheKey = ImageLoader::GenImageLabel(m_imageProperties);
QString filename = m_imageProperties.filename;

MythImageReader imageReader(tmpFilename);

if (imageReader.supportsAnimation())
// NOTE Do NOT use MythImageReader::supportsAnimation here, it defeats
// the point of caching remote images
if (ImageLoader::SupportsAnimation(filename))
{
AnimationFrames *frames;

frames = ImageLoader::LoadAnimatedImage(imageReader, m_painter,
frames = ImageLoader::LoadAnimatedImage(m_painter,
m_imageProperties,
m_cacheMode, m_parent,
aborted);
Expand All @@ -472,7 +482,7 @@ class ImageLoadThread : public QRunnable
}
else
{
MythImage *image = ImageLoader::LoadImage(imageReader, m_painter,
MythImage *image = ImageLoader::LoadImage(m_painter,
m_imageProperties,
m_cacheMode, m_parent,
aborted);
Expand Down Expand Up @@ -999,20 +1009,13 @@ bool MythUIImage::Load(bool allowLoadInBackground, bool forceStat)
// Perform a blocking load
LOG(VB_GUI | VB_FILE, LOG_DEBUG, LOC +
QString("Load(), loading '%1' in foreground").arg(filename));
QString tmpFilename;
bool aborted = false;

if (!(filename.startsWith("myth://")))
tmpFilename = filename;

MythImageReader imageReader(tmpFilename);

if (imageReader.supportsAnimation())
if (ImageLoader::SupportsAnimation(filename))
{
AnimationFrames *myFrames;

myFrames = ImageLoader::LoadAnimatedImage(imageReader,
GetPainter(), imProps,
myFrames = ImageLoader::LoadAnimatedImage(GetPainter(), imProps,
static_cast<ImageCacheMode>(cacheMode2),
this, aborted);

Expand All @@ -1030,7 +1033,7 @@ bool MythUIImage::Load(bool allowLoadInBackground, bool forceStat)
{
MythImage *image = NULL;

image = ImageLoader::LoadImage(imageReader, GetPainter(),
image = ImageLoader::LoadImage(GetPainter(),
imProps,
static_cast<ImageCacheMode>(cacheMode2),
this, aborted);
Expand Down

0 comments on commit d25fad2

Please sign in to comment.