Permalink
Browse files

Move list insertion/deletion operations into a new file,

Move list insertion/deletion operations into a new file, phoimagelist.c.
Clean up operations that change the image list, most notably NextImage().
Add a couple of null checks.
Add the test images, and a file suggesting some regression tests to run.
  • Loading branch information...
1 parent 7d0d9e8 commit ff9f901b720a8aaf6d738af23ee503a6a044a42d @akkana committed Apr 10, 2010
View
@@ -23,7 +23,7 @@ TARFILE = pho-$(VERSION).tar.gz
EXIFLIB = exif/libphoexif.a
-SRCS = pho.c gmain.c gwin.c imagenote.c gdialogs.c keydialog.c
+SRCS = pho.c gmain.c phoimglist.c gwin.c imagenote.c gdialogs.c keydialog.c
# winman.c
View
@@ -0,0 +1,44 @@
+
+Important regression tests:
+
+LIST HANDLING TESTS
+
+List of one image
+List of one image: delete it
+
+List of two images, delete first
+List of two images, delete second
+
+List consisting of only a bogus item
+List with bogus first item
+List with bogus item in the middle
+List with bogus last item
+
+pho
+
+pho ?.jpg
+
+pho 1.jpg
+pho 1.jpg 2.jpg
+cp delete.jpg a.jpg
+pho a.jpg
+cp delete.jpg a.jpg; cp delete.jpg b.jpg
+pho a.jpg 1.jpg b.jpg
+
+pho nosuchfile 1.jpg 6.jpg noneatall
+
+ROTATION TESTS
+
+Start with a larger-than-screen image, and rotate it 4 ways,
+ go forward then back
+Start with a file with EXIF rotation, rotate it 4 ways, go forward then back
+Start with a file with EXIF orientation, rotate it 4 ways, forward then back
+
+SCALING/MODE TESTS
+
+(something to test fullscreen and fullsize modes)
+
+go between presentation and normal mode, while advancing images.
+
+
+
View
@@ -247,22 +247,7 @@ PhoImage* AddImage(char* filename)
exit(1);
}
/* Make img the new last image in the list */
- if (gFirstImage == 0) {
- gFirstImage = img->next = img->prev = img;
- }
- else {
- PhoImage* lastImg = gFirstImage->prev;
- if (lastImg == gFirstImage || lastImg == 0) { /* only 1 img in list */
- gFirstImage->next = img;
- img->prev = gFirstImage;
- }
- else {
- lastImg->next = img;
- img->prev = lastImg;
- }
- gFirstImage->prev = img;
- img->next = gFirstImage;
- }
+ AppendItem(img);
return img;
}
View
@@ -36,15 +36,6 @@ int gDisplayMode = PHO_DISPLAY_NORMAL;
int gDelaySeconds = 0;
int gPendingTimeout = 0;
-/* This routine exists to keep track of any allocated memory
- * existing in the PhoImage structure.
- */
-static void FreePhoImage(PhoImage* img)
-{
- if (img->comment) free(img->comment);
- free(img);
-}
-
static int RotateImage(PhoImage* img, int degrees); /* forward */
static gint DelayTimer(gpointer data)
@@ -127,8 +118,10 @@ static int LoadImageFromFile(PhoImage* img)
static int LoadImageAndRotate(PhoImage* img)
{
int e;
- int rot = img->curRot;
- int firsttime = (img->trueWidth == 0);
+ int rot = (img ? img->curRot : 0);
+ int firsttime = (img && (img->trueWidth == 0));
+
+ if (!img) return -1;
img->trueWidth = img->trueHeight = img->curRot = 0;
@@ -161,14 +154,30 @@ int ThisImage()
int NextImage()
{
- PhoImage* curSav = gCurImage;
int retval = 0;
+ int looping = 0;
if (gDebug)
printf("\n================= NextImage ====================\n");
- while (1) {
- if (gCurImage == 0) /* no image loaded yet, first call */
- gCurImage = curSav = gFirstImage;
+ /* Loop, since images may fail to load
+ * and may need to be deleted from the list
+ */
+ while (1)
+ {
+ if (gFirstImage == 0)
+ /* There's no list! How can we go to the next item? */
+ return -1;
+
+ if (gCurImage == 0) { /* no image loaded yet, first call */
+ if (gDebug)
+ printf("NextImage: going to first image\n");
+ gCurImage = gFirstImage;
+ }
+
+ else if (looping && gCurImage->next == gFirstImage)
+ /* We're to the end of the list, after deleting something bogus */
+ return -1;
+
else if ((gCurImage->next == 0) || (gCurImage->next == gFirstImage))
/* We're at the end of the list, can't go farther.
* However, we may have gotten here by trying to go to
@@ -177,22 +186,26 @@ int NextImage()
* but we'll want to return -1 to indicate we didn't progress.
*/
retval = -1;
- else
+
+ /* We only want to go to ->next the first time;
+ * if we're looping because of an error, gCurImage is already set.
+ */
+ else if (!looping)
gCurImage = gCurImage->next;
if (LoadImageAndRotate(gCurImage) == 0) { /* Success! */
ShowImage();
return retval;
}
- /* The image didn't load. Remove it from the list. */
+ /* The image didn't load. Remove it from the list.
+ * That means we're going to loop around and try again,
+ * so keep gCurImage where it is (but change gCurImage->next).
+ */
if (gDebug)
- printf("Removing '%s' from list\n", gCurImage->filename);
- curSav->next = gCurImage->next;
- if (gCurImage->next)
- gCurImage->next->prev = curSav;
- FreePhoImage(gCurImage);
- gCurImage = curSav;
+ printf("Skipping '%s' (didn't load)\n", gCurImage->filename);
+ DeleteItem(gCurImage);
+ looping = 1;
}
/* NOTREACHED */
return 0;
@@ -491,19 +504,6 @@ PhoImage* NewPhoImage(char* fnam)
return newimg;
}
-/* Remove all images from the image list, to start fresh. */
-void ClearImageList()
-{
- PhoImage* img = gFirstImage;
- do {
- PhoImage* next = img->next;
- FreePhoImage(img);
- img = next;
- } while (img && img != gFirstImage);
-
- gCurImage = gFirstImage = 0;
-}
-
void ReallyDelete(PhoImage* delImg)
{
if (unlink(delImg->filename) < 0)
@@ -512,47 +512,11 @@ void ReallyDelete(PhoImage* delImg)
return;
}
- /* next and prev should never be 0, but check anyway */
- if (delImg->next == 0)
- printf("BUG: delImg->next is zero!\n");
- if (delImg->prev == 0)
- printf("BUG: delImg->prev is zero!\n");
+ DeleteItem(delImg);
- /* Disconnect it from the linked list, and reset gCurImage. */
- if (delImg == gFirstImage /* this is the only image! */
- && (delImg->next == gFirstImage || delImg->next == 0)) {
+ /* If we just deleted the last image, all we can do is quit */
+ if (!gFirstImage)
EndSession();
- }
- else if (delImg->prev == delImg->next) { /* One image left after this */
- gFirstImage = gCurImage = delImg->prev;
- gCurImage->prev = gCurImage->next = gCurImage;
- }
- else if (delImg->next == gFirstImage) { /* Last image, so go back */
- gCurImage = delImg->prev;
- gCurImage->next = gFirstImage;
- gFirstImage->prev = gCurImage;
- }
- else {
- gCurImage = delImg->next;
- gCurImage->prev = delImg->prev;
- if (delImg->prev == 0) {
- printf("BUG: delImg->prev == 0!\n");
- delImg->prev = gFirstImage;
- }
- delImg->prev->next = gCurImage;
- if (gCurImage->next == 0) {
- printf("BUG: delImg->prev == 0!\n");
- gCurImage->next = gFirstImage;
- }
- gCurImage->next->prev = gCurImage;
- }
- /* If we deleted the first image, make sure we reset the list */
- if (delImg == gFirstImage) {
- gFirstImage = delImg->next;
- }
-
- /* It's disconnected. Free all the memory */
- FreePhoImage(delImg);
ThisImage();
}
View
@@ -86,6 +86,12 @@ extern int gDisplayMode;
*/
extern void SetViewModes(int dispmode, int scalemode, double scalefactor);
+/* ************** List maintenance functions ************** */
+extern void DeleteItem(PhoImage* item);
+extern void AppendItem(PhoImage* item);
+extern void ClearImageList();
+
+/* ************** Misc. functions ************** */
/* Some window managers don't deal well with windows that resize,
* or don't retain focus if a resized window no longer contains
* the mouse pointer. This allows making new windows instead.
View
@@ -0,0 +1,110 @@
+/* -*- Mode: C; tab-width: 4; indent-tabs-mode: nil; c-basic-offset: 4 -*- */
+/*
+ * phoimglist.c: list manipulation for PhoImage type.
+ *
+ * Copyright 2010 by Akkana Peck.
+ * You are free to use or modify this code under the Gnu Public License.
+ */
+
+/* A PhoImage is a doubly-linked list, with ->next and ->prev.
+ * The global gFirstImage marks the head of the list;
+ * the list is circular, so the last item has next = gFirstImage.
+ *
+ * gCurImage points to the current list item.
+ *
+ * List items are freed with FreePhoImage()
+ */
+
+#include "pho.h"
+#include <stdlib.h>
+
+/* This routine exists to keep track of any allocated memory
+ * existing in the PhoImage structure.
+ */
+static void FreePhoImage(PhoImage* img)
+{
+ if (img->comment) free(img->comment);
+ free(img);
+}
+
+/* Delete an image, or gCurImage if item == 0.
+ * Update gCurImage if need be.
+ */
+void DeleteItem(PhoImage* item)
+{
+ if (!item)
+ item = gCurImage;
+
+ /* Is this the only image? */
+ if (item == gFirstImage && item->next == gFirstImage) {
+ gFirstImage = gCurImage = 0;
+ }
+ else {
+ PhoImage* newitem = item->next;
+
+ newitem->prev = item->prev;
+ newitem->prev->next = newitem;
+
+ /* Only change gCurImage if item was the current image. */
+ if (item == gCurImage) {
+ /* If we just deleted the last item, newitem will be gFirstImage.
+ * But we don't really want to loop around; we should stop
+ * upon reaching the last image, and stay on the new last image.
+ * Of course, this assumes that there's more than one item left.
+ */
+ if (newitem == gFirstImage && newitem->prev != gFirstImage)
+ gCurImage = newitem->prev;
+ else
+ gCurImage = newitem;
+ }
+
+ if (item == gFirstImage)
+ gFirstImage = newitem;
+ }
+
+ /* It's disconnected. Free all the memory */
+ FreePhoImage(item);
+}
+
+/* Append an item to the end of the list */
+void AppendItem(PhoImage* item)
+{
+ PhoImage* last;
+
+ if (!item)
+ return;
+
+ /* Is the list empty? */
+ if (gFirstImage == 0) {
+ gFirstImage = item;
+ item->next = item->prev = item;
+ return;
+ }
+
+ /* Is there only one item in the list? */
+ if (gFirstImage->next == gFirstImage) {
+ gFirstImage->next = gFirstImage->prev = item;
+ item->next = item->prev = gFirstImage;
+ return;
+ }
+
+ last = gFirstImage->prev;
+ last->next = item;
+ item->prev = last;
+ item->next = gFirstImage;
+ gFirstImage->prev = item;
+}
+
+/* Remove all images from the image list, to start fresh. */
+void ClearImageList()
+{
+ PhoImage* img = gFirstImage;
+ do {
+ PhoImage* next = img->next;
+ FreePhoImage(img);
+ img = next;
+ } while (img && img != gFirstImage);
+
+ gCurImage = gFirstImage = 0;
+}
+
View
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
View
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
View
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
View
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
View
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
View
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
View
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.

0 comments on commit ff9f901

Please sign in to comment.