On Sun, Aug 23, 2009 at 12:40 PM, Andrew
Ross<andrewr...@users.sourceforge.net> wrote:
> On Sat, Aug 22, 2009 at 06:42:21PM -0700, Alan Irwin wrote:
>> On 2009-08-22 16:57-0500 Hezekiah M. Carty wrote:
>>
>> >
>> > I do not have a working Ada install yet, so I can not test the effect [of
>> this patch]
>> > this has on the Ada bindings.  I can, however, call plscmap0n with 0,
>> > negative and positive values before calling plinit from OCaml,
>> > resulting in a proper palette of colors.
>> >
>> > If it works for you and/or others I will go ahead and commit the change.
>>
>> The patch solves the Ada colour initialization problems for me.  However,
>> the
>>
>> if (ncol0 == 0 || first_allocation == TRUE) {
>>      plspal0("");
>>     }
>>
>> logic in plscmap0n is going to sometimes have quite a different effect than
>> the old logic that always initialized colours for the part of the cmap0
>> palette that was expanded from the previous call.  For example, for the old
>> logic if the users code called plscmap0n(3), made some changes to the first
>> 3 colours, then followed by plscmap0n(13), the expanded colours (i.e., the
>> 4th through 13th colours) would get properly initialized to their default
>> values without messing with the first 3 colours. For the new logic the
>> expanded colours will just be red.  I am having a hard time wrapping my head
>> around all the implications of the above if statement so there may be other
>> user scenarios where the results are different than before.
>>
>> Therefore, I think we should do something that mimics the effect of the old
>> plscmap0n code much better.  The current coding logic is already pretty
>> messy because plspal0 calls plscmap0n and vice versa and adding logic to
>> plspal0 so that it only overwrites the expanded range of cmap0 indices is
>> only going to make it worse.  Thus, perhaps the best way out of this mess is
>> to go back to what I said I didn't want before which is hard-coded colours
>> set inside plcmap0_def (but this time with a comment they must be consistent
>> with data/cmap0_default.pal).
>>
>> If upon reflection you (and others here who are interested) also feel the
>> safest thing to do is go back to hard-coded colours, I am willing to take
>> your patched version of plctrl.c and do the editing of plscmap0n (to remove
>> the call to plspal0) and plcmap0_def (to put back the default colours) but
>> otherwise leave your patched version alone (e.g., the is_cmap0_ready logic
>> which I like better than the previous logic) to make up for the wasted work
>> I caused you.  :-)
>>
>> Let me know what you think.
>
> Let's not have a mix of palette files and hard coded otherwise it will produce
> very unexpected results where the user has changed the default palette from
> that hard coded into plplot. It may need a little more thought but the right
> solution is definitely to disentagle and loops from plspal / plscmap0n.

Here is another iteration of the patch which, hopefully, matches the
previous and documented behavior of plscmap0n.  The "full" patch
attachment provides a full patch against PLplot trunk.  The "partial"
patch attachment shows what was added to the original patch, so it
would be applied after and in addition to the original patch I posted.

This patch adds, as illustrated by the "partial" patch, a work-around
for the issue of overwritten existing cmap0 colors on calls to
plscmap0n.  While plspal0("") is still called, if there are existing
colors then they are saved prior to this call.  After the plspal0
call, the previously existing cmap0 colors are restored.

This fix is not as clean as I would prefer, given that colors are
potentially set by the user, saved, overwritten, and restored.  It
tests well here though, with setting and getting colors before plinit,
and not overwriting already defined colors when plscmap0n is called.

Hez
diff --git a/src/plctrl.c b/src/plctrl.c
index edab174..c99eeda 100644
--- a/src/plctrl.c
+++ b/src/plctrl.c
@@ -81,6 +81,9 @@ plcmap1_def(void);
 static PLFLT
 value(double n1, double n2, double hue);
 
+PLBOOL
+is_cmap0_ready(PLINT ncol0);
+
 /* An additional hardwired location for lib files. */
 /* I have no plans to change these again, ever. */
 
@@ -749,6 +752,35 @@ plcmap1_calc(void)
 }
 
 /*--------------------------------------------------------------------------*\
+ * is_cmap0_ready()
+ *
+ * Check to see if color map 0 has enough color slots allocated to house the
+ * requested number of colors.
+\*--------------------------------------------------------------------------*/
+
+PLBOOL
+is_cmap0_ready(PLINT ncol0)
+{
+    PLBOOL ret_val;
+    ret_val = FALSE;
+
+    if (ncol0 > 0 && plsc->ncol0 == ncol0) {
+        /* OK because ncol0 = the number of currently allocated cmap0 colors */
+	ret_val = TRUE;
+    }
+    else if (plsc->ncol0 <= 0 || ncol0 <= 0) {
+        /* NOT OK because cmap0 is not ready to hold any colors */
+        ret_val = FALSE;
+    }
+    else {
+        /* NOT OK because more colors are requested than are currently
+           allocated */
+        ret_val = FALSE;
+    }
+    return ret_val;
+}
+
+/*--------------------------------------------------------------------------*\
  * plscmap0n()
  *
  * Set number of colors in cmap 0, (re-)allocate cmap 0, and fill with
@@ -762,6 +794,13 @@ void
 c_plscmap0n(PLINT ncol0)
 {
     int ncol, size, imin, imax;
+    PLBOOL first_allocation;
+    int i;
+    PLINT *r, *g, *b;
+    PLFLT *a;
+
+    /* Is this the first initialization of cmap0? */
+    first_allocation = FALSE;
 
 /* No change */
 
@@ -788,6 +827,7 @@ c_plscmap0n(PLINT ncol0)
        plexit("c_plscmap0n: Insufficient memory");
      }
 	imin = 0;
+        first_allocation = TRUE;
     }
     else {
 	if ((plsc->cmap0 = (PLColor *) realloc(plsc->cmap0, size))==NULL)
@@ -795,6 +835,7 @@ c_plscmap0n(PLINT ncol0)
        plexit("c_plscmap0n: Insufficient memory");
      }
 	imin = plsc->ncol0;
+        first_allocation = FALSE;
     }
 
 /* Fill in default entries */
@@ -802,6 +843,33 @@ c_plscmap0n(PLINT ncol0)
     plsc->ncol0 = ncol;
     plcmap0_def(imin, imax);
 
+    if (ncol0 == 0 || first_allocation == TRUE) {
+        /* Save existing colors, if there are any */
+        if (imin > 0) {
+            r = (PLINT *)malloc(imin * sizeof(PLINT));
+            g = (PLINT *)malloc(imin * sizeof(PLINT));
+            b = (PLINT *)malloc(imin * sizeof(PLINT));
+            a = (PLFLT *)malloc(imin * sizeof(PLFLT));
+            for (i = 0; i <= imin; i++) {
+                plgcol0a(i, &r[i], &g[i], &b[i], &a[i]);
+            }
+        }
+
+        /* Load the default palette */
+        plspal0("");
+
+        /* Restore the (previously) existing colors */
+        if (imin > 0) {
+            for (i = 0; i <= imin; i++) {
+                plscol0a(i, r[i], g[i], b[i], a[i]);
+            }
+            free(r);
+            free(g);
+            free(b);
+            free(a);
+        }
+    }
+
     if (plsc->level > 0)
 	plP_state(PLSTATE_CMAP0);
 }
@@ -1208,11 +1276,9 @@ c_plspal0(const char *filename)
     return;
   }
 
-  /* Allocate default number of cmap0 colours if cmap0 allocation not
+  /* Allocate sufficient cmap0 colors to contain the data if it has not been
      done already. */
-  plscmap0n(0);
-  /* Allocate sufficient cmap0 colours to contain present data. */
-  if(number_colors > plsc->ncol0) {
+  if(!is_cmap0_ready(number_colors)) {
     plscmap0n(number_colors);
   }
 
diff --git a/src/plctrl.c b/src/plctrl.c
index 10189f6..c99eeda 100644
--- a/src/plctrl.c
+++ b/src/plctrl.c
@@ -795,6 +795,9 @@ c_plscmap0n(PLINT ncol0)
 {
     int ncol, size, imin, imax;
     PLBOOL first_allocation;
+    int i;
+    PLINT *r, *g, *b;
+    PLFLT *a;
 
     /* Is this the first initialization of cmap0? */
     first_allocation = FALSE;
@@ -841,7 +844,30 @@ c_plscmap0n(PLINT ncol0)
     plcmap0_def(imin, imax);
 
     if (ncol0 == 0 || first_allocation == TRUE) {
+        /* Save existing colors, if there are any */
+        if (imin > 0) {
+            r = (PLINT *)malloc(imin * sizeof(PLINT));
+            g = (PLINT *)malloc(imin * sizeof(PLINT));
+            b = (PLINT *)malloc(imin * sizeof(PLINT));
+            a = (PLFLT *)malloc(imin * sizeof(PLFLT));
+            for (i = 0; i <= imin; i++) {
+                plgcol0a(i, &r[i], &g[i], &b[i], &a[i]);
+            }
+        }
+
+        /* Load the default palette */
         plspal0("");
+
+        /* Restore the (previously) existing colors */
+        if (imin > 0) {
+            for (i = 0; i <= imin; i++) {
+                plscol0a(i, r[i], g[i], b[i], a[i]);
+            }
+            free(r);
+            free(g);
+            free(b);
+            free(a);
+        }
     }
 
     if (plsc->level > 0)
------------------------------------------------------------------------------
Let Crystal Reports handle the reporting - Free Crystal Reports 2008 30-Day 
trial. Simplify your report design, integration and deployment - and focus on 
what you do best, core application coding. Discover what's new with 
Crystal Reports now.  http://p.sf.net/sfu/bobj-july
_______________________________________________
Plplot-devel mailing list
Plplot-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/plplot-devel

Reply via email to