From: Mark de W. <m.d...@da...> - 2013-01-02 09:27:12
Attachments:
example.patch
fix.patch
|
Hi, In my application I use a custom label function to plot the time on the x-axis. When the values on the y-axis are large there's a exponent label with '(x10#u%d#d)' as label. The placement of this label seems to be wrong; on Linux it's drawn on the right side of the y-axis inside the plot itself, on Windows it's not drawn at all. The behaviour might be seen with the attached patch for example 01. (Might be since the behaviour depends on the environment it's build in.) I tracked the issue down and created a patch to fix the issue. At least the code now matches with the comment; The original code (plbox.c:1510) read: // Assume label data is for placement of exponents if no custom // label function is provided. if ( plsc->label_data ) { The attached patch fixes the problem for my use case, but I'm not sure it's enough. I haven't been able to find this behaviour in the documentation and wonder whether this behaviour is intentionally and should be documented, or is not intended. The documentation of the label_func argument of plslabelfunc seems to indicate it's not intended "This is the custom label function. In order to reset to the default labeling, set this to NULL." It doesn't mention that the label_data also needs to be set to NULL to reset the default behaviour of the placement of the exponent label. The current behaviour makes it impossible to combine a custom label function and change the placement of exponent labels. This is not something I need, but I wonder whether it's wanted to combine this behaviour. Another argument against the combination is that the plslabelfunc takes a void pointer and then access the pointer as if it points to a PLLabelDefaults structure. If the user, for example, sends a pointer to an ingeger, it might cause accessing memory out of bounds and can casting `random' data to a floating point value. When documented it becomes a smaller issue, but might still catch some users by surprise. Regards, Mark de Wever |
From: Mark de W. <m.d...@da...> - 2013-03-25 09:03:01
Attachments:
example.patch
fix.patch
|
Hi, I didn't get a reponse to the original email so resending it. If this is not the proper mailinglist for this issue, please let me know. In my application I use a custom label function to plot the time on the x-axis. When the values on the y-axis are large there's a exponent label with '(x10#u%d#d)' as label. The placement of this label seems to be wrong; on Linux it's drawn on the right side of the y-axis inside the plot itself, on Windows it's not drawn at all. The behaviour might be seen with the attached patch for example 01. (Might be since the behaviour depends on the environment it's build in.) I tracked the issue down and created a patch to fix the issue. At least the code now matches with the comment; The original code (plbox.c:1510) read: // Assume label data is for placement of exponents if no custom // label function is provided. if ( plsc->label_data ) { The attached patch fixes the problem for my use case, but I'm not sure it's enough. I haven't been able to find this behaviour in the documentation and wonder whether this behaviour is intentionally and should be documented, or is not intended. The documentation of the label_func argument of plslabelfunc seems to indicate it's not intended "This is the custom label function. In order to reset to the default labeling, set this to NULL." It doesn't mention that the label_data also needs to be set to NULL to reset the default behaviour of the placement of the exponent label. The current behaviour makes it impossible to combine a custom label function and change the placement of exponent labels. This is not something I need, but I wonder whether it's wanted to combine this behaviour. Another argument against the combination is that the plslabelfunc takes a void pointer and then access the pointer as if it points to a PLLabelDefaults structure. If the user, for example, sends a pointer to an ingeger, it might cause accessing memory out of bounds and can casting `random' data to a floating point value. When documented it becomes a smaller issue, but might still catch some users by surprise. Regards, Mark de Wever |
From: Alan W. I. <ir...@be...> - 2013-03-29 00:37:30
|
On 2013-03-25 09:33+0100 Mark de Wever wrote: > Hi, > > I didn't get a reponse to the original email so resending it. If this is not > the proper mailinglist for this issue, please let me know. @Mark: This is the right place to post concerning PLplot development issues such as the one you have found. And although we are all volunteers just working on PLplot in our spare time, it is certainly okay for you to ping this list if you don't receive a timely response. Here is my response to your fix.patch. I think it is correct to check for a NULL plsc->label_func (at least according to the in-code documentation) in the four locations you have spotted. And I cannot find any other case where this change should obviously be made. Therefore, I have tentatively applied your patch (revision 12302). However, I think this revision needs further review because I am not that expert in custom labelling. @Hez: will you please review revision 12302? Alan __________________________ Alan W. Irwin Astronomical research affiliation with Department of Physics and Astronomy, University of Victoria (astrowww.phys.uvic.ca). 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 __________________________ |
From: Hezekiah M. C. <hez...@us...> - 2013-04-28 10:20:41
|
On Thu, Mar 28, 2013 at 8:37 PM, Alan W. Irwin <ir...@be...> wrote: > On 2013-03-25 09:33+0100 Mark de Wever wrote: > >> Hi, >> >> I didn't get a reponse to the original email so resending it. If this is >> not the proper mailinglist for this issue, please let me know. > > @Hez: will you please review revision 12302? > The commit looks ok to me. Thank you for doing the initial review and application of the patch. Mark - thanks for the patch! Hez |
From: Mark de W. <m.d...@da...> - 2013-04-29 12:20:25
|
Hi Alan, Seems I didn't notice your reply until today. Alan W. Irwin wrote: > On 2013-03-25 09:33+0100 Mark de Wever wrote: > > This is the right place to post concerning PLplot development issues > such as the one you have found. Ok thanks for the confirmation. > Therefore, I have tentatively applied your patch > (revision 12302). However, I think this revision needs further review > because I am not that expert in custom labelling. Thanks for applying the patch and Hez for reviewing. Regards, Mark de Wever |