From: Alan W. I. <Ala...@gm...> - 2018-11-22 22:49:51
|
On 2018-11-20 11:15-0700 Steve Schwartz wrote: > Hi Plplot > > > > We have been pretty quiet, happily employing plplot in our QSAS software. We recently decided to update our plplot version, which we don’t do regurarly since it requires a few tweaks to work with/embed in our system. > > > > We (meaning my colleague and software person Tony Allen – in cc) have discovered a bug in the handling of exponential label placement. While we haven’t prepared a minimal example, the exchange below describes the symptoms. The attached plbox.c includes Tony’s correction. Hi Steve and Tony: @Steve: Thanks for passing along Tony's fix for a bug in PLplot that he noticed. @Tony: Most of this is addressed to you. But before you respond please take a few minutes to subscribe to plplot-devel so everybody subscribed to that list (not just those you directly e-mail) and those looking at the list archive later can see what you say. (See <https://sourceforge.net/projects/plplot/lists/plplot-devel> for the simple subscription directions and remember to always use your subscription address as the return address for your posts to this list or otherwise your message will be lost to everybody other than the list manager [who happens to be me].) I took your version of plbox.c (as forwarded by Steve), changed from Windows to Unix line endings (git assumes native line endings, e.g., Windows on Windows and Unix on Linux), commented out some debugging print statements, changed one comment reference from "qsas" to "QSAS" to be consistent with the same comment in another routine, and styled that result. So after those further changes here are the differences with the PLplot HEAD version (from the "git diff --ignore-all-space --unified=10" command with that last-parameter used to give lots of context). -------------------------------------- diff --git a/src/plbox.c b/src/plbox.c index b2b25840f..01c5aee7b 100644 --- a/src/plbox.c +++ b/src/plbox.c @@ -1470,21 +1470,21 @@ label_box( PLCHAR_VECTOR xopt, PLFLT xtick1, PLCHAR_VECTOR yopt, PLFLT ytick1 ) ldy = plP_stsearch( yopt, 'd' ); lfy = plP_stsearch( yopt, 'f' ); liy = plP_stsearch( yopt, 'i' ); lly = plP_stsearch( yopt, 'l' ); lmy = plP_stsearch( yopt, 'm' ); lny = plP_stsearch( yopt, 'n' ); lty = plP_stsearch( yopt, 't' ); lvy = plP_stsearch( yopt, 'v' ); loy = plP_stsearch( yopt, 'o' ); lxy = plP_stsearch( yopt, 'x' ); - + //printf( "=== yopt %d %d %d \n", lny, lty, lvy ); plP_xgvpw( &vpwxmin, &vpwxmax, &vpwymin, &vpwymax ); // vpwxmi always numerically less than vpwxma, and // similarly for vpwymi vpwxmi = ( vpwxmax > vpwxmin ) ? vpwxmin : vpwxmax; vpwxma = ( vpwxmax > vpwxmin ) ? vpwxmax : vpwxmin; vpwymi = ( vpwymax > vpwymin ) ? vpwymin : vpwymax; vpwyma = ( vpwymax > vpwymin ) ? vpwymax : vpwymin; // Write label(s) for horizontal axes. if ( ( lmx || lnx ) && ( ltx || lxx ) ) @@ -1819,21 +1819,24 @@ label_box( PLCHAR_VECTOR xopt, PLFLT xtick1, PLCHAR_VECTOR yopt, PLFLT ytick1 ) // Write separate exponential label if mode = 1. if ( !lly && !ldy && !loy && ymode ) { snprintf( string, STRING_LEN, "(x10%su%d%sd)", esc_string, (int) yscale, esc_string ); if ( custom_exponent_placement ) { height = ( (PLLabelDefaults *) plsc->label_data )->exp_label_disp; pos = ( (PLLabelDefaults *) plsc->label_data )->exp_label_pos; just = ( (PLLabelDefaults *) plsc->label_data )->exp_label_just; + plmtex( "t", height, pos, just, string ); // added for QSAS } + else + { if ( lvy ) { offset = 0.1; // more space to clear labels in "v" mode } else { offset = 0.02; } // Left axis exponent if ( lny ) @@ -1907,20 +1910,21 @@ label_box( PLCHAR_VECTOR xopt, PLFLT xtick1, PLCHAR_VECTOR yopt, PLFLT ytick1 ) } else { plmtex( "r", height, pos, just, string ); } } } } } } +} //-------------------------------------------------------------------------- // void label_box_custom() // // Writes numeric labels on side(s) of box in custom locations //-------------------------------------------------------------------------- void label_box_custom( PLCHAR_VECTOR xopt, PLINT n_xticks, PLFLT_VECTOR xticks, PLCHAR_VECTOR yopt, PLINT n_yticks, PLFLT_VECTOR yticks ) { @@ -1931,21 +1935,21 @@ label_box_custom( PLCHAR_VECTOR xopt, PLINT n_xticks, PLFLT_VECTOR xticks, PLCHA PLFLT vpwxmin, vpwxmax, vpwymin, vpwymax; PLFLT tn, offset; PLCHAR_VECTOR timefmt; PLINT i; PLINT xdigmax, xdigits, xdigmax_old, xdigits_old; PLINT ydigmax, ydigits, ydigmax_old, ydigits_old; PLINT lxmin, lxmax, lymin, lymax; PLINT pxmin, pxmax, pymin, pymax; PLFLT default_mm, char_height_mm, height_mm; PLFLT string_length_mm = 0.0, pos_mm = 0.0; - + //printf( "=== custom\n" ); // Assume label data is for placement of exponents if no custom // label function is provided. PLBOOL custom_exponent_placement = !plsc->label_func && plsc->label_data; // pos, height, and just are unnecessarily set to quiet // -O3 -Wuninitialized warnings that are obvious false alarms from // the clarity of the code associated with the true or false // result for custom_exponent_placement. PLFLT pos = 0.0, height = 0.0, just = 0.0; PLCHAR_VECTOR esc_string = plgesc_string(); @@ -2384,20 +2388,21 @@ label_box_custom( PLCHAR_VECTOR xopt, PLINT n_xticks, PLFLT_VECTOR xticks, PLCHA // Write separate exponential label if mode = 1. if ( !lly && !ldy && !loy && ymode ) { snprintf( string, STRING_LEN, "(x10%su%d%sd)", esc_string, (int) yscale, esc_string ); if ( custom_exponent_placement ) { height = ( (PLLabelDefaults *) plsc->label_data )->exp_label_disp; pos = ( (PLLabelDefaults *) plsc->label_data )->exp_label_pos; just = ( (PLLabelDefaults *) plsc->label_data )->exp_label_just; + plmtex( "t", height, pos, just, string ); //added for QSAS } if ( lvy ) { offset = 0.1; // more space to clear labels in "v" mode } else { offset = 0.02; } // Left axis exponent. -------------------------------------- For the record, label_box_custom is a private function that is only called by the static function draw_box in src/pllegend.c, which in turn is only called by c_plcolorbar within that source file. So label_box_custom is a misnomer, and I plan (after the current release is done) to address that issue by changing that name to label_box_colorbar. But from now on in this e-mail I will refer to that function with its present name of label_box_custom. (@ Everybody: Note that Tony's fix is limited to changes when custom_exponent_placement is true, i.e., when there is no custom label function but (custom?) label data are still provided to change the placement of the exponential label.) @Tony: We really do need a simple test case for both the revised label_box and label_box_custom functions. Could you provide that? I suggest that simple example doesn't need any data to plot, but it does need to contain a labelled box and a simple colorbar (see example 16 for an example of a simple colorbar) to exercise both label_box and label_box_custom. And, of course, both the box and colorbar should have exponential labels and appropriate label data should be provided in each case to exercise the cases for both functions where custom_exponent_placement is true. It has been a long time since I looked at plbox.c, but my impression now is label_box_custom is quite similar to label_box code so I am pretty sure label_box_custom badly needs documentation about exactly how and why it is changed from label_box for the special needs of plcolorbar. So providing such documentation is on my post-release agenda, but in the absence of such documentation I am frankly currently ignorant of the designed differences between label_box and label_box_custom. However, with that caveat I don't understand why your fix is so different in the two separate functions. For example, prior to your fix label_box and label_box_custom had very similar custom_exponent_placement logic, but now they are quite different (e.g., the above added "else" in label_box that skips a huge amount of code is not present in label_box_custom). Did you really mean to have such quite different logic in those two cases? Even if the present fix works, it would be best for the logic in those two functions to be implemented as similarly as possible. Assuming your reply to my questions includes another revised version of plbox.c, I don't care about the line endings and whitespace styling changes I made since those are easy to do again. However, please do incorporate my further changes (commenting out of the print functions and the qsas ==> QSAS change) in your revised code. N.B. I am happy to take a quick break (as now) from getting out the 5.14.0 release to discuss these changes with you, but we are in deep freeze now (i.e., it is obviously much too late in the present release cycle to get these or virtually any other code change into that release). But once you provide a good simple test example to make sure all is well with these changes, it would be ideal to get these changes accepted for PLplot early in the release cycle for 5.15.0. Best wishes, Alan __________________________ Alan W. Irwin Programming affiliations with the FreeEOS equation-of-state implementation for stellar interiors (freeeos.sf.net); the Time Ephemerides project (timeephem.sf.net); PLplot scientific plotting software package (plplot.sf.net); the libLASi project (unifont.org/lasi); the Loads of Linux Links project (loll.sf.net); and the Linux Brochure Project (lbproject.sf.net). __________________________ Linux-powered Science __________________________ |