a color cache to be proud of!

8 files changed, 138 insertions, 239 deletions

diff --git a/otk_c/Makefile b/otk_c/Makefile
index f0052b41..b9253343 100644
--- a/otk_c/Makefile
+++ b/otk_c/Makefile
@@ -6,7 +6,7 @@ targets = libotk.so libotk.a
 sources = display.c screeninfo.c rect.c gccache.c color.c
 headers = display.h screeninfo.h rect.h gccache.h color.h
 
-CFLAGS+=-I/usr/gwar/include/python2.2
+CFLAGS+=-g -I/usr/gwar/include/python2.2 -W -Wall
 
 .PHONY: all install clean
 
diff --git a/otk_c/color.c b/otk_c/color.c
index 9f718217..593687d2 100644
--- a/otk_c/color.c
+++ b/otk_c/color.c
@@ -10,30 +10,21 @@
 #endif
 
 static Bool cleancache = False;
-static PyObject *colorcache;
+static PyObject *colorcache = NULL;
 
-// global color allocator/deallocator
-typedef struct RGB {
-  PyObject_HEAD
-  int screen;
-  int r, g, b;
-} RGB;
-
-static void rgb_dealloc(PyObject* self)
+static void otkcolor_dealloc(OtkColor* self)
 {
-  PyObject_Del(self);
+  // when this is called, the color has already been cleaned out of the cache
+  PyObject_Del((PyObject*)self);
 }
 
-static int rgb_compare(PyObject *py1, PyObject *py2)
+static int otkcolor_compare(OtkColor *c1, OtkColor *c2)
 {
   long result;
   unsigned long p1, p2;
-  RGB *r1, *r2;
 
-  r1 = (RGB*) r1;
-  r2 = (RGB*) r2;
-  p1 = (r1->screen << 24 | r1->r << 16 | r1->g << 8 | r1->b) & 0x00ffffff;
-  p2 = (r2->screen << 24 | r2->r << 16 | r2->g << 8 | r2->b) & 0x00ffffff;
+  p1 = c1->red << 16 | c1->green << 8 | c1->blue;
+  p2 = c2->red << 16 | c2->green << 8 | c2->blue;
 
   if (p1 < p2)
     result = -1;
@@ -44,193 +35,145 @@ static int rgb_compare(PyObject *py1, PyObject *py2)
   return result;
 }
 
-static PyTypeObject RGB_Type = {
+static PyObject *otkcolor_repr(OtkColor *self)
+{
+  return PyString_FromFormat("rgb:%x/%x/%x", self->red, self->green,
+                             self->blue);
+}
+
+static long otkcolor_hash(OtkColor *self)
+{
+  return self->screen << 24 | self->red << 16 | self->green << 8 | self->blue;
+}
+
+static PyTypeObject OtkColor_Type = {
   PyObject_HEAD_INIT(NULL)
   0,
-  "RGB",
-  sizeof(RGB),
+  "Color",
+  sizeof(OtkColor),
   0,
-  rgb_dealloc, /*tp_dealloc*/
-  0,          /*tp_print*/
-  0,          /*tp_getattr*/
-  0,          /*tp_setattr*/
-  rgb_compare,          /*tp_compare*/
-  0,          /*tp_repr*/
-  0,          /*tp_as_number*/
-  0,          /*tp_as_sequence*/
-  0,          /*tp_as_mapping*/
-  0,          /*tp_hash */
+  (destructor)otkcolor_dealloc, /*tp_dealloc*/
+  0,                            /*tp_print*/
+  0,                            /*tp_getattr*/
+  0,                            /*tp_setattr*/
+  (cmpfunc)otkcolor_compare,    /*tp_compare*/
+  (reprfunc)otkcolor_repr,      /*tp_repr*/
+  0,                            /*tp_as_number*/
+  0,                            /*tp_as_sequence*/
+  0,                            /*tp_as_mapping*/
+  (hashfunc)otkcolor_hash,      /*tp_hash */
 };
 
-static PyObject *RGB_New(int screen, int r, int g, int b) {
-  RGB *self = (RGB*) PyObject_New(RGB, &RGB_Type);
-  self->screen = screen;
-  self->r = r;
-  self->g = g;
-  self->b = b;
-  return (PyObject*)self;
-}
 
-typedef struct PixelRef {
-  unsigned long p;
-  unsigned int count;
-} PixelRef;
-
-static PixelRef *PixelRef_New(unsigned long p) {
-  PixelRef* self = malloc(sizeof(PixelRef));
-  self->p = p;
-  self->count = 1;
-  return self;
-}
-
-static void OtkColor_ParseColorName(OtkColor *self) {
+static void parseColorName(OtkColor *self, const char *name) {
   XColor xcol;
 
-  if (!self->colorname) {
-    fprintf(stderr, "OtkColor: empty colorname, cannot parse (using black)\n");
-    OtkColor_SetRGB(self, 0, 0, 0);
-  }
-
   // get rgb values from colorname
   xcol.red = 0;
   xcol.green = 0;
   xcol.blue = 0;
   xcol.pixel = 0;
 
-  if (!XParseColor(OBDisplay->display, self->colormap,
-		   PyString_AsString(self->colorname), &xcol)) {
-    fprintf(stderr, "BColor::allocate: color parse error: \"%s\"\n",
-            PyString_AsString(self->colorname));
-    OtkColor_SetRGB(self, 0, 0, 0);
-    return;
+  if (!XParseColor(OBDisplay->display,
+                   OtkDisplay_ScreenInfo(OBDisplay, self->screen)->colormap,
+                   name, &xcol)) {
+    fprintf(stderr, "OtkColor: color parse error: \"%s\"\n", name);
+    self->red = self->green = self->blue = 0;
+  } else {
+    self->red = xcol.red >> 8;
+    self->green = xcol.green >> 8;
+    self->blue = xcol.blue >> 8;
   }
-
-  OtkColor_SetRGB(self, xcol.red >> 8, xcol.green >> 8, xcol.blue >> 8);
 }
 
-static void OtkColor_DoCacheCleanup() {
+#include <stdio.h>
+static void doCacheCleanup() {
   unsigned long *pixels;
-  int i;
+  int i, ppos;
   unsigned int count;
-  PyObject *rgb, *pixref;
-  int ppos;
+  PyObject *key; // this is a color too, but i dont need to use it as such
+  OtkColor *color;
 
   // ### TODO - support multiple displays!
-  if (!PyDict_Size(colorcache)) {
-    // nothing to do
-    return;
-  }
+  if (!PyDict_Size(colorcache)) return; // nothing to do
 
+  printf("Cleaning Cache...\n");
+  
   pixels = malloc(sizeof(unsigned long) * PyDict_Size(colorcache));
 
   for (i = 0; i < ScreenCount(OBDisplay->display); i++) {
+    printf("Screen %d\n", i);
     count = 0;
     ppos = 0;
 
-    while (PyDict_Next(colorcache, &ppos, &rgb, &pixref)) {
-      if (((PixelRef*)pixref)->count != 0 || ((RGB*)rgb)->screen != i)
+    while (PyDict_Next(colorcache, &ppos, &key, (PyObject**)&color)) {
+      // get the screen from the hash
+      if (color->screen != i) continue; // wrong screen
+
+      printf("has %d refs\n", color->ob_refcnt);
+
+      // does someone other than the cache have a reference? (the cache gets 2)
+      if (color->ob_refcnt > 2)
         continue;
 
-      pixels[count++] = ((PixelRef*)pixref)->p;
-      PyDict_DelItem(colorcache, rgb);
-      free(pixref); // not really a PyObject, it just pretends
+      printf("ppos: %d\n", ppos);
+      printf("Cleaning pixel: %lx Count: %d\n", color->pixel, count+1);
+      
+      pixels[count++] = color->pixel;
+      printf("pixref references before: %d\n", color->ob_refcnt);
+      PyDict_DelItem(colorcache, key);
+      printf("pixref references after: %d\n", color->ob_refcnt);
       --ppos; // back up one in the iteration
     }
 
     if (count > 0)
       XFreeColors(OBDisplay->display,
-		  OtkDisplay_ScreenInfo(OBDisplay, i)->colormap,
+                  OtkDisplay_ScreenInfo(OBDisplay, i)->colormap,
                   pixels, count, 0);
+    printf("Done Cleaning Cache. Cleaned %d pixels\n", count);
   }
 
   free(pixels);
   cleancache = False;
 }
 
-static void OtkColor_Allocate(OtkColor *self) {
+static void allocate(OtkColor *self) {
   XColor xcol;
-  PyObject *rgb, *pixref;
-
-  if (!OtkColor_IsValid(self)) {
-    if (!self->colorname) {
-      fprintf(stderr, "BColor: cannot allocate invalid color (using black)\n");
-      OtkColor_SetRGB(self, 0, 0, 0);
-    } else {
-      OtkColor_ParseColorName(self);
-    }
-  }
 
-  // see if we have allocated this color before
-  rgb = RGB_New(self->screen, self->red, self->green, self->blue);
-  pixref = PyDict_GetItem((PyObject*)colorcache, rgb);
-  if (pixref) {
-    // found
-    self->allocated = True;
-    self->pixel = ((PixelRef*)pixref)->p;
-    ((PixelRef*)pixref)->count++;
-    return;
-  }
+  assert(!self->allocated);
+
+  printf("allocating! %d\n", cleancache);
 
   // allocate color from rgb values
   xcol.red =   self->red   | self->red   << 8;
   xcol.green = self->green | self->green << 8;
   xcol.blue =  self->blue  | self->blue  << 8;
   xcol.pixel = 0;
-
-  if (!XAllocColor(OBDisplay->display, self->colormap, &xcol)) {
-    fprintf(stderr, "BColor::allocate: color alloc error: rgb:%x/%x/%x\n",
+  
+  if (!XAllocColor(OBDisplay->display,
+                   OtkDisplay_ScreenInfo(OBDisplay, self->screen)->colormap,
+                   &xcol)) {
+    fprintf(stderr, "OtkColor: color alloc error: rgb:%x/%x/%x\n",
             self->red, self->green, self->blue);
     xcol.pixel = 0;
   }
-
+  
   self->pixel = xcol.pixel;
   self->allocated = True;
-
-  PyDict_SetItem(colorcache, rgb, (PyObject*)PixelRef_New(self->pixel));
-
-  if (cleancache)
-    OtkColor_DoCacheCleanup();
-}
-
-static void OtkColor_Deallocate(OtkColor *self) {
-  PyObject *rgb, *pixref;
-
-  if (!self->allocated)
-    return;
-
-  rgb = RGB_New(self->screen, self->red, self->green, self->blue);
-  pixref = PyDict_GetItem(colorcache, rgb);
-  if (pixref) {
-    if (((PixelRef*)pixref)->count >= 1)
-      ((PixelRef*)pixref)->count--;
-  }
-
+  
   if (cleancache)
-    OtkColor_DoCacheCleanup();
-
-  self->allocated = False;
+    doCacheCleanup();
 }
 
-
-OtkColor *OtkColor_New(int screen)
+PyObject *OtkColor_FromRGB(int r, int g, int b, int screen)
 {
-  OtkColor *self = malloc(sizeof(OtkColor));
+  OtkColor *self = PyObject_New(OtkColor, &OtkColor_Type);
+  PyObject *cached;
 
-  self->allocated = False;
-  self->red = -1;
-  self->green = -1;
-  self->blue = -1;
-  self->pixel = 0;
-  self->screen = screen;
-  self->colorname = NULL;
-  self->colormap = OtkDisplay_ScreenInfo(OBDisplay, self->screen)->colormap;
+  assert(screen >= 0); assert(r >= 0); assert(g >= 0); assert(b >= 0);
+  assert(r <= 0xff); assert(g <= 0xff); assert(b <= 0xff);
 
-  return self;
-}
-
-OtkColor *OtkColor_FromRGB(int r, int g, int b, int screen)
-{
-  OtkColor *self = malloc(sizeof(OtkColor));
+  if (!colorcache) colorcache = PyDict_New();
 
   self->allocated = False;
   self->red = r;
@@ -238,83 +181,56 @@ OtkColor *OtkColor_FromRGB(int r, int g, int b, int screen)
   self->blue = b;
   self->pixel = 0;
   self->screen = screen;
-  self->colorname = NULL;
-  self->colormap = OtkDisplay_ScreenInfo(OBDisplay, self->screen)->colormap;
 
-  return self;
+  // does this color already exist in the cache?
+  cached = PyDict_GetItem(colorcache, (PyObject*)self);
+  if (cached) {
+    Py_INCREF(cached);
+    return cached;
+  }
+
+  // add it to the cache
+  PyDict_SetItem(colorcache, (PyObject*)self, (PyObject*)self);
+  return (PyObject*)self;
 }
 
-OtkColor *OtkColor_FromName(const char *name, int screen)
+PyObject *OtkColor_FromName(const char *name, int screen)
 {
-  OtkColor *self = malloc(sizeof(OtkColor));
+  OtkColor *self = PyObject_New(OtkColor, &OtkColor_Type);
+  PyObject *cached;
 
+  assert(screen >= 0); assert(name);
+
+  if (!colorcache) colorcache = PyDict_New();
+  
   self->allocated = False;
   self->red = -1;
   self->green = -1;
   self->blue = -1;
   self->pixel = 0;
   self->screen = screen;
-  self->colorname = PyString_FromString(name);
-  self->colormap = OtkDisplay_ScreenInfo(OBDisplay, self->screen)->colormap;
 
-  return self;
-}
+  parseColorName(self, name);
 
-void OtkColor_Destroy(OtkColor *self)
-{
-  if (self->colorname)
-    Py_DECREF(self->colorname);
-  free(self);
-}
-
-void OtkColor_SetRGB(OtkColor *self, int r, int g, int b)
-{
-  OtkColor_Deallocate(self);
-  self->red = r;
-  self->green = g;
-  self->blue = b;
-}
-
-void OtkColor_SetScreen(OtkColor *self, int screen)
-{
-  if (screen == self->screen) {
-    // nothing to do
-    return;
+  // does this color already exist in the cache?
+  cached = PyDict_GetItem(colorcache, (PyObject*)self);
+  if (cached) {
+    Py_INCREF(cached);
+    return cached;
   }
 
-  Otk_Deallocate(self);
-  
-  self->colormap = OtkDisplay_ScreenInfo(OBDisplay, self->screen)->colormap;
-
-  self->screen = screen;
-
-  if (self->colorname)
-    parseColorName();
-}
-
-Bool OtkColor_IsValid(OtkColor *self)
-{
-  return self->red != -1 && self->blue != -1 && self->green != -1;
+  // add it to the cache
+  PyDict_SetItem(colorcache, (PyObject*)self, (PyObject*)self);
+  return (PyObject*)self;
 }
 
 unsigned long OtkColor_Pixel(OtkColor *self)
 {
   if (!self->allocated)
-    OtkColor_Allocate(self);
+    allocate(self);
   return self->pixel;
 }
 
-void OtkColor_InitializeCache()
-{
-  colorcache = PyDict_New();
-}
-
-void OtkColor_DestroyCache()
-{
-  Py_DECREF(colorcache);
-  colorcache = NULL;
-}
-
 void OtkColor_CleanupColorCache()
 {
   cleancache = True;
diff --git a/otk_c/color.h b/otk_c/color.h
index 91b83e01..6e9f421b 100644
--- a/otk_c/color.h
+++ b/otk_c/color.h
@@ -5,29 +5,20 @@
 #include <X11/Xlib.h>
 #include <Python.h>
 
-
+//!  OtkColor objects are immutable. DONT CHANGE THEM.
 typedef struct OtkColor {
+  PyObject_HEAD
   int red, green, blue;
   int screen;
   Bool allocated;
   unsigned long pixel;
-  PyObject *colorname; // PyStringObject
-  Colormap colormap;
 } OtkColor;
 
-OtkColor *OtkColor_New(int screen);
-OtkColor *OtkColor_FromRGB(int r, int g, int b, int screen);
-OtkColor *OtkColor_FromName(const char *name, int screen);
-
-void OtkColor_Destroy(OtkColor *self);
+PyObject *OtkColor_FromRGB(int r, int g, int b, int screen);
+PyObject *OtkColor_FromName(const char *name, int screen);
 
-void OtkColor_SetRGB(OtkColor *self, int r, int g, int b);
-void OtkColor_SetScreen(OtkColor *self, int screen);
-Bool OtkColor_IsValid(OtkColor *self);
 unsigned long OtkColor_Pixel(OtkColor *self);
 
-void OtkColor_InitializeCache();
-void OtkColor_DestroyCache();
 void OtkColor_CleanupColorCache();
 
 #endif // __color_h
diff --git a/otk_c/display.c b/otk_c/display.c
index 47b1191f..5efe7197 100644
--- a/otk_c/display.c
+++ b/otk_c/display.c
@@ -33,7 +33,9 @@ extern PyTypeObject OtkDisplay_Type;
 
 static int xerrorHandler(Display *d, XErrorEvent *e);
 
-PyObject *OtkDisplay_New(char *name)
+struct OtkDisplay *OBDisplay = NULL;
+
+void OtkDisplay_Initialize(char *name)
 {
   OtkDisplay* self;
   PyObject *disp_env;
@@ -117,14 +119,13 @@ line argument.\n\n"));
   self->mask_list[6] = ScrollLockMask | NumLockMask;
   self->mask_list[7] = ScrollLockMask | LockMask | NumLockMask;
 
+  // set the global var, for the new screeninfo's
+  OBDisplay = self;
+  
   // Get information on all the screens which are available.
   self->screenInfoList = PyList_New(ScreenCount(self->display));
   for (i = 0; i < ScreenCount(self->display); ++i)
     PyList_SetItem(self->screenInfoList, i, OtkScreenInfo_New(i));
-
-  self->gccache = OtkGCCache_New(PyList_Size(self->screenInfoList));
-  
-  return (PyObject*)self;
 }
 
 void OtkDisplay_Grab(OtkDisplay *self)
diff --git a/otk_c/display.h b/otk_c/display.h
index a59d8482..a36cd853 100644
--- a/otk_c/display.h
+++ b/otk_c/display.h
@@ -6,10 +6,9 @@
 #include <Python.h>
 
 struct OtkScreenInfo;
-struct OtkGCCache;
 struct OtkDisplay;
 
-struct OtkDisplay *OBDisplay; // the global display XXX: move this to app.h and ob.h?
+extern struct OtkDisplay *OBDisplay; // the global display XXX: move this to app.h and ob.h?
 
 typedef struct OtkDisplay {
   PyObject_HEAD
@@ -35,25 +34,15 @@ typedef struct OtkDisplay {
 
   //! A list of information for all screens on the display
   PyObject *screenInfoList; // PyListObject
-
-  //! A cache for re-using GCs, used by the drawing objects
-  /*!
-    @see BPen
-    @see BFont
-    @see BImage
-    @see BImageControl
-    @see BTexture
-  */
-  struct OtkGCCache *gccache;
 } OtkDisplay;
 
-//! Creates a struct, opens the X display
+//! Opens the X display, and sets the global OBDisplay variable
 /*!
   @see OBDisplay::display
   @param name The name of the X display to open. If it is null, the DISPLAY
   environment variable is used instead.
 */
-PyObject *OtkDisplay_New(char *name);
+void OtkDisplay_Initialize(char *name);
 
 //! Grabs the display
 void OtkDisplay_Grab(OtkDisplay *self);
diff --git a/otk_c/gccache.c b/otk_c/gccache.c
index 7d96677c..71232bf4 100644
--- a/otk_c/gccache.c
+++ b/otk_c/gccache.c
@@ -8,7 +8,7 @@
 # include <stdlib.h>
 #endif
 
-static OtkGCCache *gccache;
+static OtkGCCache *gccache = NULL;
 
 OtkGCCacheContext *OtkGCCacheContext_New()
 {
@@ -81,12 +81,14 @@ OtkGCCacheItem *OtkGCCacheItem_New()
   self->count = 0;
   self->hits = 0;
   self->fault = False;
+
+  return self;
 }
 
 
 void OtkGCCache_Initialize(int screen_count)
 {
-  int i;
+  unsigned int i;
 
   gccache = malloc(sizeof(OtkGCCache));
 
@@ -108,7 +110,7 @@ void OtkGCCache_Initialize(int screen_count)
 
 void OtkGCCache_Destroy()
 {
-  int i;
+  unsigned int i;
 
   for (i = 0; i < gccache->context_count; ++i)
     OtkGCCacheContext_Destroy(gccache->contexts[i]);
@@ -126,7 +128,7 @@ OtkGCCacheContext *OtkGCCache_NextContext(int screen)
 {
   Window hd = OtkDisplay_ScreenInfo(OBDisplay, screen)->root_window;
   OtkGCCacheContext *c;
-  int i;
+  unsigned int i;
 
   for (i = 0; i < gccache->context_count; ++i) {
     c = gccache->contexts[i];
@@ -155,10 +157,10 @@ OtkGCCacheItem *OtkGCCache_Find(OtkColor *color, XFontStruct *font,
 				int function, int subwindow, int linewidth)
 {
   const unsigned long pixel = OtkColor_Pixel(color);
-  const unsigned int screen = color->screen;
+  const int screen = color->screen;
   const int key = color->red ^ color->green ^ color->blue;
   int k = (key % gccache->cache_size) * gccache->cache_buckets;
-  int i = 0; // loop variable
+  unsigned int i = 0; // loop variable
   OtkGCCacheItem *c = gccache->cache[k], *prev = 0;
 
   /*
@@ -219,13 +221,13 @@ void OtkGCCache_Release(OtkGCCacheItem *item)
 
 void OtkGCCache_Purge()
 {
-  int i;
+  unsigned int i;
   
   for (i = 0; i < gccache->cache_total_size; ++i) {
     OtkGCCacheItem *d = gccache->cache[i];
 
     if (d->ctx && d->count == 0) {
-      release(d->ctx);
+      OtkGCCache_InternalRelease(d->ctx);
       d->ctx = 0;
     }
   }
diff --git a/otk_c/gccache.h b/otk_c/gccache.h
index f6657f48..ccd95524 100644
--- a/otk_c/gccache.h
+++ b/otk_c/gccache.h
@@ -16,7 +16,7 @@ typedef struct OtkGCCacheContext {
   int function;
   int subwindow;
   Bool used;
-  unsigned int screen;
+  int screen;
   int linewidth;
 } OtkGCCacheContext;
 
diff --git a/otk_c/screeninfo.c b/otk_c/screeninfo.c
index fb0de498..3d253d1b 100644
--- a/otk_c/screeninfo.c
+++ b/otk_c/screeninfo.c
@@ -85,7 +85,7 @@ PyObject *OtkScreenInfo_New(int num)
   if (dstr2) {
     PyObject *str;
     
-    PyString_Resize(self->display_string, dstr2 - dstr);
+    _PyString_Resize(&self->display_string, dstr2 - dstr);
     str = PyString_FromFormat(".%d", self->screen);
     PyString_Concat(&self->display_string, str);
   }